Fix some code duplication and crashes when decoding FFmpeg-embedded ASS subtitles...

diff --git a/src/lib/ffmpeg_decoder.cc b/src/lib/ffmpeg_decoder.cc
index 57d300e18cf0b18e0c863df0aa26fc38ca5ea014..28c638998de6a850b2478b9346434de7efe537f7 100644 (file)
--- 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<sub::RawSubtitle> raw = sub::SSAReader::parse_line (base, bits[9]);
-       list<sub::Subtitle> subs = sub::collect<list<sub::Subtitle> > (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<list<sub::Subtitle> > (raw)) {
+               subtitle->give_text (period, i);
        }
-
-       list<dcp::SubtitleString> ss;
-
-       BOOST_FOREACH (sub::Subtitle i, sub::collect<list<sub::Subtitle> > (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<string> (),
-                                               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 766c7ed87738814c3185202142c987252fb2ace4..c0f402e2009688b2b6192fc60c7bd970b20ecb49 100644 (file)
--- 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 <sub/subtitle.h>
 #include <boost/shared_ptr.hpp>
 #include <boost/foreach.hpp>
 #include <iostream>
 
 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<dcp::SubtitleString> 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 73d447b9a9480990cee298e5da13758e56f81efb..7a8ab32ecce704c953a58f40c0ba8c5442ed4617 100644 (file)
--- a/src/lib/subtitle_decoder.h
+++ b/src/lib/subtitle_decoder.h
@@ -27,6 +27,10 @@
 #include "content_subtitle.h"
 #include <dcp/subtitle_string.h>
 
+namespace sub {
+       class Subtitle;
+}
+
 class Image;
 
 class SubtitleDecoder
@@ -50,6 +54,7 @@ public:
 
        void give_image (ContentTimePeriod period, boost::shared_ptr<Image>, dcpomatic::Rect<double>);
        void give_text (ContentTimePeriod period, std::list<dcp::SubtitleString>);
+       void give_text (ContentTimePeriod period, sub::Subtitle const & subtitle);
 
        boost::shared_ptr<const SubtitleContent> content () const {
                return _content;

diff --git a/src/lib/text_subtitle_content.cc b/src/lib/text_subtitle_content.cc
index 37ad5649aec353faa2a37070b71c5519a0dddfe2..24a328d088e3c05001f9be31f2f1b614233010da 100644 (file)
--- 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<const Film> film, boost::filesystem::path path)
        : Content (film, path)
 {
@@ -60,7 +58,7 @@ TextSubtitleContent::examine (boost::shared_ptr<Job> job)
 
        boost::mutex::scoped_lock lm (_mutex);
        _length = s.length ();
-       subtitle->add_font (shared_ptr<Font> (new Font (font_id)));
+       subtitle->add_font (shared_ptr<Font> (new Font (TEXT_FONT_ID)));
 }
 
 string

diff --git a/src/lib/text_subtitle_content.h b/src/lib/text_subtitle_content.h
index 318d37803fd933ea4f270729f893909903be7feb..3b9d396f610dd2f56f7d5b069f099d23734d9371 100644 (file)
--- 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 ecca5188be349441a1ddb0e09c9ebcd426993384..5216863a0e8e76a6c7e1e23df7b53ff85b51a40c 100644 (file)
--- 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<dcp::SubtitleString> 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 0b572b8ab2d60c0abd9e7b8e2cc6b631860a2c2f..f6306c9712528fb04b0e6f3b8f019048b6ef5ba5 100644 (file)
--- 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;