From 9726a58f44d52d235b027225ddd68c6acf83c733 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Wed, 6 Jul 2022 12:25:15 +0200 Subject: [PATCH] Handle vertical alignment of subs correctly wrt the difference between Interop and SMPTE. --- src/lib/dcp_decoder.cc | 6 ++-- src/lib/dcp_subtitle_decoder.cc | 8 ++++- src/lib/dcp_subtitle_decoder.h | 2 ++ src/lib/render_text.cc | 52 +++++++++++++++++++++------------ src/lib/string_text.h | 17 ++++++++++- src/lib/text_decoder.cc | 20 +++++++++---- src/lib/text_decoder.h | 4 +-- src/lib/util.cc | 2 +- test/render_subtitles_test.cc | 3 +- 9 files changed, 82 insertions(+), 32 deletions(-) diff --git a/src/lib/dcp_decoder.cc b/src/lib/dcp_decoder.cc index 7e2001e0e..9064627ba 100644 --- a/src/lib/dcp_decoder.cc +++ b/src/lib/dcp_decoder.cc @@ -303,7 +303,8 @@ DCPDecoder::pass_texts ( ContentTime::from_frames(_offset - entry_point, vfr) + ContentTime::from_seconds(b.in().as_seconds()), ContentTime::from_frames(_offset - entry_point, vfr) + ContentTime::from_seconds(b.out().as_seconds()) ), - strings + strings, + _dcp_content->standard() ); strings.clear (); } @@ -338,7 +339,8 @@ DCPDecoder::pass_texts ( ContentTime::from_frames(_offset - entry_point, vfr) + ContentTime::from_seconds(b.in().as_seconds()), ContentTime::from_frames(_offset - entry_point, vfr) + ContentTime::from_seconds(b.out().as_seconds()) ), - strings + strings, + _dcp_content->standard() ); strings.clear (); } diff --git a/src/lib/dcp_subtitle_decoder.cc b/src/lib/dcp_subtitle_decoder.cc index 08c2bf7ec..cbfe6fdbe 100644 --- a/src/lib/dcp_subtitle_decoder.cc +++ b/src/lib/dcp_subtitle_decoder.cc @@ -46,6 +46,12 @@ DCPSubtitleDecoder::DCPSubtitleDecoder (shared_ptr film, shared_ptr< _subtitles = asset->subtitles (); _next = _subtitles.begin (); + if (dynamic_pointer_cast(asset)) { + _standard = dcp::Standard::INTEROP; + } else { + _standard = dcp::Standard::SMPTE; + } + text.push_back (make_shared(this, content->only_text())); update_position(); } @@ -102,7 +108,7 @@ DCPSubtitleDecoder::pass () } } - only_text()->emit_plain (p, s); + only_text()->emit_plain(p, s, _standard); update_position(); diff --git a/src/lib/dcp_subtitle_decoder.h b/src/lib/dcp_subtitle_decoder.h index 16f89b530..3eed4ad24 100644 --- a/src/lib/dcp_subtitle_decoder.h +++ b/src/lib/dcp_subtitle_decoder.h @@ -42,4 +42,6 @@ private: std::vector> _subtitles; std::vector>::const_iterator _next; + + dcp::Standard _standard; }; diff --git a/src/lib/render_text.cc b/src/lib/render_text.cc index 574ee21a0..9c1c233c2 100644 --- a/src/lib/render_text.cc +++ b/src/lib/render_text.cc @@ -245,27 +245,41 @@ x_position (StringText const& first, int target_width, int layout_width) static int -y_position (StringText const& first, int target_height, int layout_height) +y_position (StringText const& first, int target_height, int baseline_to_bottom, int layout_height) { int y = 0; - switch (first.v_align()) { - case dcp::VAlign::TOP: - /* SMPTE says that v_position is the distance between top - of frame and top of subtitle, but this doesn't always seem to be - the case in practice; Gunnar Ásgeirsson's Dolby server appears - to put VAlign::TOP subs with v_position as the distance between top - of frame and bottom of subtitle. - */ - y = first.v_position() * target_height - layout_height; - break; - case dcp::VAlign::CENTER: - /* v_position is distance between centre of frame and centre of subtitle */ - y = (0.5 + first.v_position()) * target_height - layout_height / 2; - break; - case dcp::VAlign::BOTTOM: - /* v_position is distance between bottom of frame and bottom of subtitle */ - y = (1.0 - first.v_position()) * target_height - layout_height; + switch (first.valign_standard) { + case dcp::Standard::INTEROP: + switch (first.v_align()) { + case dcp::VAlign::TOP: + /* v_position is distance from top of frame to subtitle baseline */ + y = first.v_position() * target_height - (layout_height - baseline_to_bottom); + break; + case dcp::VAlign::CENTER: + /* v_position is distance from centre of frame to subtitle baseline */ + y = (0.5 + first.v_position()) * target_height - (layout_height - baseline_to_bottom); + break; + case dcp::VAlign::BOTTOM: + /* v_position is distance from bottom of frame to subtitle baseline */ + y = (1.0 - first.v_position()) * target_height - (layout_height - baseline_to_bottom); + break; + } break; + case dcp::Standard::SMPTE: + switch (first.v_align()) { + case dcp::VAlign::TOP: + /* v_position is distance from top of frame to top of subtitle */ + y = first.v_position() * target_height; + break; + case dcp::VAlign::CENTER: + /* v_position is distance from centre of frame to centre of subtitle */ + y = (0.5 + first.v_position()) * target_height - layout_height / 2; + break; + case dcp::VAlign::BOTTOM: + /* v_position is distance from bottom of frame to bottom of subtitle */ + y = (1.0 - first.v_position()) * target_height - layout_height; + break; + } } return y; @@ -359,7 +373,7 @@ render_line (list subtitles, dcp::Size target, DCPTime time, int fra context->stroke (); int const x = x_position (first, target.width, size.width); - int const y = y_position (first, target.height, size.height); + int const y = y_position (first, target.height, ink.get_y() / Pango::SCALE, size.height); return PositionImage (image, Position(max (0, x), max(0, y))); } diff --git a/src/lib/string_text.h b/src/lib/string_text.h index 8c505f36a..4eef7da05 100644 --- a/src/lib/string_text.h +++ b/src/lib/string_text.h @@ -32,20 +32,35 @@ * - include settings that are not applicable to true DCP subtitles. * For example, we can set outline width for burn-in but this cannot be specified in DCP XML. * + * - include details of how v_align should be interpreted + * * - specify the font by referring to a Font object from the content we came from, rather than * having to use a DCP ID like in dcp::SubtitleString. */ class StringText : public dcp::SubtitleString { public: - StringText (dcp::SubtitleString dcp_, int outline_width_, std::shared_ptr font_) + StringText(dcp::SubtitleString dcp_, int outline_width_, std::shared_ptr font_, dcp::Standard valign_standard_) : dcp::SubtitleString (dcp_) , outline_width (outline_width_) , font (font_) + , valign_standard (valign_standard_) {} int outline_width; std::shared_ptr font; + /** Interop and SMPTE use the same VAlign choices (top, center, bottom) but give them different + * meanings. This is the standard which should be used to interpret v_align() in this subtitle; + * valign_standard == SMPTE means: + * top - top of screen to top of subtitle + * center - centre of screen to center of subtitle + * bottom - bottom of screen to bottom of subtitle + * valign_standard == Interop means: + * top - top of screen to baseline of subtitle + * center - centre of screen to baseline of subtitle + * bottom - bottom of screen to baseline of subtitle + */ + dcp::Standard valign_standard; }; diff --git a/src/lib/text_decoder.cc b/src/lib/text_decoder.cc index b1b6cbcc4..930262a74 100644 --- a/src/lib/text_decoder.cc +++ b/src/lib/text_decoder.cc @@ -102,12 +102,17 @@ set_forced_appearance(shared_ptr content, StringText& subtitl void -TextDecoder::emit_plain_start (ContentTime from, vector subtitles) +TextDecoder::emit_plain_start (ContentTime from, vector subtitles, dcp::Standard valign_standard) { vector string_texts; for (auto& subtitle: subtitles) { - auto string_text = StringText(subtitle, content()->outline_width(), subtitle.font() ? content()->get_font(*subtitle.font()) : shared_ptr()); + auto string_text = StringText( + subtitle, + content()->outline_width(), + subtitle.font() ? content()->get_font(*subtitle.font()) : shared_ptr(), + valign_standard + ); string_text.set_text(escape_text(string_text.text())); set_forced_appearance(content(), string_text); string_texts.push_back(string_text); @@ -265,7 +270,12 @@ TextDecoder::emit_plain_start (ContentTime from, sub::Subtitle const & sub_subti 0 ); - auto string_text = StringText(dcp_subtitle, content()->outline_width(), content()->get_font(block.font.get_value_or(""))); + auto string_text = StringText( + dcp_subtitle, + content()->outline_width(), + content()->get_font(block.font.get_value_or("")), + dcp::Standard::SMPTE + ); set_forced_appearance(content(), string_text); string_texts.push_back(string_text); } @@ -284,9 +294,9 @@ TextDecoder::emit_stop (ContentTime to) void -TextDecoder::emit_plain (ContentTimePeriod period, vector subtitles) +TextDecoder::emit_plain (ContentTimePeriod period, vector subtitles, dcp::Standard valign_standard) { - emit_plain_start (period.from, subtitles); + emit_plain_start (period.from, subtitles, valign_standard); emit_stop (period.to); } diff --git a/src/lib/text_decoder.h b/src/lib/text_decoder.h index 5f01c5b1f..9b3050f71 100644 --- a/src/lib/text_decoder.h +++ b/src/lib/text_decoder.h @@ -50,9 +50,9 @@ public: void emit_bitmap_start (ContentBitmapText const& bitmap); void emit_bitmap (dcpomatic::ContentTimePeriod period, std::shared_ptr image, dcpomatic::Rect rect); - void emit_plain_start (dcpomatic::ContentTime from, std::vector s); + void emit_plain_start (dcpomatic::ContentTime from, std::vector s, dcp::Standard valign_standard); void emit_plain_start (dcpomatic::ContentTime from, sub::Subtitle const & subtitle); - void emit_plain (dcpomatic::ContentTimePeriod period, std::vector s); + void emit_plain (dcpomatic::ContentTimePeriod period, std::vector s, dcp::Standard valign_standard); void emit_plain (dcpomatic::ContentTimePeriod period, sub::Subtitle const & subtitle); void emit_stop (dcpomatic::ContentTime to); diff --git a/src/lib/util.cc b/src/lib/util.cc index dd9bbb7eb..cfeacdb92 100644 --- a/src/lib/util.cc +++ b/src/lib/util.cc @@ -411,7 +411,7 @@ LIBDCP_ENABLE_WARNINGS optional(), false, false, false, dcp::Colour(), 42, 1, dcp::Time(), dcp::Time(), 0, dcp::HAlign::CENTER, 0, dcp::VAlign::CENTER, dcp::Direction::LTR, "Hello dolly", dcp::Effect::NONE, dcp::Colour(), dcp::Time(), dcp::Time(), 0 ); - subs.push_back (StringText(ss, 0, {})); + subs.push_back (StringText(ss, 0, {}, dcp::Standard::SMPTE)); render_text (subs, dcp::Size(640, 480), DCPTime(), 24); #endif diff --git a/test/render_subtitles_test.cc b/test/render_subtitles_test.cc index cd696c269..282c9311f 100644 --- a/test/render_subtitles_test.cc +++ b/test/render_subtitles_test.cc @@ -64,7 +64,8 @@ add (std::list& s, std::string text, bool italic, bool bold, bool un 0 ), 2, - std::shared_ptr() + std::shared_ptr(), + dcp::Standard::SMPTE ) ); } -- 2.30.2