From 17f8f22b07bd32d57edada0acde6fd9fffe3baf3 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Mon, 4 Mar 2024 00:41:01 +0100 Subject: More correctly calculate bitmap subtitle scaling (#2670). This was partially fixed before in 6ac468554c7fea0dfaefde85fb6cdd0fceaf5cad The last try accounted for cropping, but not for cases where the source video (after crop) does not precisely fit the DCP container. In those cases the x scale for the subtitles could be different to the y scale, squashing or stretching them. --- src/lib/bitmap_text.h | 8 ++++++-- src/lib/content_text.h | 4 ++-- src/lib/ffmpeg_decoder.cc | 38 ++++++++++++++++++++------------------ src/lib/player.cc | 12 +++++++----- src/lib/text_decoder.cc | 4 ++-- src/lib/text_decoder.h | 2 +- 6 files changed, 38 insertions(+), 30 deletions(-) diff --git a/src/lib/bitmap_text.h b/src/lib/bitmap_text.h index 04cb61e3e..50f3f507a 100644 --- a/src/lib/bitmap_text.h +++ b/src/lib/bitmap_text.h @@ -23,6 +23,7 @@ #include "rect.h" +#include #include @@ -31,11 +32,14 @@ class Image; class BitmapText { public: - BitmapText(std::shared_ptr image_, dcpomatic::Rect rectangle_) - : image(image_) + BitmapText(dcp::Size parent_size_, std::shared_ptr image_, dcpomatic::Rect rectangle_) + : parent_size(parent_size_) + , image(image_) , rectangle(rectangle_) {} + /** Size of the image that this subtitle has come from, with any crop already applied */ + dcp::Size parent_size; std::shared_ptr image; /** Area that the subtitle covers on its corresponding video, expressed in * proportions of the image size; e.g. rectangle.x = 0.5 would mean that diff --git a/src/lib/content_text.h b/src/lib/content_text.h index 8a7f007ef..d04383c8b 100644 --- a/src/lib/content_text.h +++ b/src/lib/content_text.h @@ -57,9 +57,9 @@ public: : ContentText(from) {} - ContentBitmapText(dcpomatic::ContentTime from, std::shared_ptr image, dcpomatic::Rect rect) + ContentBitmapText(dcpomatic::ContentTime from, dcp::Size parent_size, std::shared_ptr image, dcpomatic::Rect rect) : ContentText(from) - , subs{ {image, rect} } + , subs{ { parent_size, image, rect } } {} /* Our texts, with their rectangles unmodified by any offsets or scales that the content specifies */ diff --git a/src/lib/ffmpeg_decoder.cc b/src/lib/ffmpeg_decoder.cc index 7f7a07863..ab3f0b5be 100644 --- a/src/lib/ffmpeg_decoder.cc +++ b/src/lib/ffmpeg_decoder.cc @@ -770,39 +770,41 @@ FFmpegDecoder::process_bitmap_subtitle (AVSubtitleRect const * rect) out_p += image->stride()[0]; } - int target_width = subtitle_codec_context()->width; - if (target_width == 0 && video_codec_context()) { - /* subtitle_codec_context()->width == 0 has been seen in the wild but I don't - know if it's supposed to mean something from FFmpeg's point of view. - */ - target_width = video_codec_context()->width; + optional video_size; + if (_ffmpeg_content->video) { + video_size = _ffmpeg_content->video->size(); } - int target_height = subtitle_codec_context()->height; - if (target_height == 0 && video_codec_context()) { - target_height = video_codec_context()->height; + + dcp::Size target_size = { subtitle_codec_context()->width, subtitle_codec_context()->height }; + if (target_size.width == 0 || target_size.height == 0 || (video_size && *video_size == target_size)) { + /* Either the subtitle codec has no specified size, or it's the same as the video. + * In either case we'll use the target size once it has been cropped etc. as we + * assume that whatever happens to the video should also be done to the subtitles. + */ + if (auto s = ffmpeg_content()->video->scaled_size(film()->frame_size())) { + target_size = *s; + } } int x_offset = 0; int y_offset = 0; if (_ffmpeg_content->video && _ffmpeg_content->video->use()) { auto const crop = _ffmpeg_content->video->actual_crop(); - target_width -= crop.left + crop.right; - target_height -= crop.top + crop.bottom; x_offset = -crop.left; y_offset = -crop.top; } - DCPOMATIC_ASSERT(target_width > 0); - DCPOMATIC_ASSERT(target_height > 0); + DCPOMATIC_ASSERT(target_size.width > 0); + DCPOMATIC_ASSERT(target_size.height > 0); dcpomatic::Rect const scaled_rect ( - static_cast(rect->x + x_offset) / target_width, - static_cast(rect->y + y_offset) / target_height, - static_cast(rect->w) / target_width, - static_cast(rect->h) / target_height + static_cast(rect->x + x_offset) / target_size.width, + static_cast(rect->y + y_offset) / target_size.height, + static_cast(rect->w) / target_size.width, + static_cast(rect->h) / target_size.height ); - return { image, scaled_rect }; + return { target_size, image, scaled_rect }; } diff --git a/src/lib/player.cc b/src/lib/player.cc index c03cb97a5..1b5eca65c 100644 --- a/src/lib/player.cc +++ b/src/lib/player.cc @@ -1233,7 +1233,8 @@ Player::bitmap_text_start (weak_ptr weak_piece, weak_ptr weak_piece, weak_ptrframe_size(), {}); + + int const width = sub.rectangle.width * inter_size.width; + int const height = sub.rectangle.height * inter_size.height; if (width == 0 || height == 0) { return; } dcp::Size scaled_size (width, height); - ps.bitmap.push_back (BitmapText(image->scale(scaled_size, dcp::YUVToRGB::REC601, image->pixel_format(), Image::Alignment::PADDED, _fast), sub.rectangle)); + ps.bitmap.push_back(BitmapText(sub.parent_size, image->scale(scaled_size, dcp::YUVToRGB::REC601, image->pixel_format(), Image::Alignment::PADDED, _fast), sub.rectangle)); } DCPTime from(content_time_to_dcp(piece, subtitle.from())); diff --git a/src/lib/text_decoder.cc b/src/lib/text_decoder.cc index 75fa33605..894145c35 100644 --- a/src/lib/text_decoder.cc +++ b/src/lib/text_decoder.cc @@ -352,9 +352,9 @@ TextDecoder::emit_plain (ContentTimePeriod period, sub::Subtitle const& subtitle * of the video frame) */ void -TextDecoder::emit_bitmap (ContentTimePeriod period, shared_ptr image, dcpomatic::Rect rect) +TextDecoder::emit_bitmap(ContentTimePeriod period, dcp::Size parent_size, shared_ptr image, dcpomatic::Rect rect) { - emit_bitmap_start ({ period.from, image, rect }); + emit_bitmap_start ({ period.from, parent_size, image, rect }); emit_stop (period.to); } diff --git a/src/lib/text_decoder.h b/src/lib/text_decoder.h index 1a7632fd8..47c1be399 100644 --- a/src/lib/text_decoder.h +++ b/src/lib/text_decoder.h @@ -51,7 +51,7 @@ public: } void emit_bitmap_start (ContentBitmapText const& bitmap); - void emit_bitmap (dcpomatic::ContentTimePeriod period, std::shared_ptr image, dcpomatic::Rect rect); + void emit_bitmap(dcpomatic::ContentTimePeriod period, dcp::Size parent_size, std::shared_ptr image, dcpomatic::Rect rect); void emit_plain_start(dcpomatic::ContentTime from, std::vector s, dcp::SubtitleStandard valign_standard); void emit_plain_start (dcpomatic::ContentTime from, sub::Subtitle const & subtitle); void emit_plain(dcpomatic::ContentTimePeriod period, std::vector s, dcp::SubtitleStandard valign_standard); -- cgit v1.2.3