From: Carl Hetherington Date: Sat, 11 Sep 2021 23:09:03 +0000 (+0200) Subject: Tidy ownership/lifetime of GLVideoView to fix crashes on close. X-Git-Tag: v2.15.163~1^2~23 X-Git-Url: https://git.carlh.net/gitweb/?p=dcpomatic.git;a=commitdiff_plain;h=0b791ea708dfa1f5cd44522988dd5efdf2a0b94b Tidy ownership/lifetime of GLVideoView to fix crashes on close. --- diff --git a/src/wx/controls.cc b/src/wx/controls.cc index 700769a50..29f4aeaa3 100644 --- a/src/wx/controls.cc +++ b/src/wx/controls.cc @@ -144,13 +144,13 @@ Controls::Controls (wxWindow* parent, shared_ptr viewer, bool editor _jump_to_selected->SetValue (Config::instance()->jump_to_selected ()); } - _viewer->Started.connect (boost::bind(&Controls::started, this)); - _viewer->Stopped.connect (boost::bind(&Controls::stopped, this)); + viewer->Started.connect (boost::bind(&Controls::started, this)); + viewer->Stopped.connect (boost::bind(&Controls::stopped, this)); Bind (wxEVT_TIMER, boost::bind(&Controls::update_position, this)); _timer.Start (80, wxTIMER_CONTINUOUS); - set_film (_viewer->film()); + set_film (viewer->film()); setup_sensitivity (); @@ -186,7 +186,12 @@ Controls::stopped () void Controls::update_position () { - if (!_slider_being_moved && !_viewer->pending_idle_get()) { + auto viewer = _viewer.lock (); + if (!viewer) { + return; + } + + if (!_slider_being_moved && !viewer->pending_idle_get()) { update_position_label (); update_position_slider (); } @@ -196,14 +201,24 @@ Controls::update_position () void Controls::eye_changed () { - _viewer->set_eyes (_eye->GetSelection() == 0 ? Eyes::LEFT : Eyes::RIGHT); + auto viewer = _viewer.lock (); + if (!viewer) { + return; + } + + viewer->set_eyes (_eye->GetSelection() == 0 ? Eyes::LEFT : Eyes::RIGHT); } void Controls::outline_content_changed () { - _viewer->set_outline_content (_outline_content->GetValue()); + auto viewer = _viewer.lock (); + if (!viewer) { + return; + } + + viewer->set_outline_content (_outline_content->GetValue()); } @@ -211,13 +226,14 @@ Controls::outline_content_changed () void Controls::slider_moved (bool page) { - if (!_film) { + auto viewer = _viewer.lock (); + if (!_film || !viewer) { return; } if (!page && !_slider_being_moved) { /* This is the first event of a drag; stop playback for the duration of the drag */ - _viewer->suspend (); + viewer->suspend (); _slider_being_moved = true; } @@ -228,10 +244,10 @@ Controls::slider_moved (bool page) */ bool accurate = false; if (t >= _film->length ()) { - t = _film->length() - _viewer->one_video_frame(); + t = _film->length() - viewer->one_video_frame(); accurate = true; } - _viewer->seek (t, accurate); + viewer->seek (t, accurate); update_position_label (); log ( @@ -245,8 +261,13 @@ Controls::slider_moved (bool page) void Controls::slider_released () { + auto viewer = _viewer.lock (); + if (!viewer) { + return; + } + /* Restart after a drag */ - _viewer->resume (); + viewer->resume (); _slider_being_moved = false; } @@ -259,10 +280,15 @@ Controls::update_position_slider () return; } + auto viewer = _viewer.lock (); + if (!viewer) { + return; + } + auto const len = _film->length (); if (len.get ()) { - int const new_slider_position = 4096 * _viewer->position().get() / len.get(); + int const new_slider_position = 4096 * viewer->position().get() / len.get(); if (new_slider_position != _slider->GetValue()) { _slider->SetValue (new_slider_position); } @@ -279,10 +305,15 @@ Controls::update_position_label () return; } + auto viewer = _viewer.lock (); + if (!viewer) { + return; + } + double const fps = _film->video_frame_rate (); /* Count frame number from 1 ... not sure if this is the best idea */ - checked_set (_frame_number, wxString::Format (wxT("%ld"), lrint (_viewer->position().seconds() * fps) + 1)); - checked_set (_timecode, time_to_timecode (_viewer->position(), fps)); + checked_set (_frame_number, wxString::Format (wxT("%ld"), lrint (viewer->position().seconds() * fps) + 1)); + checked_set (_timecode, time_to_timecode (viewer->position(), fps)); } @@ -297,7 +328,12 @@ Controls::active_jobs_changed (optional j) DCPTime Controls::nudge_amount (wxKeyboardState& ev) { - auto amount = _viewer->one_video_frame (); + auto viewer = _viewer.lock (); + if (!viewer) { + return {}; + } + + auto amount = viewer->one_video_frame (); if (ev.ShiftDown() && !ev.ControlDown()) { amount = DCPTime::from_seconds (1); @@ -314,7 +350,10 @@ Controls::nudge_amount (wxKeyboardState& ev) void Controls::rewind_clicked (wxMouseEvent& ev) { - _viewer->seek (DCPTime(), true); + auto viewer = _viewer.lock (); + if (viewer) { + viewer->seek (DCPTime(), true); + } ev.Skip(); } @@ -322,28 +361,40 @@ Controls::rewind_clicked (wxMouseEvent& ev) void Controls::back_frame () { - _viewer->seek_by (-_viewer->one_video_frame(), true); + auto viewer = _viewer.lock (); + if (viewer) { + viewer->seek_by (-viewer->one_video_frame(), true); + } } void Controls::forward_frame () { - _viewer->seek_by (_viewer->one_video_frame(), true); + auto viewer = _viewer.lock (); + if (viewer) { + viewer->seek_by (viewer->one_video_frame(), true); + } } void Controls::back_clicked (wxKeyboardState& ev) { - _viewer->seek_by (-nudge_amount(ev), true); + auto viewer = _viewer.lock (); + if (viewer) { + viewer->seek_by (-nudge_amount(ev), true); + } } void Controls::forward_clicked (wxKeyboardState& ev) { - _viewer->seek_by (nudge_amount(ev), true); + auto viewer = _viewer.lock (); + if (viewer) { + viewer->seek_by (nudge_amount(ev), true); + } } @@ -376,9 +427,14 @@ Controls::setup_sensitivity () void Controls::timecode_clicked () { - auto dialog = new PlayheadToTimecodeDialog (this, _viewer->position(), _film->video_frame_rate()); + auto viewer = _viewer.lock (); + if (!viewer) { + return; + } + + auto dialog = new PlayheadToTimecodeDialog (this, viewer->position(), _film->video_frame_rate()); if (dialog->ShowModal() == wxID_OK) { - _viewer->seek (dialog->get(), true); + viewer->seek (dialog->get(), true); } dialog->Destroy (); } @@ -387,9 +443,14 @@ Controls::timecode_clicked () void Controls::frame_number_clicked () { - auto dialog = new PlayheadToFrameDialog (this, _viewer->position(), _film->video_frame_rate()); + auto viewer = _viewer.lock (); + if (!viewer) { + return; + } + + auto dialog = new PlayheadToFrameDialog (this, viewer->position(), _film->video_frame_rate()); if (dialog->ShowModal() == wxID_OK) { - _viewer->seek (dialog->get(), true); + viewer->seek (dialog->get(), true); } dialog->Destroy (); } diff --git a/src/wx/controls.h b/src/wx/controls.h index 9a22d7709..377960425 100644 --- a/src/wx/controls.h +++ b/src/wx/controls.h @@ -79,7 +79,7 @@ protected: wxBoxSizer* _button_sizer; std::shared_ptr _film; wxSlider* _slider; - std::shared_ptr _viewer; + std::weak_ptr _viewer; boost::optional _active_job; private: diff --git a/src/wx/film_viewer.cc b/src/wx/film_viewer.cc index a3c015ab1..77c2e85d6 100644 --- a/src/wx/film_viewer.cc +++ b/src/wx/film_viewer.cc @@ -96,10 +96,10 @@ FilmViewer::FilmViewer (wxWindow* p) { switch (Config::instance()->video_view_type()) { case Config::VIDEO_VIEW_OPENGL: - _video_view = new GLVideoView (this, p); + _video_view = std::make_shared(this, p); break; case Config::VIDEO_VIEW_SIMPLE: - _video_view = new SimpleVideoView (this, p); + _video_view = std::make_shared(this, p); break; } diff --git a/src/wx/film_viewer.h b/src/wx/film_viewer.h index ef5ce41c9..c467eedc1 100644 --- a/src/wx/film_viewer.h +++ b/src/wx/film_viewer.h @@ -62,7 +62,7 @@ public: return _video_view->get(); } - VideoView const * video_view () const { + std::shared_ptr video_view () const { return _video_view; } @@ -169,7 +169,7 @@ private: std::shared_ptr _film; std::shared_ptr _player; - VideoView* _video_view = nullptr; + std::shared_ptr _video_view; bool _coalesce_player_changes = false; std::vector _pending_player_changes; diff --git a/src/wx/playlist_controls.cc b/src/wx/playlist_controls.cc index d65cb0fcc..129e0ceca 100644 --- a/src/wx/playlist_controls.cc +++ b/src/wx/playlist_controls.cc @@ -109,7 +109,7 @@ PlaylistControls::PlaylistControls (wxWindow* parent, shared_ptr vie _previous_button->Bind (wxEVT_BUTTON, boost::bind(&PlaylistControls::previous_clicked, this)); _spl_view->Bind (wxEVT_LIST_ITEM_SELECTED, boost::bind(&PlaylistControls::spl_selection_changed, this)); _spl_view->Bind (wxEVT_LIST_ITEM_DESELECTED, boost::bind(&PlaylistControls::spl_selection_changed, this)); - _viewer->Finished.connect (boost::bind(&PlaylistControls::viewer_finished, this)); + viewer->Finished.connect (boost::bind(&PlaylistControls::viewer_finished, this)); _refresh_spl_view->Bind (wxEVT_BUTTON, boost::bind(&PlaylistControls::update_playlist_directory, this)); _refresh_content_view->Bind (wxEVT_BUTTON, boost::bind(&ContentView::update, _content_view)); @@ -148,18 +148,26 @@ PlaylistControls::deselect_playlist () void PlaylistControls::play_clicked () { - _viewer->start (); + auto viewer = _viewer.lock (); + if (viewer) { + viewer->start (); + } } void PlaylistControls::setup_sensitivity () { + auto viewer = _viewer.lock (); + if (!viewer) { + return; + } + Controls::setup_sensitivity (); bool const active_job = _active_job && *_active_job != "examine_content"; bool const c = _film && !_film->content().empty() && !active_job; - _play_button->Enable (c && !_viewer->playing()); - _pause_button->Enable (_viewer->playing()); - _spl_view->Enable (!_viewer->playing()); + _play_button->Enable (c && !viewer->playing()); + _pause_button->Enable (viewer->playing()); + _spl_view->Enable (!viewer->playing()); _next_button->Enable (can_do_next()); _previous_button->Enable (can_do_previous()); } @@ -167,14 +175,22 @@ PlaylistControls::setup_sensitivity () void PlaylistControls::pause_clicked () { - _viewer->stop (); + auto viewer = _viewer.lock (); + if (viewer) { + viewer->stop (); + } } void PlaylistControls::stop_clicked () { - _viewer->stop (); - _viewer->seek (DCPTime(), true); + auto viewer = _viewer.lock (); + if (!viewer) { + return; + } + + viewer->stop (); + viewer->seek (DCPTime(), true); if (_selected_playlist) { _selected_playlist_position = 0; update_current_content (); @@ -436,7 +452,8 @@ PlaylistControls::update_current_content () void PlaylistControls::viewer_finished () { - if (!_selected_playlist) { + auto viewer = _viewer.lock (); + if (!_selected_playlist || !viewer) { return; } @@ -444,7 +461,7 @@ PlaylistControls::viewer_finished () if (_selected_playlist_position < int(_playlists[*_selected_playlist].get().size())) { /* Next piece of content on the SPL */ update_current_content (); - _viewer->start (); + viewer->start (); } else { /* Finished the whole SPL */ _selected_playlist_position = 0; diff --git a/src/wx/standard_controls.cc b/src/wx/standard_controls.cc index 1e4ecc8d7..6196c1b5c 100644 --- a/src/wx/standard_controls.cc +++ b/src/wx/standard_controls.cc @@ -63,14 +63,15 @@ StandardControls::play_clicked () void StandardControls::check_play_state () { - if (!_film || _film->video_frame_rate() == 0) { + auto viewer = _viewer.lock (); + if (!_film || _film->video_frame_rate() == 0 || !viewer) { return; } if (_play_button->GetValue()) { - _viewer->start (); + viewer->start (); } else { - _viewer->stop (); + viewer->stop (); } } diff --git a/src/wx/system_information_dialog.cc b/src/wx/system_information_dialog.cc index 968cd5740..7aabf1bf5 100644 --- a/src/wx/system_information_dialog.cc +++ b/src/wx/system_information_dialog.cc @@ -38,8 +38,11 @@ using std::shared_ptr; SystemInformationDialog::SystemInformationDialog (wxWindow* parent, weak_ptr weak_viewer) : TableDialog (parent, _("System information"), 2, 1, false) { - shared_ptr viewer = weak_viewer.lock (); - GLVideoView const * gl = viewer ? dynamic_cast(viewer->video_view()) : 0; + auto viewer = weak_viewer.lock (); + shared_ptr gl; + if (viewer) { + gl = std::dynamic_pointer_cast(viewer->video_view()); + } if (!gl) { add (_("OpenGL version"), true);