From 323b8cbb0b95297fbd027ffdc4ea5003b59ef25f Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Fri, 18 Nov 2022 10:56:42 +0100 Subject: Fix subtitle vertical position (#2367). Previously we would not account for the differences in what vertical position means between Interop and SMPTE. For interop, vertical position is the distance from the reference point to the text baseline, whereas for SMPTE it is the distance from the reference point to the top/middle/bottom of the subtitle (depending on the reference). This caused differences between the preview and the DCP for some cases (notably, using SRT/SSA and making Interop DCPs, or converting Interop DCP subs to SMPTE, or vice versa). --- src/lib/reel_writer.cc | 29 +++++++++++++++++++++ src/lib/reel_writer.h | 4 +++ src/lib/render_text.cc | 68 +++++++++++++++++++++++++++++++++++++++++++++++--- src/lib/render_text.h | 41 ++++++++++++++++++++++++++++++ 4 files changed, 138 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/lib/reel_writer.cc b/src/lib/reel_writer.cc index f9e29a16f..94b12ec7d 100644 --- a/src/lib/reel_writer.cc +++ b/src/lib/reel_writer.cc @@ -114,6 +114,7 @@ ReelWriter::ReelWriter ( , _content_summary (film()->content_summary(period)) , _job (job) , _text_only (text_only) + , _font_metrics(film()->frame_size().height) { /* Create or find our picture asset in a subdirectory, named according to those film's parameters which affect the video @@ -889,6 +890,33 @@ ReelWriter::empty_text_asset (TextType type, optional track, bool } +float +ReelWriter::convert_vertical_position(StringText const& subtitle, dcp::Standard to) const +{ + if (subtitle.valign_standard == to) { + return subtitle.v_position(); + } + + auto const baseline_to_bottom = _font_metrics.baseline_to_bottom(subtitle); + auto const height = _font_metrics.height(subtitle); + + float correction = 0; + switch (subtitle.v_align()) { + case dcp::VAlign::TOP: + correction = height - baseline_to_bottom; + break; + case dcp::VAlign::CENTER: + correction = (height / 2) - baseline_to_bottom; + break; + case dcp::VAlign::BOTTOM: + correction = baseline_to_bottom; + break; + } + + return subtitle.v_position() + ((subtitle.valign_standard == dcp::Standard::SMPTE) ? correction : -correction); +} + + void ReelWriter::write (PlayerText subs, TextType type, optional track, DCPTimePeriod period, FontIdMap const& fonts) { @@ -928,6 +956,7 @@ ReelWriter::write (PlayerText subs, TextType type, optional track, for (auto i: subs.string) { i.set_in (dcp::Time(period.from.seconds() - _period.from.seconds(), tcr)); i.set_out (dcp::Time(period.to.seconds() - _period.from.seconds(), tcr)); + i.set_v_position(convert_vertical_position(i, film()->interop() ? dcp::Standard::INTEROP : dcp::Standard::SMPTE)); auto sub = make_shared(i); if (type == TextType::OPEN_SUBTITLE) { sub->set_font(fonts.get(i.font)); diff --git a/src/lib/reel_writer.h b/src/lib/reel_writer.h index e2c713cc5..d5b531f41 100644 --- a/src/lib/reel_writer.h +++ b/src/lib/reel_writer.h @@ -26,6 +26,7 @@ #include "font_id_map.h" #include "player_text.h" #include "referenced_reel_asset.h" +#include "render_text.h" #include "types.h" #include "weak_film.h" #include @@ -121,6 +122,7 @@ private: std::set ensure_closed_captions ) const; void create_reel_markers (std::shared_ptr reel) const; + float convert_vertical_position(StringText const& subtitle, dcp::Standard to) const; dcpomatic::DCPTimePeriod _period; /** the first picture frame index that does not already exist in our MXF */ @@ -147,5 +149,7 @@ private: std::shared_ptr _atmos_asset; std::shared_ptr _atmos_asset_writer; + mutable FontMetrics _font_metrics; + static int const _info_size; }; diff --git a/src/lib/render_text.cc b/src/lib/render_text.cc index 9c1c233c2..d3db4ac19 100644 --- a/src/lib/render_text.cc +++ b/src/lib/render_text.cc @@ -171,12 +171,12 @@ create_surface (shared_ptr image) static string -setup_font (StringText const& subtitle) +setup_font(shared_ptr font) { auto font_file = default_font_file (); - if (subtitle.font && subtitle.font->file()) { - font_file = *subtitle.font->file(); + if (font && font->file()) { + font_file = *font->file(); } return FontConfig::instance()->make_font_available(font_file); @@ -299,7 +299,7 @@ render_line (list subtitles, dcp::Size target, DCPTime time, int fra DCPOMATIC_ASSERT (!subtitles.empty ()); auto const& first = subtitles.front (); - auto const font_name = setup_font (first); + auto const font_name = setup_font(first.font); auto const fade_factor = calculate_fade_factor (first, time, frame_rate); auto const markup = marked_up (subtitles, target.height, fade_factor, font_name); auto layout = create_layout (); @@ -402,3 +402,63 @@ render_text (list subtitles, dcp::Size target, DCPTime time, int fra return images; } + + +float +FontMetrics::height(StringText const& subtitle) +{ + return get(subtitle)->second.second; +} + + +float +FontMetrics::baseline_to_bottom(StringText const& subtitle) +{ + return get(subtitle)->second.first; +} + + +FontMetrics::Cache::iterator +FontMetrics::get(StringText const& subtitle) +{ + auto id = Identifier(subtitle); + + auto iter = _cache.find(id); + if (iter != _cache.end()) { + return iter; + } + + auto const font_name = setup_font(subtitle.font); + auto layout = create_layout(); + auto copy = subtitle; + copy.set_text("Qypjg"); + setup_layout(layout, font_name, marked_up({copy}, _target_height, 1, font_name)); + auto ink = layout->get_ink_extents(); + auto const scale = float(_target_height * Pango::SCALE); + return _cache.insert({id, { ink.get_y() / scale, ink.get_height() / scale}}).first; +} + + +FontMetrics::Identifier::Identifier(StringText const& subtitle) + : font(subtitle.font) + , size(subtitle.size()) + , aspect_adjust(subtitle.aspect_adjust()) +{ + +} + + +bool +FontMetrics::Identifier::operator<(FontMetrics::Identifier const& other) const +{ + if (font != other.font) { + return font < other.font; + } + + if (size != other.size) { + return size < other.size; + } + + return aspect_adjust < other.aspect_adjust; +} + diff --git a/src/lib/render_text.h b/src/lib/render_text.h index 6ff91dce1..762d79446 100644 --- a/src/lib/render_text.h +++ b/src/lib/render_text.h @@ -18,14 +18,55 @@ */ + #include "position_image.h" #include "dcpomatic_time.h" #include "string_text.h" #include +#include + namespace dcpomatic { class Font; } + std::string marked_up (std::list subtitles, int target_height, float fade_factor, std::string font_name); std::list render_text (std::list, dcp::Size, dcpomatic::DCPTime, int); + + +class FontMetrics +{ +public: + FontMetrics(int target_height) + : _target_height(target_height) + {} + + float baseline_to_bottom(StringText const& subtitle); + float height(StringText const& subtitle); + +private: + /** Class to collect the properties of a subtitle which affect the metrics we care about + * i.e. baseline position and height. + */ + class Identifier + { + public: + Identifier(StringText const& subtitle); + + std::shared_ptr font; + int size; + float aspect_adjust; + + bool operator<(Identifier const& other) const; + }; + + using Cache = std::map>; + + Cache::iterator get(StringText const& subtitle); + + Cache _cache; + + int _target_height; +}; + -- cgit v1.2.3