From 67ff55886b1ee86d99c2ea27d10c73b85b0504b7 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Thu, 23 Sep 2021 00:09:47 +0200 Subject: [PATCH] Various alignment tidying/fixups. --- src/lib/dcp_video.cc | 2 +- src/lib/ffmpeg_file_encoder.cc | 1 - src/lib/player_video.cc | 13 ++++++------- src/lib/player_video.h | 4 ++-- src/lib/util.cc | 2 +- src/wx/film_viewer.cc | 6 ++++-- src/wx/gl_video_view.cc | 2 +- src/wx/simple_video_view.cc | 7 +++---- src/wx/simple_video_view.h | 4 ---- test/dcp_playback_test.cc | 2 +- test/player_test.cc | 6 +++--- test/video_level_test.cc | 2 +- 12 files changed, 23 insertions(+), 28 deletions(-) diff --git a/src/lib/dcp_video.cc b/src/lib/dcp_video.cc index 9daeb45c8..3a85a5ac6 100644 --- a/src/lib/dcp_video.cc +++ b/src/lib/dcp_video.cc @@ -105,7 +105,7 @@ DCPVideo::convert_to_xyz (shared_ptr frame, dcp::NoteHandler { shared_ptr xyz; - auto image = frame->image (bind(&PlayerVideo::keep_xyz_or_rgb, _1), VideoRange::FULL, Image::Alignment::PADDED, false); + auto image = frame->image (bind(&PlayerVideo::keep_xyz_or_rgb, _1), VideoRange::FULL, false); if (frame->colour_conversion()) { xyz = dcp::rgb_to_xyz ( image->data()[0], diff --git a/src/lib/ffmpeg_file_encoder.cc b/src/lib/ffmpeg_file_encoder.cc index ef02f30c8..705557f79 100644 --- a/src/lib/ffmpeg_file_encoder.cc +++ b/src/lib/ffmpeg_file_encoder.cc @@ -402,7 +402,6 @@ FFmpegFileEncoder::video (shared_ptr video, DCPTime time) auto image = video->image ( bind (&PlayerVideo::force, _1, _pixel_format), VideoRange::VIDEO, - Image::Alignment::PADDED, false ); diff --git a/src/lib/player_video.cc b/src/lib/player_video.cc index 2d60efe10..7c36af31b 100644 --- a/src/lib/player_video.cc +++ b/src/lib/player_video.cc @@ -110,13 +110,13 @@ PlayerVideo::set_text (PositionImage image) } shared_ptr -PlayerVideo::image (function pixel_format, VideoRange video_range, Image::Alignment alignment, 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) */ boost::mutex::scoped_lock lm (_mutex); if (!_image || _crop != _image_crop || _inter_size != _image_inter_size || _out_size != _image_out_size || _fade != _image_fade) { - make_image (pixel_format, video_range, alignment, fast); + make_image (pixel_format, video_range, fast); } return _image; } @@ -133,11 +133,10 @@ PlayerVideo::raw_image () const * @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 alignment PADDED if the output image should be aligned to 32-byte boundaries, otherwise COMPACT. * @param fast true to be fast at the expense of quality. */ void -PlayerVideo::make_image (function pixel_format, VideoRange video_range, Image::Alignment alignment, bool fast) const +PlayerVideo::make_image (function pixel_format, VideoRange video_range, bool fast) const { _image_crop = _crop; _image_inter_size = _inter_size; @@ -180,11 +179,11 @@ PlayerVideo::make_image (function pixel_format, V } _image = prox.image->crop_scale_window ( - total_crop, _inter_size, _out_size, yuv_to_rgb, _video_range, pixel_format (prox.image->pixel_format()), video_range, alignment, 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) { - _image->alpha_blend (Image::ensure_alignment(_text->image, Image::Alignment::PADDED), _text->position); + _image->alpha_blend (_text->image, _text->position); } if (_fade) { @@ -303,7 +302,7 @@ PlayerVideo::prepare (function pixel_format, Vide _in->prepare (alignment, _inter_size); boost::mutex::scoped_lock lm (_mutex); if (!_image && !proxy_only) { - make_image (pixel_format, video_range, alignment, fast); + make_image (pixel_format, video_range, fast); } } diff --git a/src/lib/player_video.h b/src/lib/player_video.h index d24620c7e..237d2e3fe 100644 --- a/src/lib/player_video.h +++ b/src/lib/player_video.h @@ -76,7 +76,7 @@ public: } void prepare (std::function pixel_format, VideoRange video_range, Image::Alignment alignment, bool fast, bool proxy_only); - std::shared_ptr image (std::function pixel_format, VideoRange video_range, Image::Alignment alignment, bool fast) const; + std::shared_ptr image (std::function pixel_format, VideoRange video_range, bool fast) const; std::shared_ptr raw_image () const; static AVPixelFormat force (AVPixelFormat, AVPixelFormat); @@ -127,7 +127,7 @@ public: } private: - void make_image (std::function pixel_format, VideoRange video_range, Image::Alignment alignment, 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 981cfa521..d3af74376 100644 --- a/src/lib/util.cc +++ b/src/lib/util.cc @@ -957,7 +957,7 @@ emit_subtitle_image (ContentTimePeriod period, dcp::SubtitleImage sub, dcp::Size { /* XXX: this is rather inefficient; decoding the image just to get its size */ FFmpegImageProxy proxy (sub.png_image()); - auto image = proxy.image(Image::Alignment::COMPACT).image; + auto image = proxy.image(Image::Alignment::PADDED).image; /* set up rect with height and width */ dcpomatic::Rect rect(0, 0, image->size().width / double(size.width), image->size().height / double(size.height)); diff --git a/src/wx/film_viewer.cc b/src/wx/film_viewer.cc index 5609ebf86..0131aa294 100644 --- a/src/wx/film_viewer.cc +++ b/src/wx/film_viewer.cc @@ -208,6 +208,8 @@ FilmViewer::recreate_butler () return; } + auto const j2k_gl_optimised = dynamic_pointer_cast(_video_view) && _optimise_for_j2k; + _butler = std::make_shared( _film, _player, @@ -215,9 +217,9 @@ FilmViewer::recreate_butler () _audio_channels, bind(&PlayerVideo::force, _1, AV_PIX_FMT_RGB24), VideoRange::FULL, - _optimise_for_j2k ? Image::Alignment::COMPACT : Image::Alignment::PADDED, + j2k_gl_optimised ? Image::Alignment::COMPACT : Image::Alignment::PADDED, true, - dynamic_pointer_cast(_video_view) && _optimise_for_j2k + j2k_gl_optimised ); if (!Config::instance()->sound() && !_audio.isStreamOpen()) { diff --git a/src/wx/gl_video_view.cc b/src/wx/gl_video_view.cc index 046465864..04e0bac46 100644 --- a/src/wx/gl_video_view.cc +++ b/src/wx/gl_video_view.cc @@ -497,7 +497,7 @@ GLVideoView::draw (Position, dcp::Size) void GLVideoView::set_image (shared_ptr pv) { - shared_ptr video = _optimise_for_j2k ? pv->raw_image() : pv->image(bind(&PlayerVideo::force, _1, AV_PIX_FMT_RGB24), VideoRange::FULL, Image::Alignment::COMPACT, true); + shared_ptr video = _optimise_for_j2k ? pv->raw_image() : pv->image(bind(&PlayerVideo::force, _1, AV_PIX_FMT_RGB24), VideoRange::FULL, true); /* Only the player's black frames should be aligned at this stage, so this should * almost always have no work to do. diff --git a/src/wx/simple_video_view.cc b/src/wx/simple_video_view.cc index e54c8390e..1ac56bbfe 100644 --- a/src/wx/simple_video_view.cc +++ b/src/wx/simple_video_view.cc @@ -72,6 +72,7 @@ SimpleVideoView::paint () if (!_image) { dc.Clear (); } else { + DCPOMATIC_ASSERT (_image->alignment() == Image::Alignment::COMPACT); out_size = _image->size(); wxImage frame (out_size.width, out_size.height, _image->data()[0], true); wxBitmap frame_bitmap (frame); @@ -188,7 +189,7 @@ void SimpleVideoView::update () { if (!player_video().first) { - set_image (shared_ptr()); + _image.reset (); refresh_panel (); return; } @@ -221,9 +222,7 @@ SimpleVideoView::update () _state_timer.set ("get image"); - set_image ( - player_video().first->image(bind(&PlayerVideo::force, _1, AV_PIX_FMT_RGB24), VideoRange::FULL, Image::Alignment::COMPACT, true) - ); + _image = player_video().first->image(bind(&PlayerVideo::force, _1, AV_PIX_FMT_RGB24), VideoRange::FULL, true); _state_timer.set ("ImageChanged"); _viewer->image_changed (player_video().first); diff --git a/src/wx/simple_video_view.h b/src/wx/simple_video_view.h index 26d1299b1..cbb162023 100644 --- a/src/wx/simple_video_view.h +++ b/src/wx/simple_video_view.h @@ -42,10 +42,6 @@ public: NextFrameResult display_next_frame (bool non_blocking) override; private: - void set_image (std::shared_ptr image) { - _image = image; - } - void refresh_panel (); void paint (); void timer (); diff --git a/test/dcp_playback_test.cc b/test/dcp_playback_test.cc index 28368dc34..6c2b0447c 100644 --- a/test/dcp_playback_test.cc +++ b/test/dcp_playback_test.cc @@ -63,7 +63,7 @@ BOOST_AUTO_TEST_CASE (dcp_playback_test) } /* assuming DCP is 24fps/48kHz */ butler->get_audio (audio_buffer, 2000); - p.first->image(bind(&PlayerVideo::force, _1, AV_PIX_FMT_RGB24), VideoRange::FULL, Image::Alignment::COMPACT, true); + p.first->image(bind(&PlayerVideo::force, _1, AV_PIX_FMT_RGB24), VideoRange::FULL, true); } delete[] audio_buffer; } diff --git a/test/player_test.cc b/test/player_test.cc index 2caa34753..60bfee1a4 100644 --- a/test/player_test.cc +++ b/test/player_test.cc @@ -241,7 +241,7 @@ BOOST_AUTO_TEST_CASE (player_seek_test) butler->seek (t, true); auto video = butler->get_video(true, 0); BOOST_CHECK_EQUAL(video.second.get(), t.get()); - write_image(video.first->image(bind(PlayerVideo::force, _1, AV_PIX_FMT_RGB24), VideoRange::FULL, Image::Alignment::COMPACT, true), String::compose("build/test/player_seek_test_%1.png", i)); + write_image(video.first->image(bind(PlayerVideo::force, _1, AV_PIX_FMT_RGB24), VideoRange::FULL, true), String::compose("build/test/player_seek_test_%1.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. @@ -265,7 +265,7 @@ BOOST_AUTO_TEST_CASE (player_seek_test2) player->set_always_burn_open_subtitles (); player->set_play_referenced (); - auto butler = std::make_shared(film, player, AudioMapping(), 2, bind(PlayerVideo::force, _1, AV_PIX_FMT_RGB24), VideoRange::FULL, Image::Alignment::COMPACT, true, false); + auto butler = std::make_shared(film, player, AudioMapping(), 2, bind(PlayerVideo::force, _1, AV_PIX_FMT_RGB24), VideoRange::FULL, Image::Alignment::PADDED, true, false); butler->disable_audio(); butler->seek(DCPTime::from_seconds(5), true); @@ -276,7 +276,7 @@ BOOST_AUTO_TEST_CASE (player_seek_test2) auto video = butler->get_video(true, 0); BOOST_CHECK_EQUAL(video.second.get(), t.get()); write_image( - video.first->image(bind(PlayerVideo::force, _1, AV_PIX_FMT_RGB24), VideoRange::FULL, Image::Alignment::COMPACT, true), String::compose("build/test/player_seek_test2_%1.png", i) + video.first->image(bind(PlayerVideo::force, _1, AV_PIX_FMT_RGB24), VideoRange::FULL, true), String::compose("build/test/player_seek_test2_%1.png", i) ); check_image(TestPaths::private_data() / String::compose("player_seek_test2_%1.png", i), String::compose("build/test/player_seek_test2_%1.png", i), 14.08); } diff --git a/test/video_level_test.cc b/test/video_level_test.cc index 54513464c..ada9b602a 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 ([](AVPixelFormat f) { return f; }, VideoRange::FULL, Image::Alignment::PADDED, false); + auto image = player_video->image ([](AVPixelFormat f) { return f; }, VideoRange::FULL, false); for (int y = 0; y < size.height; ++y) { uint8_t* p = image->data()[0] + y * image->stride()[0]; -- 2.30.2