Don't bind a shared_ptr<PlayerVideo> to ImageChanged (#3013).

Otherwise if the GUI is busy when the emissions build up, each one holds
a reference to a potentially large image.

This caused enormous memory use when playing a DCP and verifying it at
the same time.

diff --git a/src/tools/dcpomatic.cc b/src/tools/dcpomatic.cc
index 78fce27f224e0e8f36c714d7df0ea7c0f7c2b1a3..8677acc18bd7824d21e345fbeeeea2a24dcd4477 100644 (file)
--- a/src/tools/dcpomatic.cc
+++ b/src/tools/dcpomatic.cc
@@ -408,6 +408,12 @@ public:
                add_accelerators ();
        }
 
+       ~DOMFrame()
+       {
+               /* This holds a reference to FilmViewer, so get rid of it first */
+               _video_waveform_dialog.reset();
+       }
+
        void add_accelerators ()
        {
 #ifdef __WXOSX__

diff --git a/src/wx/content_menu.cc b/src/wx/content_menu.cc
index 62421a20bfeeeeb3e3d04cf5ee24a0378cfd75d0..559b57553ce4d97628539d53b48bb09a48b5330f 100644 (file)
--- a/src/wx/content_menu.cc
+++ b/src/wx/content_menu.cc
@@ -586,7 +586,7 @@ ContentMenu::auto_crop ()
        });
 
        /* Also update the dialog and view when we're looking at a different frame */
-       _auto_crop_viewer_connection = _viewer.ImageChanged.connect([this, guess_crop_for_content, update_viewer](shared_ptr<PlayerVideo>) {
+       _auto_crop_viewer_connection = _viewer.ImageChanged.connect([this, guess_crop_for_content, update_viewer]() {
                auto const crop = guess_crop_for_content();
                _auto_crop_dialog->set(crop);
                update_viewer(crop);

diff --git a/src/wx/film_viewer.cc b/src/wx/film_viewer.cc
index 9574073381c84221cf32bbb0626740c6f9e2a25c..0efa75380abe65bfe20f012a8233e57875ad5d58 100644 (file)
--- a/src/wx/film_viewer.cc
+++ b/src/wx/film_viewer.cc
@@ -864,7 +864,15 @@ FilmViewer::gets () const
 void
 FilmViewer::image_changed (shared_ptr<PlayerVideo> pv)
 {
-       emit (boost::bind(boost::ref(ImageChanged), pv));
+       _last_image = pv;
+       emit(boost::bind(boost::ref(ImageChanged)));
+}
+
+
+shared_ptr<const PlayerVideo>
+FilmViewer::last_image() const
+{
+       return _last_image;
 }
 
 

diff --git a/src/wx/film_viewer.h b/src/wx/film_viewer.h
index 8f0cad54577aac9d1cf4bd65261c5c7ac07cb716..c8aade6c072c887c2da519750461fa5452c49319 100644 (file)
--- a/src/wx/film_viewer.h
+++ b/src/wx/film_viewer.h
@@ -155,7 +155,10 @@ public:
 
        Frame average_latency() const;
 
-       boost::signals2::signal<void (std::shared_ptr<PlayerVideo>)> ImageChanged;
+       /** The image we are viewing changed: call last_image() to get the image */
+       boost::signals2::signal<void ()> ImageChanged;
+       std::shared_ptr<const PlayerVideo> last_image() const;
+
        boost::signals2::signal<void ()> Started;
        boost::signals2::signal<void ()> Stopped;
        /** While playing back we reached the end of the film (emitted from GUI thread) */
@@ -222,5 +225,10 @@ private:
 
        boost::optional<dcpomatic::Rect<float>> _crop_guess;
 
+       /** Keep track of the image that we were talking about with the last
+        *  emission of ImageChanged.
+        */
+       std::shared_ptr<const PlayerVideo> _last_image;
+
        boost::signals2::scoped_connection _config_changed_connection;
 };

diff --git a/src/wx/video_waveform_plot.cc b/src/wx/video_waveform_plot.cc
index ebff68096ea86b7944549047b8287bccd8c20655..2827689a72b5aefb48092726cf8388d95d50262b 100644 (file)
--- a/src/wx/video_waveform_plot.cc
+++ b/src/wx/video_waveform_plot.cc
@@ -56,12 +56,13 @@ int const VideoWaveformPlot::_x_axis_width = 52;
 VideoWaveformPlot::VideoWaveformPlot(wxWindow* parent, weak_ptr<const Film> film, FilmViewer& viewer)
        : wxPanel (parent, wxID_ANY, wxDefaultPosition, wxDefaultSize, wxFULL_REPAINT_ON_RESIZE)
        , _film (film)
+       , _viewer(viewer)
 {
 #ifndef __WXOSX__
        SetDoubleBuffered (true);
 #endif
 
-       _viewer_connection = viewer.ImageChanged.connect(boost::bind(&VideoWaveformPlot::set_image, this, _1));
+       _viewer_connection = _viewer.ImageChanged.connect(boost::bind(&VideoWaveformPlot::set_image, this));
 
        Bind (wxEVT_PAINT, boost::bind(&VideoWaveformPlot::paint, this));
        Bind (wxEVT_SIZE,  boost::bind(&VideoWaveformPlot::sized, this, _1));
@@ -186,7 +187,7 @@ VideoWaveformPlot::create_waveform ()
 
 
 void
-VideoWaveformPlot::set_image (shared_ptr<PlayerVideo> image)
+VideoWaveformPlot::set_image()
 {
        if (!_enabled) {
                return;
@@ -195,7 +196,7 @@ VideoWaveformPlot::set_image (shared_ptr<PlayerVideo> image)
        /* We must copy the PlayerVideo here as we will call ::image() on it, potentially
           with a different pixel_format than was used when ::prepare() was called.
        */
-       _image = DCPVideo::convert_to_xyz(image->shallow_copy());
+       _image = DCPVideo::convert_to_xyz(_viewer.last_image()->shallow_copy());
        _dirty = true;
        Refresh ();
 }

diff --git a/src/wx/video_waveform_plot.h b/src/wx/video_waveform_plot.h
index f482da8f8a2d75cb2876071abbfcf5aed0c11519..e6e7a6f3da3a39c82394898966e9c6f61148ca8b 100644 (file)
--- a/src/wx/video_waveform_plot.h
+++ b/src/wx/video_waveform_plot.h
@@ -56,7 +56,7 @@ private:
        void paint ();
        void sized (wxSizeEvent &);
        void create_waveform ();
-       void set_image (std::shared_ptr<PlayerVideo>);
+       void set_image();
        void mouse_moved (wxMouseEvent &);
 
        std::weak_ptr<const Film> _film;
@@ -67,6 +67,8 @@ private:
        int _component = 0;
        int _contrast = 0;
 
+       FilmViewer& _viewer;
+
        static int const _vertical_margin;
        static int const _pixel_values;
        static int const _x_axis_width;