Do image crop/scale/window in the butler prepare threads.

diff --git a/src/lib/butler.cc b/src/lib/butler.cc
index 018334c7e3db12022bb6742176620d93b232c3d6..409963016f76304d7829d47f2170984c9799fe1d 100644 (file)
--- a/src/lib/butler.cc
+++ b/src/lib/butler.cc
@@ -1,5 +1,5 @@
 /*
-    Copyright (C) 2016-2017 Carl Hetherington <cth@carlh.net>
+    Copyright (C) 2016-2018 Carl Hetherington <cth@carlh.net>
 
     This file is part of DCP-o-matic.
 
@@ -39,6 +39,7 @@ using boost::weak_ptr;
 using boost::shared_ptr;
 using boost::bind;
 using boost::optional;
+using boost::function;
 
 /** Minimum video readahead in frames */
 #define MINIMUM_VIDEO_READAHEAD 10
@@ -49,7 +50,20 @@ using boost::optional;
 /** Minimum audio readahead in frames; should never be reached unless there are bugs in Player */
 #define MAXIMUM_AUDIO_READAHEAD (48000 * MAXIMUM_VIDEO_READAHEAD / 24)
 
-Butler::Butler (shared_ptr<Player> player, shared_ptr<Log> log, AudioMapping audio_mapping, int audio_channels)
+/** @param pixel_format Pixel format functor that will be used when calling ::image on PlayerVideos coming out of this
+ *  butler.  This will be used (where possible) to prepare the PlayerVideos so that calling image() on them is quick().
+ *  @param aligned Same as above for the `aligned' flag.
+ *  @param fast Same as above for the `fast' flag.
+ */
+Butler::Butler (
+       shared_ptr<Player> player,
+       shared_ptr<Log> log,
+       AudioMapping audio_mapping,
+       int audio_channels,
+       function<AVPixelFormat (AVPixelFormat)> pixel_format,
+       bool aligned,
+       bool fast
+       )
        : _player (player)
        , _log (log)
        , _prepare_work (new boost::asio::io_service::work (_prepare_service))
