From 9b73c143ce568bd8694e3a50f2fefc1ee3a03515 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Tue, 22 Jul 2025 00:18:32 +0200 Subject: Replace AVPixelFormat parameter to PlayerVideo::image with a functor. This was removed in df9b4676aba8b941f124b174393988cad21677e1 and surrounding commits, but it turns out we need it after all. --- src/lib/dcp_video.cc | 9 +++++---- src/lib/ffmpeg_file_encoder.cc | 2 +- src/lib/mpeg2_encoder.cc | 2 +- src/lib/player_video.cc | 10 +++++----- src/lib/player_video.h | 4 ++-- src/lib/util.cc | 7 +++++++ src/lib/util.h | 1 + src/tools/dcpomatic_player.cc | 4 ++-- src/wx/gl_video_view.cc | 2 +- src/wx/simple_video_view.cc | 2 +- test/dcp_playback_test.cc | 2 +- test/player_test.cc | 10 +++++----- test/video_level_test.cc | 2 +- test/video_trim_test.cc | 2 +- 14 files changed, 34 insertions(+), 25 deletions(-) diff --git a/src/lib/dcp_video.cc b/src/lib/dcp_video.cc index f07801c03..823325c3e 100644 --- a/src/lib/dcp_video.cc +++ b/src/lib/dcp_video.cc @@ -41,6 +41,7 @@ #include "log.h" #include "player_video.h" #include "rng.h" +#include "util.h" #include #include #include @@ -101,7 +102,7 @@ DCPVideo::convert_to_xyz(shared_ptr frame) shared_ptr xyz; if (frame->colour_conversion()) { - auto image = frame->image(AV_PIX_FMT_RGB48LE, VideoRange::FULL, false); + auto image = frame->image(force(AV_PIX_FMT_RGB48LE), VideoRange::FULL, false); xyz = dcp::rgb_to_xyz( image->data()[0], image->size(), @@ -109,7 +110,7 @@ DCPVideo::convert_to_xyz(shared_ptr frame) frame->colour_conversion().get() ); } else { - auto image = frame->image(AV_PIX_FMT_XYZ12LE, VideoRange::FULL, false); + auto image = frame->image(force(AV_PIX_FMT_RGB48LE), VideoRange::FULL, false); xyz = make_shared(image->data()[0], image->size(), image->stride()[0]); } @@ -119,7 +120,7 @@ DCPVideo::convert_to_xyz(shared_ptr frame) dcp::Size DCPVideo::get_size() const { - auto image = _frame->image(AV_PIX_FMT_RGB48LE, VideoRange::FULL, false); + auto image = _frame->image(force(AV_PIX_FMT_RGB48LE), VideoRange::FULL, false); return image->size(); } @@ -129,7 +130,7 @@ DCPVideo::convert_to_xyz(uint16_t* dst) const { DCPOMATIC_ASSERT(_frame->colour_conversion()); - auto image = _frame->image(AV_PIX_FMT_RGB48LE, VideoRange::FULL, false); + auto image = _frame->image(force(AV_PIX_FMT_RGB48LE), VideoRange::FULL, false); dcp::rgb_to_xyz( image->data()[0], dst, diff --git a/src/lib/ffmpeg_file_encoder.cc b/src/lib/ffmpeg_file_encoder.cc index 3355c2ac8..9df078ad4 100644 --- a/src/lib/ffmpeg_file_encoder.cc +++ b/src/lib/ffmpeg_file_encoder.cc @@ -416,7 +416,7 @@ void FFmpegFileEncoder::video (shared_ptr video, DCPTime time) { /* All our output formats are video range at the moment */ - auto image = video->image(_pixel_format, VideoRange::VIDEO, false); + auto image = video->image(force(_pixel_format), VideoRange::VIDEO, false); auto frame = av_frame_alloc (); DCPOMATIC_ASSERT (frame); diff --git a/src/lib/mpeg2_encoder.cc b/src/lib/mpeg2_encoder.cc index 73f3d57e7..38388431d 100644 --- a/src/lib/mpeg2_encoder.cc +++ b/src/lib/mpeg2_encoder.cc @@ -43,7 +43,7 @@ MPEG2Encoder::encode(shared_ptr pv, dcpomatic::DCPTime time) { VideoEncoder::encode(pv, time); - auto image = pv->image(AV_PIX_FMT_YUV420P, VideoRange::VIDEO, false); + auto image = pv->image(force(AV_PIX_FMT_YUV420P), VideoRange::VIDEO, false); dcp::FFmpegImage ffmpeg_image(time.get() * _film->video_frame_rate() / dcpomatic::DCPTime::HZ); diff --git a/src/lib/player_video.cc b/src/lib/player_video.cc index b304d60df..63811fb69 100644 --- a/src/lib/player_video.cc +++ b/src/lib/player_video.cc @@ -116,7 +116,7 @@ PlayerVideo::set_text(PositionImage image) shared_ptr -PlayerVideo::image(AVPixelFormat pixel_format, VideoRange video_range, bool fast) const +PlayerVideo::image(function pixel_format, VideoRange video_range, bool fast) const { /* XXX: this assumes that image() and prepare() are only ever called with the same parameters (except crop, inter size, out size, fade) */ @@ -136,11 +136,11 @@ PlayerVideo::raw_image() const /** Create an image for this frame. A lock must be held on _mutex. - * @param pixel_format Output image pixel format. + * @param pixel_format Functor returning output image pixel format for a given input pixel format. * @param fast true to be fast at the expense of quality. */ void -PlayerVideo::make_image(AVPixelFormat pixel_format, VideoRange video_range, bool fast) const +PlayerVideo::make_image(function pixel_format, VideoRange video_range, bool fast) const { _image_crop = _crop; _image_inter_size = _inter_size; @@ -183,7 +183,7 @@ PlayerVideo::make_image(AVPixelFormat pixel_format, VideoRange video_range, bool } _image = prox.image->crop_scale_window( - total_crop, _inter_size, _out_size, yuv_to_rgb, _video_range, pixel_format, video_range, Image::Alignment::COMPACT, fast + total_crop, _inter_size, _out_size, yuv_to_rgb, _video_range, pixel_format(prox.image->pixel_format()), video_range, Image::Alignment::COMPACT, fast ); if (_text) { @@ -301,7 +301,7 @@ PlayerVideo::prepare(AVPixelFormat pixel_format, VideoRange video_range, Image:: _in->prepare(alignment, _inter_size); boost::mutex::scoped_lock lm(_mutex); if (!_image && !proxy_only) { - make_image(pixel_format, video_range, fast); + make_image(force(pixel_format), video_range, fast); } } diff --git a/src/lib/player_video.h b/src/lib/player_video.h index df0ef6d15..48c927f85 100644 --- a/src/lib/player_video.h +++ b/src/lib/player_video.h @@ -76,7 +76,7 @@ public: } void prepare(AVPixelFormat pixel_format, VideoRange video_range, Image::Alignment alignment, bool fast, bool proxy_only); - std::shared_ptr image(AVPixelFormat pixel_format, VideoRange video_range, bool fast) const; + std::shared_ptr image(std::function pixel_format, VideoRange video_range, bool fast) const; std::shared_ptr raw_image() const; void add_metadata(xmlpp::Element* element) const; @@ -128,7 +128,7 @@ public: } private: - void make_image(AVPixelFormat pixel_format, VideoRange video_range, bool fast) const; + void make_image(std::function pixel_format, VideoRange video_range, bool fast) const; std::shared_ptr _in; Crop _crop; diff --git a/src/lib/util.cc b/src/lib/util.cc index 12ea8c0eb..fb9c1bb2d 100644 --- a/src/lib/util.cc +++ b/src/lib/util.cc @@ -1171,3 +1171,10 @@ paths_exist(vector const& paths) return std::all_of(paths.begin(), paths.end(), [](boost::filesystem::path const& path) { return dcp::filesystem::exists(path); }); } + +std::function +force(AVPixelFormat format) +{ + return [format](AVPixelFormat) { return format; }; +} + diff --git a/src/lib/util.h b/src/lib/util.h index 79ac87db1..846059e1a 100644 --- a/src/lib/util.h +++ b/src/lib/util.h @@ -101,6 +101,7 @@ extern void setup_grok_library_path(); extern std::string join_strings(std::vector const& in, std::string const& separator = " "); extern std::string rfc_2822_date(time_t time); bool paths_exist(std::vector const& paths); +std::function force(AVPixelFormat format); template diff --git a/src/tools/dcpomatic_player.cc b/src/tools/dcpomatic_player.cc index 2bf560811..344b47711 100644 --- a/src/tools/dcpomatic_player.cc +++ b/src/tools/dcpomatic_player.cc @@ -827,10 +827,10 @@ private: player->Video.connect ([path, &done, this](shared_ptr video, DCPTime) { auto ext = boost::algorithm::to_lower_copy(path.extension().string()); if (ext == ".png") { - auto image = video->image(AV_PIX_FMT_RGBA, VideoRange::FULL, false); + auto image = video->image(force(AV_PIX_FMT_RGBA), VideoRange::FULL, false); image_as_png(image).write(path); } else if (ext == ".jpg" || ext == ".jpeg") { - auto image = video->image(AV_PIX_FMT_RGB24, VideoRange::FULL, false); + auto image = video->image(force(AV_PIX_FMT_RGB24), VideoRange::FULL, false); image_as_jpeg(image, 80).write(path); } else { error_dialog(this, wxString::Format(_("Unrecognised file extension %s (use .jpg, .jpeg or .png)"), std_to_wx(ext))); diff --git a/src/wx/gl_video_view.cc b/src/wx/gl_video_view.cc index d1107f94f..cc4315019 100644 --- a/src/wx/gl_video_view.cc +++ b/src/wx/gl_video_view.cc @@ -645,7 +645,7 @@ GLVideoView::set_image(shared_ptr pv) video = pv->raw_image(); break; case Optimisation::NONE: - video = pv->image(AV_PIX_FMT_RGB24, VideoRange::FULL, true); + video = pv->image(force(AV_PIX_FMT_RGB24), VideoRange::FULL, true); break; } diff --git a/src/wx/simple_video_view.cc b/src/wx/simple_video_view.cc index ff16b6146..0fd162196 100644 --- a/src/wx/simple_video_view.cc +++ b/src/wx/simple_video_view.cc @@ -252,7 +252,7 @@ SimpleVideoView::update () _state_timer.set ("get image"); auto const pv = player_video(); - _image = pv.first->image(AV_PIX_FMT_RGB24, VideoRange::FULL, true); + _image = pv.first->image(force(AV_PIX_FMT_RGB24), VideoRange::FULL, true); if (pv.first->colour_conversion() && pv.first->colour_conversion()->about_equal(dcp::ColourConversion::rec2020_to_xyz(), 1e-6)) { _image = Image::ensure_alignment(_rec2020_filter_graph.get(_image->size(), _image->pixel_format())->process(_image).front(), Image::Alignment::COMPACT); } diff --git a/test/dcp_playback_test.cc b/test/dcp_playback_test.cc index 7b2280883..752ae56a2 100644 --- a/test/dcp_playback_test.cc +++ b/test/dcp_playback_test.cc @@ -60,6 +60,6 @@ BOOST_AUTO_TEST_CASE (dcp_playback_test) } /* assuming DCP is 24fps/48kHz */ butler->get_audio (Butler::Behaviour::BLOCKING, audio_buffer.data(), 2000); - p.first->image(AV_PIX_FMT_RGB24, VideoRange::FULL, true); + p.first->image(force(AV_PIX_FMT_RGB24), VideoRange::FULL, true); } } diff --git a/test/player_test.cc b/test/player_test.cc index acbd019dc..7fc21ac36 100644 --- a/test/player_test.cc +++ b/test/player_test.cc @@ -224,7 +224,7 @@ BOOST_AUTO_TEST_CASE (player_seek_test) butler->seek (t, true); auto video = butler->get_video(Butler::Behaviour::BLOCKING, 0); BOOST_CHECK_EQUAL(video.second.get(), t.get()); - write_image(video.first->image(AV_PIX_FMT_RGB24, VideoRange::FULL, true), fmt::format("build/test/player_seek_test_{}.png", i)); + write_image(video.first->image(force(AV_PIX_FMT_RGB24), VideoRange::FULL, true), fmt::format("build/test/player_seek_test_{}.png", i)); /* This 14.08 is empirically chosen (hopefully) to accept changes in rendering between the reference and a test machine (17.10 and 16.04 seem to anti-alias a little differently) but to reject gross errors e.g. missing fonts or missing text altogether. @@ -260,7 +260,7 @@ BOOST_AUTO_TEST_CASE (player_seek_test2) auto video = butler->get_video(Butler::Behaviour::BLOCKING, 0); BOOST_CHECK_EQUAL(video.second.get(), t.get()); write_image( - video.first->image(AV_PIX_FMT_RGB24, VideoRange::FULL, true), fmt::format("build/test/player_seek_test2_{}.png", i) + video.first->image(force(AV_PIX_FMT_RGB24), VideoRange::FULL, true), fmt::format("build/test/player_seek_test2_{}.png", i) ); check_image(TestPaths::private_data() / fmt::format("player_seek_test2_{}.png", i), fmt::format("build/test/player_seek_test2_{}.png", i), 14.08); } @@ -618,7 +618,7 @@ BOOST_AUTO_TEST_CASE(two_d_in_three_d_duplicates) } last_time = time; - auto image = video->image(AV_PIX_FMT_RGB24, VideoRange::FULL, false); + auto image = video->image(force(AV_PIX_FMT_RGB24), VideoRange::FULL, false); auto const size = image->size(); for (int y = 0; y < size.height; ++y) { uint8_t* line = image->data()[0] + y * image->stride()[0]; @@ -668,7 +668,7 @@ BOOST_AUTO_TEST_CASE(three_d_in_two_d_chooses_left) BOOST_CHECK(!last_time || time == *last_time + DCPTime::from_frames(1, 24)); last_time = time; - auto image = video->image(AV_PIX_FMT_RGB24, VideoRange::FULL, false); + auto image = video->image(force(AV_PIX_FMT_RGB24), VideoRange::FULL, false); auto const size = image->size(); for (int y = 0; y < size.height; ++y) { uint8_t* line = image->data()[0] + y * image->stride()[0]; @@ -745,7 +745,7 @@ BOOST_AUTO_TEST_CASE(frames_are_copied_correctly_for_low_frame_rates) /* Check that only red frames come out - previously there would be some black ones mixed in */ for (auto i = 0; i < 24; ++i) { auto frame = butler.get_video(Butler::Behaviour::BLOCKING); - auto image = frame.first->image(AV_PIX_FMT_RGB24, VideoRange::FULL, false); + auto image = frame.first->image(force(AV_PIX_FMT_RGB24), VideoRange::FULL, false); for (int y = 0; y < image->size().height; ++y) { uint8_t const* p = image->data()[0] + image->stride()[0] * y; for (int x = 0; x < image->size().width; ++x) { diff --git a/test/video_level_test.cc b/test/video_level_test.cc index 82038e33b..802c8c8fa 100644 --- a/test/video_level_test.cc +++ b/test/video_level_test.cc @@ -127,7 +127,7 @@ BOOST_AUTO_TEST_CASE (ffmpeg_image_video_range_expanded) BOOST_REQUIRE (!player->pass()); } - auto image = player_video->image(AV_PIX_FMT_RGB24, VideoRange::FULL, false); + auto image = player_video->image(force(AV_PIX_FMT_RGB24), VideoRange::FULL, false); for (int y = 0; y < size.height; ++y) { uint8_t* p = image->data()[0] + y * image->stride()[0]; diff --git a/test/video_trim_test.cc b/test/video_trim_test.cc index b3d3bbb84..8ec96dde5 100644 --- a/test/video_trim_test.cc +++ b/test/video_trim_test.cc @@ -51,7 +51,7 @@ BOOST_AUTO_TEST_CASE(video_trim_test) BOOST_REQUIRE(!player->pass()); } - image_as_png(first_video->image(AV_PIX_FMT_RGB24, VideoRange::FULL, true)).write("build/test/video_trim_test.png"); + image_as_png(first_video->image(force(AV_PIX_FMT_RGB24), VideoRange::FULL, true)).write("build/test/video_trim_test.png"); check_image("test/data/video_trim_test.png", "build/test/video_trim_test.png"); } -- cgit v1.2.3