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

7 files changed, 83 insertions, 18 deletions

diff --git a/src/lib/butler.cc b/src/lib/butler.cc
index 018334c7e..409963016 100644
--- 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 fb133d108..b1debfca2 100644
--- 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 eb3b0c28a..bb696055c 100644
--- 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 7cc4d26a4..804022f0b 100644
--- 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 803125bd3..63826e2d5 100644
--- 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 c9ed6a6e4..94432d66e 100644
--- 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 8e41e8f23..8ffe70afb 100644
--- 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