From: Carl Hetherington Date: Tue, 6 Nov 2018 23:06:21 +0000 (+0000) Subject: Do image crop/scale/window in the butler prepare threads. X-Git-Tag: v2.13.67~5 X-Git-Url: https://main.carlh.net/gitweb/?p=dcpomatic.git;a=commitdiff_plain;h=f41310384889e4cfb6e709d098b316e212d8bf22;ds=sidebyside Do image crop/scale/window in the butler prepare threads. --- 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 + Copyright (C) 2016-2018 Carl Hetherington 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, shared_ptr 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, + shared_ptr log, + AudioMapping audio_mapping, + int audio_channels, + function 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, shared_ptr 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 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, boost::shared_ptr log, AudioMapping map, int audio_channels); + Butler ( + boost::shared_ptr player, + boost::shared_ptr log, + AudioMapping map, + int audio_channels, + boost::function pixel_format, + bool aligned, + bool fast + ); + ~Butler (); void seek (DCPTime position, bool accurate); @@ -99,6 +108,10 @@ private: bool _disable_audio; + boost::function _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 @@ -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 +PlayerVideo::image (function 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 -PlayerVideo::image (function pixel_format, bool aligned, bool fast) const +void +PlayerVideo::make_image (function pixel_format, bool aligned, bool fast) const { pair, int> prox = _in->image (_inter_size); shared_ptr im = prox.first; @@ -148,19 +160,17 @@ PlayerVideo::image (function pixel_format, bool a yuv_to_rgb = _colour_conversion.get().yuv_to_rgb(); } - shared_ptr 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 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 pixel_format, bool aligned, bool fast); boost::shared_ptr image (boost::function pixel_format, bool aligned, bool fast) const; static AVPixelFormat force (AVPixelFormat, AVPixelFormat); @@ -105,6 +105,8 @@ public: } private: + void make_image (boost::function pixel_format, bool aligned, bool fast) const; + boost::shared_ptr _in; Crop _crop; boost::optional _fade; @@ -120,6 +122,9 @@ private: boost::weak_ptr _content; /** Video frame that we came from. Again, this is for reset_metadata() */ boost::optional _video_frame; + + mutable boost::mutex _mutex; + mutable boost::shared_ptr _image; }; #endif diff --git a/src/wx/film_viewer.cc b/src/wx/film_viewer.cc index 019633720..6d9592b65 100644 --- 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 d1741262b..0ee06c1ac 100644 --- 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(new Player(film, film->playlist())), film->log(), map, 6); + Butler butler (shared_ptr(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 17318e0ff..3e3ffe089 100644 --- 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 (new Butler(shared_ptr(new Player(film, film->playlist())), shared_ptr(), AudioMapping(6, 6), 6)); + shared_ptr butler ( + new Butler( + shared_ptr(new Player(film, film->playlist())), + shared_ptr(), + AudioMapping(6, 6), + 6, + bind(&PlayerVideo::force, _1, AV_PIX_FMT_RGB24), + false, + true) + ); float* audio_buffer = new float[2000*6]; while (true) { pair, DCPTime> p = butler->get_video (); diff --git a/test/player_test.cc b/test/player_test.cc index 0c9b6f21f..598e7a01a 100644 --- 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 (new Butler (player, film->log(), AudioMapping(), 2)); + shared_ptr 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 (new Butler (player, film->log(), AudioMapping(), 2)); + shared_ptr 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);