From: Carl Hetherington Date: Thu, 9 Jun 2016 22:28:41 +0000 (+0100) Subject: Fix some code duplication and crashes when decoding FFmpeg-embedded ASS subtitles... X-Git-Tag: v2.8.10~47 X-Git-Url: https://git.carlh.net/gitweb/?p=dcpomatic.git;a=commitdiff_plain;h=31a33018f113cb7b631dd7bb4d0ed2a7de914cd8 Fix some code duplication and crashes when decoding FFmpeg-embedded ASS subtitles along the way. --- diff --git a/src/lib/ffmpeg_decoder.cc b/src/lib/ffmpeg_decoder.cc index 57d300e18..28c638998 100644 --- a/src/lib/ffmpeg_decoder.cc +++ b/src/lib/ffmpeg_decoder.cc @@ -614,54 +614,8 @@ FFmpegDecoder::decode_ass_subtitle (string ass, ContentTimePeriod period) sub::RawSubtitle base; list raw = sub::SSAReader::parse_line (base, bits[9]); - list subs = sub::collect > (raw); - /* XXX: lots of this is copied from TextSubtitle; there should probably be some sharing */ - - /* Highest line index in this subtitle */ - int highest = 0; - BOOST_FOREACH (sub::Subtitle i, subs) { - BOOST_FOREACH (sub::Line j, i.lines) { - DCPOMATIC_ASSERT (j.vertical_position.reference && j.vertical_position.reference.get() == sub::TOP_OF_SUBTITLE); - DCPOMATIC_ASSERT (j.vertical_position.line); - highest = max (highest, j.vertical_position.line.get()); - } + BOOST_FOREACH (sub::Subtitle const & i, sub::collect > (raw)) { + subtitle->give_text (period, i); } - - list ss; - - BOOST_FOREACH (sub::Subtitle i, sub::collect > (sub::SSAReader::parse_line (base, bits[9]))) { - BOOST_FOREACH (sub::Line j, i.lines) { - BOOST_FOREACH (sub::Block k, j.blocks) { - ss.push_back ( - dcp::SubtitleString ( - boost::optional (), - k.italic, - k.bold, - subtitle->content()->colour(), - /* 48pt is 1/22nd of the screen height */ - 48, - 1, - dcp::Time (i.from.seconds(), 1000), - dcp::Time (i.to.seconds(), 1000), - 0, - dcp::HALIGN_CENTER, - /* This 1.015 is an arbitrary value to lift the bottom sub off the bottom - of the screen a bit to a pleasing degree. - */ - 1.015 - ((1 + highest - j.vertical_position.line.get()) * 1.5 / 22), - dcp::VALIGN_TOP, - dcp::DIRECTION_LTR, - k.text, - subtitle->content()->outline() ? dcp::BORDER : dcp::NONE, - subtitle->content()->outline_colour(), - dcp::Time (), - dcp::Time () - ) - ); - } - } - } - - subtitle->give_text (period, ss); } diff --git a/src/lib/subtitle_decoder.cc b/src/lib/subtitle_decoder.cc index 766c7ed87..c0f402e20 100644 --- a/src/lib/subtitle_decoder.cc +++ b/src/lib/subtitle_decoder.cc @@ -20,12 +20,15 @@ #include "subtitle_decoder.h" #include "subtitle_content.h" +#include "util.h" +#include #include #include #include using std::list; using std::cout; +using std::string; using boost::shared_ptr; using boost::optional; using boost::function; @@ -132,3 +135,76 @@ SubtitleDecoder::seek (ContentTime, bool) _decoded_text.clear (); _decoded_image.clear (); } + +void +SubtitleDecoder::give_text (ContentTimePeriod period, sub::Subtitle const & subtitle) +{ + /* See if our next subtitle needs to be placed on screen by us */ + bool needs_placement = false; + BOOST_FOREACH (sub::Line i, subtitle.lines) { + if (!i.vertical_position.reference && i.vertical_position.reference.get() == sub::TOP_OF_SUBTITLE) { + needs_placement = true; + } + } + + list out; + BOOST_FOREACH (sub::Line i, subtitle.lines) { + BOOST_FOREACH (sub::Block j, i.blocks) { + + float v_position; + dcp::VAlign v_align; + if (needs_placement) { + DCPOMATIC_ASSERT (i.vertical_position.line); + /* This 0.878 is an arbitrary value to lift the bottom sub off the bottom + of the screen a bit to a pleasing degree. + */ + v_position = 0.878 + i.vertical_position.line.get() * 1.5 / 22; + v_align = dcp::VALIGN_BOTTOM; + } else { + DCPOMATIC_ASSERT (i.vertical_position.proportional); + DCPOMATIC_ASSERT (i.vertical_position.reference); + v_position = i.vertical_position.proportional.get(); + switch (i.vertical_position.reference.get()) { + case sub::TOP_OF_SCREEN: + v_align = dcp::VALIGN_TOP; + break; + case sub::CENTRE_OF_SCREEN: + v_align = dcp::VALIGN_CENTER; + break; + case sub::BOTTOM_OF_SCREEN: + v_align = dcp::VALIGN_BOTTOM; + break; + default: + v_align = dcp::VALIGN_TOP; + break; + } + } + + out.push_back ( + dcp::SubtitleString ( + string(TEXT_FONT_ID), + j.italic, + j.bold, + /* force the colour to whatever is configured */ + content()->colour(), + j.font_size.points (72 * 11), + 1.0, + dcp::Time (subtitle.from.all_as_seconds(), 1000), + dcp::Time (subtitle.to.all_as_seconds(), 1000), + 0, + dcp::HALIGN_CENTER, + v_position, + v_align, + dcp::DIRECTION_LTR, + j.text, + content()->outline() ? dcp::BORDER : dcp::NONE, + content()->outline_colour(), + dcp::Time (0, 1000), + dcp::Time (0, 1000) + ) + ); + } + } + + give_text (period, out); +} diff --git a/src/lib/subtitle_decoder.h b/src/lib/subtitle_decoder.h index 73d447b9a..7a8ab32ec 100644 --- a/src/lib/subtitle_decoder.h +++ b/src/lib/subtitle_decoder.h @@ -27,6 +27,10 @@ #include "content_subtitle.h" #include +namespace sub { + class Subtitle; +} + class Image; class SubtitleDecoder @@ -50,6 +54,7 @@ public: void give_image (ContentTimePeriod period, boost::shared_ptr, dcpomatic::Rect); void give_text (ContentTimePeriod period, std::list); + void give_text (ContentTimePeriod period, sub::Subtitle const & subtitle); boost::shared_ptr content () const { return _content; diff --git a/src/lib/text_subtitle_content.cc b/src/lib/text_subtitle_content.cc index 37ad5649a..24a328d08 100644 --- a/src/lib/text_subtitle_content.cc +++ b/src/lib/text_subtitle_content.cc @@ -34,8 +34,6 @@ using std::string; using std::cout; using boost::shared_ptr; -std::string const TextSubtitleContent::font_id = "font"; - TextSubtitleContent::TextSubtitleContent (shared_ptr film, boost::filesystem::path path) : Content (film, path) { @@ -60,7 +58,7 @@ TextSubtitleContent::examine (boost::shared_ptr job) boost::mutex::scoped_lock lm (_mutex); _length = s.length (); - subtitle->add_font (shared_ptr (new Font (font_id))); + subtitle->add_font (shared_ptr (new Font (TEXT_FONT_ID))); } string diff --git a/src/lib/text_subtitle_content.h b/src/lib/text_subtitle_content.h index 318d37803..3b9d396f6 100644 --- a/src/lib/text_subtitle_content.h +++ b/src/lib/text_subtitle_content.h @@ -41,8 +41,6 @@ public: void as_xml (xmlpp::Node *) const; DCPTime full_length () const; - static std::string const font_id; - private: ContentTime _length; }; diff --git a/src/lib/text_subtitle_decoder.cc b/src/lib/text_subtitle_decoder.cc index ecca5188b..5216863a0 100644 --- a/src/lib/text_subtitle_decoder.cc +++ b/src/lib/text_subtitle_decoder.cc @@ -66,75 +66,7 @@ TextSubtitleDecoder::pass (PassReason, bool) return true; } - list out; - - /* See if our next subtitle needs to be placed on screen by us */ - bool needs_placement = false; - BOOST_FOREACH (sub::Line i, _subtitles[_next].lines) { - if (!i.vertical_position.reference && i.vertical_position.reference.get() == sub::TOP_OF_SUBTITLE) { - needs_placement = true; - } - } - - BOOST_FOREACH (sub::Line i, _subtitles[_next].lines) { - BOOST_FOREACH (sub::Block j, i.blocks) { - - float v_position; - dcp::VAlign v_align; - if (needs_placement) { - DCPOMATIC_ASSERT (i.vertical_position.line); - /* This 0.878 is an arbitrary value to lift the bottom sub off the bottom - of the screen a bit to a pleasing degree. - */ - v_position = 0.878 + i.vertical_position.line.get() * 1.5 / 22; - v_align = dcp::VALIGN_BOTTOM; - } else { - DCPOMATIC_ASSERT (i.vertical_position.proportional); - DCPOMATIC_ASSERT (i.vertical_position.reference); - v_position = i.vertical_position.proportional.get(); - switch (i.vertical_position.reference.get()) { - case sub::TOP_OF_SCREEN: - v_align = dcp::VALIGN_TOP; - break; - case sub::CENTRE_OF_SCREEN: - v_align = dcp::VALIGN_CENTER; - break; - case sub::BOTTOM_OF_SCREEN: - v_align = dcp::VALIGN_BOTTOM; - break; - default: - v_align = dcp::VALIGN_TOP; - break; - } - } - - out.push_back ( - dcp::SubtitleString ( - TextSubtitleContent::font_id, - j.italic, - j.bold, - /* force the colour to whatever is configured */ - subtitle->content()->colour(), - j.font_size.points (72 * 11), - 1.0, - dcp::Time (_subtitles[_next].from.all_as_seconds(), 1000), - dcp::Time (_subtitles[_next].to.all_as_seconds(), 1000), - 0, - dcp::HALIGN_CENTER, - v_position, - v_align, - dcp::DIRECTION_LTR, - j.text, - subtitle->content()->outline() ? dcp::BORDER : dcp::NONE, - subtitle->content()->outline_colour(), - dcp::Time (0, 1000), - dcp::Time (0, 1000) - ) - ); - } - } - - subtitle->give_text (content_time_period (_subtitles[_next]), out); + subtitle->give_text (content_time_period (_subtitles[_next]), _subtitles[_next]); ++_next; return false; diff --git a/src/lib/util.h b/src/lib/util.h index 0b572b8ab..f6306c971 100644 --- a/src/lib/util.h +++ b/src/lib/util.h @@ -48,6 +48,7 @@ namespace dcp { /** Number of films to keep in history */ #define HISTORY_SIZE 10 #define REPORT_PROBLEM _("Please report this problem by using Help -> Report a problem or via email to carl@dcpomatic.com") +#define TEXT_FONT_ID "font" extern std::string program_name;