@@ -61,6 +75,9 @@ Butler::Butler (shared_ptr<Player> player, shared_ptr<Log> log, AudioMapping aud
        , _audio_mapping (audio_mapping)
        , _audio_channels (audio_channels)
        , _disable_audio (false)
+       , _pixel_format (pixel_format)
+       , _aligned (aligned)
+       , _fast (fast)
 {
        _player_video_connection = _player->Video.connect (bind (&Butler::video, this, _1, _2));
        _player_audio_connection = _player->Audio.connect (bind (&Butler::audio, this, _1, _2));
@@ -266,7 +283,7 @@ Butler::prepare (weak_ptr<PlayerVideo> weak_video) const
                        LOG_TIMING("start-prepare in %1", thread_id());
                }
 
-               video->prepare ();
+               video->prepare (_pixel_format, _aligned, _fast);
 
                if (_log) {
                        LOG_TIMING("finish-prepare in %1", thread_id());

diff --git a/src/lib/butler.h b/src/lib/butler.h
index fb133d10809413298364faba4e02a8d62c42c5b1..b1debfca2b2c948bb407cb8faa3cc844d6444ae3 100644 (file)
--- a/src/lib/butler.h
+++ b/src/lib/butler.h
@@ -37,7 +37,16 @@ class Log;
 class Butler : public ExceptionStore, public boost::noncopyable
 {
 public:
-       Butler (boost::shared_ptr<Player> player, boost::shared_ptr<Log> log, AudioMapping map, int audio_channels);
+       Butler (
+               boost::shared_ptr<Player> player,
+               boost::shared_ptr<Log> log,
+               AudioMapping map,
+               int audio_channels,
+               boost::function<AVPixelFormat (AVPixelFormat)> pixel_format,
+               bool aligned,
+               bool fast
+               );
+
        ~Butler ();
 
        void seek (DCPTime position, bool accurate);
@@ -99,6 +108,10 @@ private:
 
        bool _disable_audio;
 
+       boost::function<AVPixelFormat (AVPixelFormat)> _pixel_format;
+       bool _aligned;
+       bool _fast;
+
        /** If we are waiting to be refilled following a seek, this is the time we were
            seeking to.
        */

diff --git a/src/lib/ffmpeg_encoder.cc b/src/lib/ffmpeg_encoder.cc
index eb3b0c28a17af40f1fbb05168a2868d00fa8bc59..bb696055c5761cdc2e9a5d2dc0c5a7cf81a1f45e 100644 (file)
--- a/src/lib/ffmpeg_encoder.cc
+++ b/src/lib/ffmpeg_encoder.cc
@@ -107,7 +107,7 @@ FFmpegEncoder::FFmpegEncoder (
                }
        }
 
-       _butler.reset (new Butler (_player, film->log(), map, _output_audio_channels));
+       _butler.reset (new Butler (_player, film->log(), map, _output_audio_channels, bind(&PlayerVideo::force, _1, FFmpegFileEncoder::pixel_format(format)), true, false));
 }
 
 void

diff --git a/src/lib/ffmpeg_file_encoder.cc b/src/lib/ffmpeg_file_encoder.cc
index 7cc4d26a47031f3ee1160eda413968c365dc20ee..804022f0bfe8ebab3660af4f8d1fd6055e062647 100644 (file)
--- a/src/lib/ffmpeg_file_encoder.cc
+++ b/src/lib/ffmpeg_file_encoder.cc
@@ -26,7 +26,6 @@
 #include "log.h"
 #include "image.h"
 #include "cross.h"
-#include "butler.h"
 #include "compose.hpp"
 #include <iostream>
 
@@ -61,9 +60,10 @@ FFmpegFileEncoder::FFmpegFileEncoder (
        , _audio_frame_rate (audio_frame_rate)
        , _log (log)
 {
+       _pixel_format = pixel_format (format);
+
        switch (format) {
        case EXPORT_FORMAT_PRORES:
-               _pixel_format = AV_PIX_FMT_YUV422P10;
                _sample_format = AV_SAMPLE_FMT_S16;
                _video_codec_name = "prores_ks";
                _audio_codec_name = "pcm_s16le";
@@ -71,7 +71,6 @@ FFmpegFileEncoder::FFmpegFileEncoder (
                av_dict_set (&_video_options, "threads", "auto", 0);
                break;
        case EXPORT_FORMAT_H264:
-               _pixel_format = AV_PIX_FMT_YUV420P;
                _sample_format = AV_SAMPLE_FMT_FLTP;
                _video_codec_name = "libx264";
                _audio_codec_name = "aac";
@@ -125,6 +124,21 @@ FFmpegFileEncoder::FFmpegFileEncoder (
        _pending_audio.reset (new AudioBuffers(channels, 0));
 }
 
+AVPixelFormat
+FFmpegFileEncoder::pixel_format (ExportFormat format)
+{
+       switch (format) {
+       case EXPORT_FORMAT_PRORES:
+               return AV_PIX_FMT_YUV422P10;
+       case EXPORT_FORMAT_H264:
+               return AV_PIX_FMT_YUV420P;
+       default:
+               DCPOMATIC_ASSERT (false);
+       }
+
+       return AV_PIX_FMT_YUV422P10;
+}
+
 void
 FFmpegFileEncoder::setup_video ()
 {

diff --git a/src/lib/ffmpeg_file_encoder.h b/src/lib/ffmpeg_file_encoder.h
index 803125bd3cf677f487a0fd70f9134083fea6e0db..63826e2d5169bbe26298d910132f17955b8d440c 100644 (file)
--- a/src/lib/ffmpeg_file_encoder.h
+++ b/src/lib/ffmpeg_file_encoder.h
@@ -50,6 +50,8 @@ public:
 
        void flush ();
 
+       static AVPixelFormat pixel_format (ExportFormat format);
+
 private:
        void setup_video ();
        void setup_audio ();

diff --git a/src/lib/player_video.cc b/src/lib/player_video.cc
index c9ed6a6e4ffa6ded15d243efa693fe1b751b8341..94432d66e046c289eb4f60f31923f8ac62623abe 100644 (file)
--- a/src/lib/player_video.cc
+++ b/src/lib/player_video.cc
@@ -102,15 +102,27 @@ PlayerVideo::set_text (PositionImage image)
        _text = image;
 }
 
-/** Create an image for this frame.
+shared_ptr<Image>
+PlayerVideo::image (function<AVPixelFormat (AVPixelFormat)> pixel_format, bool aligned, bool fast) const
+{
+       /* XXX: this assumes that image() and prepare() are only ever called with the same parameters */
+
+       boost::mutex::scoped_lock lm (_mutex);
+       if (!_image) {
+               make_image (pixel_format, aligned, fast);
+       }
+       return _image;
+}
+
+/** Create an image for this frame.  A lock must be held on _mutex.
  *  @param pixel_format Function which is called to decide what pixel format the output image should be;
  *  it is passed the pixel format of the input image from the ImageProxy, and should return the desired
  *  output pixel format.  Two functions force and keep_xyz_or_rgb are provided for use here.
  *  @param aligned true if the output image should be aligned to 32-byte boundaries.
  *  @param fast true to be fast at the expense of quality.
  */
-shared_ptr<Image>
-PlayerVideo::image (function<AVPixelFormat (AVPixelFormat)> pixel_format, bool aligned, bool fast) const
+void
+PlayerVideo::make_image (function<AVPixelFormat (AVPixelFormat)> pixel_format, bool aligned, bool fast) const
 {
        pair<shared_ptr<Image>, int> prox = _in->image (_inter_size);
        shared_ptr<Image> im = prox.first;
@@ -148,19 +160,17 @@ PlayerVideo::image (function<AVPixelFormat (AVPixelFormat)> pixel_format, bool a
                yuv_to_rgb = _colour_conversion.get().yuv_to_rgb();
        }
 
-       shared_ptr<Image> out = im->crop_scale_window (
+       _image = im->crop_scale_window (
                total_crop, _inter_size, _out_size, yuv_to_rgb, pixel_format (im->pixel_format()), aligned, fast
                );
 
        if (_text) {
-               out->alpha_blend (Image::ensure_aligned (_text->image), _text->position);
+               _image->alpha_blend (Image::ensure_aligned (_text->image), _text->position);
        }
 
        if (_fade) {
-               out->fade (_fade.get ());
+               _image->fade (_fade.get ());
        }
-
-       return out;
 }
 
 void
@@ -266,9 +276,13 @@ PlayerVideo::keep_xyz_or_rgb (AVPixelFormat p)
 }
 
 void
-PlayerVideo::prepare ()
+PlayerVideo::prepare (function<AVPixelFormat (AVPixelFormat)> pixel_format, bool aligned, bool fast)
 {
        _in->prepare (_inter_size);
+       boost::mutex::scoped_lock lm (_mutex);
+       if (!_image) {
+               make_image (pixel_format, aligned, fast);
+       }
 }
 
 size_t

diff --git a/src/lib/player_video.h b/src/lib/player_video.h
index 8e41e8f2371dc5dff5d504aabf65fad32acca172..8ffe70afb02f0d0d1de0c7d091c232c7876d8a27 100644 (file)
--- a/src/lib/player_video.h
+++ b/src/lib/player_video.h
@@ -62,7 +62,7 @@ public:
 
        void set_text (PositionImage);
 
-       void prepare ();
+       void prepare (boost::function<AVPixelFormat (AVPixelFormat)> pixel_format, bool aligned, bool fast);
        boost::shared_ptr<Image> image (boost::function<AVPixelFormat (AVPixelFormat)> pixel_format, bool aligned, bool fast) const;
 
        static AVPixelFormat force (AVPixelFormat, AVPixelFormat);
@@ -105,6 +105,8 @@ public:
        }
 
 private:
+       void make_image (boost::function<AVPixelFormat (AVPixelFormat)> pixel_format, bool aligned, bool fast) const;
+
        boost::shared_ptr<const ImageProxy> _in;
        Crop _crop;
        boost::optional<double> _fade;
@@ -120,6 +122,9 @@ private:
        boost::weak_ptr<Content> _content;
        /** Video frame that we came from.  Again, this is for reset_metadata() */
        boost::optional<Frame> _video_frame;
+
+       mutable boost::mutex _mutex;
+       mutable boost::shared_ptr<Image> _image;
 };
 
 #endif

diff --git a/src/wx/film_viewer.cc b/src/wx/film_viewer.cc
index 01963372061d8ddef25851b78a375c036b8e2cb3..6d9592b650cae3d0c47aa0db9c7624d0e8ad3397 100644 (file)
--- a/src/wx/film_viewer.cc
+++ b/src/wx/film_viewer.cc
@@ -193,7 +193,7 @@ FilmViewer::recreate_butler ()
                map.set (dcp::RS,     1, 1 / sqrt(2)); // Rs -> Rt
        }
 
-       _butler.reset (new Butler (_player, _film->log(), map, _audio_channels));
+       _butler.reset (new Butler(_player, _film->log(), map, _audio_channels, bind(&PlayerVideo::force, _1, AV_PIX_FMT_RGB24), false, true));
        if (!Config::instance()->sound() && !_audio.isStreamOpen()) {
                _butler->disable_audio ();
        }
@@ -271,10 +271,7 @@ FilmViewer::display_player_video ()
         * image and convert it (from whatever the user has said it is) to RGB.
         */
 
-       _frame = _player_video.first->image (
-               bind (&PlayerVideo::force, _1, AV_PIX_FMT_RGB24),
-               false, true
-               );
+       _frame = _player_video.first->image (bind(&PlayerVideo::force, _1, AV_PIX_FMT_RGB24), false, true);
 
        ImageChanged (_player_video.first);
 

diff --git a/test/butler_test.cc b/test/butler_test.cc
index d1741262b139fd06f004e278255c0a6ee6dd860f..0ee06c1acd694ef084457887de4efef4f96faeb9 100644 (file)
--- a/test/butler_test.cc
+++ b/test/butler_test.cc
@@ -51,7 +51,7 @@ BOOST_AUTO_TEST_CASE (butler_test1)
                map.set (i, i, 1);
        }
 
