Tidy ownership/lifetime of GLVideoView to fix crashes on close.

diff --git a/src/wx/controls.cc b/src/wx/controls.cc
index 700769a50c7543cdaee030223128facda4541323..29f4aeaa364e2416de48dda239f25e2ebbd8ee27 100644 (file)
--- a/src/wx/controls.cc
+++ b/src/wx/controls.cc
@@ -144,13 +144,13 @@ Controls::Controls (wxWindow* parent, shared_ptr<FilmViewer> 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<string> 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 9a22d770908ae6d6e581fbce6427b59c4389ba72..377960425ba2807e5982ede7f25ffd4ec8e376a5 100644 (file)
--- a/src/wx/controls.h
+++ b/src/wx/controls.h
@@ -79,7 +79,7 @@ protected:
        wxBoxSizer* _button_sizer;
        std::shared_ptr<Film> _film;
        wxSlider* _slider;
-       std::shared_ptr<FilmViewer> _viewer;
+       std::weak_ptr<FilmViewer> _viewer;
        boost::optional<std::string> _active_job;
 
 private:

diff --git a/src/wx/film_viewer.cc b/src/wx/film_viewer.cc
index a3c015ab197299d0225408906026316f19e781a5..77c2e85d690a6cb7055dc02063412812abd1b22b 100644 (file)
--- 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<GLVideoView>(this, p);
                break;
        case Config::VIDEO_VIEW_SIMPLE:
-               _video_view = new SimpleVideoView (this, p);
+               _video_view = std::make_shared<SimpleVideoView>(this, p);
                break;
        }
 

diff --git a/src/wx/film_viewer.h b/src/wx/film_viewer.h
index ef5ce41c925d2e535204c9c0d9fd441f02791829..c467eedc1645d806495ea0d01cf1ddbd59cfed6b 100644 (file)
--- 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<const VideoView> video_view () const {
                return _video_view;
        }
 
@@ -169,7 +169,7 @@ private:
        std::shared_ptr<Film> _film;
        std::shared_ptr<Player> _player;
 
-       VideoView* _video_view = nullptr;
+       std::shared_ptr<VideoView> _video_view;
        bool _coalesce_player_changes = false;
        std::vector<int> _pending_player_changes;
 

diff --git a/src/wx/playlist_controls.cc b/src/wx/playlist_controls.cc
index d65cb0fccaf2999574405a86c73017823916cba3..129e0ceca407eee9990cac7a69e092552c8bb90e 100644 (file)
--- a/src/wx/playlist_controls.cc
+++ b/src/wx/playlist_controls.cc
@@ -109,7 +109,7 @@ PlaylistControls::PlaylistControls (wxWindow* parent, shared_ptr<FilmViewer> 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 1e4ecc8d7fc74e5469c76759272110d209f42f1d..6196c1b5c29e10c39fcb55277a7e833116d7cf3c 100644 (file)
--- 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 968cd57404dd729957d97f842a7dd82482771807..7aabf1bf52cbed9bce3f3ae73f10e0332d35010d 100644 (file)
--- 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<FilmViewer> weak_viewer)
        : TableDialog (parent, _("System information"), 2, 1, false)
 {
-       shared_ptr<FilmViewer> viewer = weak_viewer.lock ();
-       GLVideoView const * gl = viewer ? dynamic_cast<GLVideoView const *>(viewer->video_view()) : 0;
+       auto viewer = weak_viewer.lock ();
+       shared_ptr<const GLVideoView> gl;
+       if (viewer) {
+               gl = std::dynamic_pointer_cast<const GLVideoView>(viewer->video_view());
+       }
 
        if (!gl) {
                add (_("OpenGL version"), true);