From 37494990bbe81f41f581de8258dc7353b82d662d Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Sat, 8 Oct 2016 00:45:22 +0100 Subject: [PATCH] Fix overlapping burnt-in subtitles in some cases (#959). Firstly, when finding subtitles that exist during a period, only return those which overlap more than half the period. This means that, in a fight over a frame, the longest-running subtitle in that frame will win. Secondly, make SubtitleDecoder::get pick the wanted subtitles from the cache simply by comparing their periods to those that were requested. I think this is nicer than what was there before (basically reevaulating 'what subtitle(s) for this period') and also makes the first part of this commit effective. --- ChangeLog | 4 ++++ src/lib/dcpomatic_time.h | 4 ++++ src/lib/subtitle_decoder.cc | 26 ++++++++++++++++---------- src/lib/subtitle_decoder.h | 2 +- src/lib/text_subtitle_decoder.cc | 12 +++++++++++- 5 files changed, 36 insertions(+), 12 deletions(-) diff --git a/ChangeLog b/ChangeLog index 34fe178a4..d603234d0 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,7 @@ +2016-10-08 Carl Hetherington + + * Fix overlapping burnt-in subtitles in some cases (#959). + 2016-10-07 Carl Hetherington * Fix XML subtitle output in some cases. diff --git a/src/lib/dcpomatic_time.h b/src/lib/dcpomatic_time.h index c13f2bbfe..da903cf09 100644 --- a/src/lib/dcpomatic_time.h +++ b/src/lib/dcpomatic_time.h @@ -275,6 +275,10 @@ public: bool operator== (TimePeriod const & other) const { return from == other.from && to == other.to; } + + bool operator!= (TimePeriod const & other) const { + return !(*this == other); + } }; typedef TimePeriod ContentTimePeriod; diff --git a/src/lib/subtitle_decoder.cc b/src/lib/subtitle_decoder.cc index bc4a75ca8..307226da6 100644 --- a/src/lib/subtitle_decoder.cc +++ b/src/lib/subtitle_decoder.cc @@ -78,13 +78,15 @@ SubtitleDecoder::give_text (ContentTimePeriod period, list _decoded_text.push_back (ContentTextSubtitle (period, s)); } -/** @param sp Full periods of subtitles that are showing or starting during the specified period */ +/** Get the subtitles that correspond to a given list of periods. + * @param subs Subtitles. + * @param sp Periods for which to extract subtitles from subs. + */ template list -SubtitleDecoder::get (list const & subs, list const & sp, ContentTimePeriod period, bool starting, bool accurate) +SubtitleDecoder::get (list const & subs, list const & sp, ContentTimePeriod period, bool accurate) { if (sp.empty ()) { - /* Nothing in this period */ return list (); } @@ -103,9 +105,13 @@ SubtitleDecoder::get (list const & subs, list const & sp, /* XXX: inefficient */ list out; - for (typename list::const_iterator i = subs.begin(); i != subs.end(); ++i) { - if ((starting && period.contains(i->period().from)) || (!starting && period.overlap(i->period()))) { - out.push_back (*i); + BOOST_FOREACH (ContentTimePeriod i, sp) { + typename list::const_iterator j = subs.begin(); + while (j != subs.end() && j->period() != i) { + ++j; + } + if (j != subs.end()) { + out.push_back (*j); } } @@ -132,13 +138,13 @@ SubtitleDecoder::get (list const & subs, list const & sp, list SubtitleDecoder::get_text (ContentTimePeriod period, bool starting, bool accurate) { - return get (_decoded_text, _text_during (period, starting), period, starting, accurate); + return get (_decoded_text, _text_during (period, starting), period, accurate); } list SubtitleDecoder::get_image (ContentTimePeriod period, bool starting, bool accurate) { - return get (_decoded_image, _image_during (period, starting), period, starting, accurate); + return get (_decoded_image, _image_during (period, starting), period, accurate); } void @@ -170,7 +176,7 @@ SubtitleDecoder::give_text (ContentTimePeriod period, sub::Subtitle const & subt } } - /* Find the lowest proportional postion */ + /* Find the lowest proportional position */ optional lowest_proportional; BOOST_FOREACH (sub::Line i, subtitle.lines) { if (i.vertical_position.proportional) { @@ -187,7 +193,7 @@ SubtitleDecoder::give_text (ContentTimePeriod period, sub::Subtitle const & subt BOOST_FOREACH (sub::Block j, i.blocks) { if (!j.font_size.specified()) { - /* Fallback default font size if none other has been specified */ + /* Fallback default font size if no other has been specified */ j.font_size.set_points (48); } diff --git a/src/lib/subtitle_decoder.h b/src/lib/subtitle_decoder.h index 726e898b1..eba36315e 100644 --- a/src/lib/subtitle_decoder.h +++ b/src/lib/subtitle_decoder.h @@ -69,7 +69,7 @@ private: boost::shared_ptr _content; template - std::list get (std::list const & subs, std::list const & sp, ContentTimePeriod period, bool starting, bool accurate); + std::list get (std::list const & subs, std::list const & sp, ContentTimePeriod period, bool accurate); boost::function (ContentTimePeriod, bool)> _image_during; boost::function (ContentTimePeriod, bool)> _text_during; diff --git a/src/lib/text_subtitle_decoder.cc b/src/lib/text_subtitle_decoder.cc index bec2ab9b7..05bde829c 100644 --- a/src/lib/text_subtitle_decoder.cc +++ b/src/lib/text_subtitle_decoder.cc @@ -85,10 +85,20 @@ TextSubtitleDecoder::text_subtitles_during (ContentTimePeriod p, bool starting) list d; + /* Only take `during' (not starting) subs if they overlap more than half the requested period; + here's the threshold for being significant. + */ + ContentTime const significant (p.duration().get() / 2); + for (vector::const_iterator i = _subtitles.begin(); i != _subtitles.end(); ++i) { ContentTimePeriod t = content_time_period (*i); - if ((starting && p.contains (t.from)) || (!starting && p.overlap (t))) { + if (starting && p.contains(t.from)) { d.push_back (t); + } else if (!starting) { + optional const o = p.overlap (t); + if (o && o->duration() > significant) { + d.push_back (t); + } } } -- 2.30.2