-       Butler butler (shared_ptr<Player>(new Player(film, film->playlist())), film->log(), map, 6);
+       Butler butler (shared_ptr<Player>(new Player(film, film->playlist())), film->log(), map, 6, bind(&PlayerVideo::force, _1, AV_PIX_FMT_RGB24), false, false);
 
        BOOST_CHECK (butler.get_video().second == DCPTime());
        BOOST_CHECK (butler.get_video().second == DCPTime::from_frames(1, 24));

diff --git a/test/dcp_playback_test.cc b/test/dcp_playback_test.cc
index 17318e0ff8328f178db64806d850e24407092ead..3e3ffe089cfa630251a347a95010078c39815c5d 100644 (file)
--- a/test/dcp_playback_test.cc
+++ b/test/dcp_playback_test.cc
@@ -37,7 +37,16 @@ BOOST_AUTO_TEST_CASE (dcp_playback_test)
        film->examine_and_add_content (content);
        wait_for_jobs ();
 
-       shared_ptr<Butler> butler (new Butler(shared_ptr<Player>(new Player(film, film->playlist())), shared_ptr<Log>(), AudioMapping(6, 6), 6));
+       shared_ptr<Butler> butler (
+               new Butler(
+                       shared_ptr<Player>(new Player(film, film->playlist())),
+                       shared_ptr<Log>(),
+                       AudioMapping(6, 6),
+                       6,
+                       bind(&PlayerVideo::force, _1, AV_PIX_FMT_RGB24),
+                       false,
+                       true)
+               );
        float* audio_buffer = new float[2000*6];
        while (true) {
                pair<shared_ptr<PlayerVideo>, DCPTime> p = butler->get_video ();

diff --git a/test/player_test.cc b/test/player_test.cc
index 0c9b6f21f0a8c81f55b0e13e383ccc5802b3148b..598e7a01a7a40d1acbfe0d7bec61b64dab104685 100644 (file)
--- a/test/player_test.cc
+++ b/test/player_test.cc
@@ -212,7 +212,7 @@ BOOST_AUTO_TEST_CASE (player_seek_test)
        player->set_always_burn_open_subtitles ();
        player->set_play_referenced ();
 
-       shared_ptr<Butler> butler (new Butler (player, film->log(), AudioMapping(), 2));
+       shared_ptr<Butler> butler (new Butler (player, film->log(), AudioMapping(), 2, bind(PlayerVideo::force, _1, AV_PIX_FMT_RGB24), false, true));
        butler->disable_audio();
 
        for (int i = 0; i < 10; ++i) {
@@ -243,7 +243,7 @@ BOOST_AUTO_TEST_CASE (player_seek_test2)
        player->set_always_burn_open_subtitles ();
        player->set_play_referenced ();
 
-       shared_ptr<Butler> butler (new Butler (player, film->log(), AudioMapping(), 2));
+       shared_ptr<Butler> butler (new Butler(player, film->log(), AudioMapping(), 2, bind(PlayerVideo::force, _1, AV_PIX_FMT_RGB24), false, true));
        butler->disable_audio();
 
        butler->seek(DCPTime::from_seconds(5), true);