Store image subtitle ID in the object, rather than a separate map. Start of reading...

diff --git a/src/interop_subtitle_asset.cc b/src/interop_subtitle_asset.cc
index 2ef82c901e64b0b9ba9ea014d8a2fb312fd70a6d..1cd995638ac1979ad85b7d31dc683627fd815093 100644 (file)
--- a/src/interop_subtitle_asset.cc
+++ b/src/interop_subtitle_asset.cc
@@ -79,6 +79,8 @@ InteropSubtitleAsset::InteropSubtitleAsset (boost::filesystem::path file)
                        parse_subtitles (e, ps, optional<int>(), INTEROP);
                }
        }
+
+       /* XXX: now find SubtitleImages in _subtitles and load their PNG */
 }
 
 InteropSubtitleAsset::InteropSubtitleAsset ()
@@ -182,9 +184,7 @@ InteropSubtitleAsset::write (boost::filesystem::path p) const
        BOOST_FOREACH (shared_ptr<dcp::Subtitle> i, _subtitles) {
                shared_ptr<dcp::SubtitleImage> im = dynamic_pointer_cast<dcp::SubtitleImage> (i);
                if (im) {
-                       ImageUUIDMap::const_iterator uuid = _image_subtitle_uuid.find(im);
-                       DCP_ASSERT (uuid != _image_subtitle_uuid.end());
-                       im->png_image().write (p.parent_path() / String::compose("%1.png", uuid->second));
+                       im->png_image().write (p.parent_path() / String::compose("%1.png", im->id()));
                }
        }
 

diff --git a/src/smpte_subtitle_asset.cc b/src/smpte_subtitle_asset.cc
index 267ff27e692f0a79efa321391f395e68d6318623..91afec143840dfd617aef7aa522a67e72ec54aa8 100644 (file)
--- a/src/smpte_subtitle_asset.cc
+++ b/src/smpte_subtitle_asset.cc
@@ -195,6 +195,8 @@ SMPTESubtitleAsset::read_mxf_descriptor (shared_ptr<ASDCP::TimedText::MXFReader>
                }
        }
 
+       /* XXX: load PNG and attach them to _subtitles */
+
        /* Get intrinsic duration */
        _intrinsic_duration = descriptor.ContainerDuration;
 }
@@ -323,13 +325,16 @@ SMPTESubtitleAsset::write (boost::filesystem::path p) const
 
        /* Image subtitle references */
 
-       for (ImageUUIDMap::const_iterator i = _image_subtitle_uuid.begin(); i != _image_subtitle_uuid.end(); ++i) {
-               ASDCP::TimedText::TimedTextResourceDescriptor res;
-               unsigned int c;
-               Kumu::hex2bin (i->second.c_str(), res.ResourceID, Kumu::UUID_Length, &c);
-               DCP_ASSERT (c == Kumu::UUID_Length);
-               res.Type = ASDCP::TimedText::MT_PNG;
-               descriptor.ResourceList.push_back (res);
+       BOOST_FOREACH (shared_ptr<Subtitle> i, _subtitles) {
+               shared_ptr<SubtitleImage> si = dynamic_pointer_cast<SubtitleImage>(i);
+               if (si) {
+                       ASDCP::TimedText::TimedTextResourceDescriptor res;
+                       unsigned int c;
+                       Kumu::hex2bin (si->id().c_str(), res.ResourceID, Kumu::UUID_Length, &c);
+                       DCP_ASSERT (c == Kumu::UUID_Length);
+                       res.Type = ASDCP::TimedText::MT_PNG;
+                       descriptor.ResourceList.push_back (res);
+               }
        }
 
        descriptor.NamespaceName = "dcst";
@@ -367,13 +372,16 @@ SMPTESubtitleAsset::write (boost::filesystem::path p) const
 
        /* Image subtitle payload */
 
-       for (ImageUUIDMap::const_iterator i = _image_subtitle_uuid.begin(); i != _image_subtitle_uuid.end(); ++i) {
-               ASDCP::TimedText::FrameBuffer buffer;
-               buffer.SetData (i->first->png_image().data().get(), i->first->png_image().size());
-               buffer.Size (i->first->png_image().size());
-               r = writer.WriteAncillaryResource (buffer, enc.context(), enc.hmac());
-               if (ASDCP_FAILURE(r)) {
-                       boost::throw_exception (MXFFileError ("could not write PNG data to timed text resource", p.string(), r));
+       BOOST_FOREACH (shared_ptr<Subtitle> i, _subtitles) {
+               shared_ptr<SubtitleImage> si = dynamic_pointer_cast<SubtitleImage>(i);
+               if (si) {
+                       ASDCP::TimedText::FrameBuffer buffer;
+                       buffer.SetData (si->png_image().data().get(), si->png_image().size());
+                       buffer.Size (si->png_image().size());
+                       r = writer.WriteAncillaryResource (buffer, enc.context(), enc.hmac());
+                       if (ASDCP_FAILURE(r)) {
+                               boost::throw_exception (MXFFileError ("could not write PNG data to timed text resource", p.string(), r));
+                       }
                }
        }
 

diff --git a/src/subtitle_asset.cc b/src/subtitle_asset.cc
index 5a096f204e55419b46bf13c001e2387ee4edac1f..31cb30c9183d6b645dce06135c2a47621712e1f7 100644 (file)
--- a/src/subtitle_asset.cc
+++ b/src/subtitle_asset.cc
@@ -152,11 +152,9 @@ SubtitleAsset::font_node_state (xmlpp::Element const * node, Standard standard)
        return ps;
 }
 
-SubtitleAsset::ParseState
-SubtitleAsset::text_node_state (xmlpp::Element const * node) const
+void
+SubtitleAsset::position_align (SubtitleAsset::ParseState& ps, xmlpp::Element const * node) const
 {
-       ParseState ps;
-
        optional<float> hp = optional_number_attribute<float> (node, "HPosition");
        if (!hp) {
                hp = optional_number_attribute<float> (node, "Hposition");
@@ -189,11 +187,34 @@ SubtitleAsset::text_node_state (xmlpp::Element const * node) const
                ps.v_align = string_to_valign (va.get ());
        }
 
+}
+
+SubtitleAsset::ParseState
+SubtitleAsset::text_node_state (xmlpp::Element const * node) const
+{
+       ParseState ps;
+
+       position_align (ps, node);
+
        optional<string> d = optional_string_attribute (node, "Direction");
        if (d) {
                ps.direction = string_to_direction (d.get ());
        }
 
+       ps.type = ParseState::TEXT;
+
+       return ps;
+}
+
+SubtitleAsset::ParseState
+SubtitleAsset::image_node_state (xmlpp::Element const * node) const
+{
+       ParseState ps;
+
+       position_align (ps, node);
+
+       ps.type = ParseState::IMAGE;
+
        return ps;
 }
 
@@ -240,6 +261,8 @@ SubtitleAsset::parse_subtitles (xmlpp::Element const * node, list<ParseState>& s
                state.push_back (text_node_state (node));
        } else if (node->get_name() == "SubtitleList") {
                state.push_back (ParseState ());
+       } else if (node->get_name() == "Image") {
+               state.push_back (image_node_state (node));
        } else {
                throw XMLError ("unexpected node " + node->get_name());
        }
@@ -248,7 +271,7 @@ SubtitleAsset::parse_subtitles (xmlpp::Element const * node, list<ParseState>& s
        for (xmlpp::Node::NodeList::const_iterator i = c.begin(); i != c.end(); ++i) {
                xmlpp::ContentNode const * v = dynamic_cast<xmlpp::ContentNode const *> (*i);
                if (v) {
-                       maybe_add_subtitle (v->get_content(), state);
+                       maybe_add_subtitle (v->get_content(), state, standard);
                }
                xmlpp::Element const * e = dynamic_cast<xmlpp::Element const *> (*i);
                if (e) {
@@ -260,7 +283,7 @@ SubtitleAsset::parse_subtitles (xmlpp::Element const * node, list<ParseState>& s
 }
 
 void
-SubtitleAsset::maybe_add_subtitle (string text, list<ParseState> const & parse_state)
+SubtitleAsset::maybe_add_subtitle (string text, list<ParseState> const & parse_state, Standard standard)
 {
        if (empty_or_white_space (text)) {
                return;
@@ -322,38 +345,66 @@ SubtitleAsset::maybe_add_subtitle (string text, list<ParseState> const & parse_s
                if (i.fade_down_time) {
                        ps.fade_down_time = i.fade_down_time.get();
                }
+               if (i.type) {
+                       ps.type = i.type.get();
+               }
        }
 
        if (!ps.in || !ps.out) {
-               /* We're not in a <Text> node; just ignore this content */
+               /* We're not in a <Subtitle> node; just ignore this content */
                return;
        }
 
-       _subtitles.push_back (
-               shared_ptr<Subtitle> (
-                       new SubtitleString (
-                               ps.font_id,
-                               ps.italic.get_value_or (false),
-                               ps.bold.get_value_or (false),
-                               ps.underline.get_value_or (false),
-                               ps.colour.get_value_or (dcp::Colour (255, 255, 255)),
-                               ps.size.get_value_or (42),
-                               ps.aspect_adjust.get_value_or (1.0),
-                               ps.in.get(),
-                               ps.out.get(),
-                               ps.h_position.get_value_or(0),
-                               ps.h_align.get_value_or(HALIGN_CENTER),
-                               ps.v_position.get_value_or(0),
-                               ps.v_align.get_value_or(VALIGN_CENTER),
-                               ps.direction.get_value_or (DIRECTION_LTR),
-                               text,
-                               ps.effect.get_value_or (NONE),
-                               ps.effect_colour.get_value_or (dcp::Colour (0, 0, 0)),
-                               ps.fade_up_time.get_value_or(Time()),
-                               ps.fade_down_time.get_value_or(Time())
+       DCP_ASSERT (ps.type);
+
+       switch (ps.type.get()) {
+       case ParseState::TEXT:
+               _subtitles.push_back (
+                       shared_ptr<Subtitle> (
+                               new SubtitleString (
+                                       ps.font_id,
+                                       ps.italic.get_value_or (false),
+                                       ps.bold.get_value_or (false),
+                                       ps.underline.get_value_or (false),
+                                       ps.colour.get_value_or (dcp::Colour (255, 255, 255)),
+                                       ps.size.get_value_or (42),
+                                       ps.aspect_adjust.get_value_or (1.0),
+                                       ps.in.get(),
+                                       ps.out.get(),
+                                       ps.h_position.get_value_or(0),
+                                       ps.h_align.get_value_or(HALIGN_CENTER),
+                                       ps.v_position.get_value_or(0),
+                                       ps.v_align.get_value_or(VALIGN_CENTER),
+                                       ps.direction.get_value_or (DIRECTION_LTR),
+                                       text,
+                                       ps.effect.get_value_or (NONE),
+                                       ps.effect_colour.get_value_or (dcp::Colour (0, 0, 0)),
+                                       ps.fade_up_time.get_value_or(Time()),
+                                       ps.fade_down_time.get_value_or(Time())
+                                       )
+                               )
+                       );
+               break;
+       case ParseState::IMAGE:
+               /* Add a subtitle with no image data and we'll fill that in later */
+               _subtitles.push_back (
+                       shared_ptr<Subtitle> (
+                               new SubtitleImage (
+                                       Data (),
+                                       standard == INTEROP ? text.substr(0, text.size() - 4) : text,
+                                       ps.in.get(),
+                                       ps.out.get(),
+                                       ps.h_position.get_value_or(0),
+                                       ps.h_align.get_value_or(HALIGN_CENTER),
+                                       ps.v_position.get_value_or(0),
+                                       ps.v_align.get_value_or(VALIGN_CENTER),
+                                       ps.fade_up_time.get_value_or(Time()),
+                                       ps.fade_down_time.get_value_or(Time())
+                                       )
                                )
-                       )
-               );
+                       );
+               break;
+       }
 }
 
 list<shared_ptr<Subtitle> >
@@ -373,11 +424,6 @@ void
 SubtitleAsset::add (shared_ptr<Subtitle> s)
 {
        _subtitles.push_back (s);
-
-       shared_ptr<SubtitleImage> si = dynamic_pointer_cast<SubtitleImage> (s);
-       if (si) {
-               _image_subtitle_uuid[si] = make_uuid ();
-       }
 }
 
 Time
@@ -578,10 +624,8 @@ SubtitleAsset::subtitles_as_xml (xmlpp::Element* xml_root, int time_code_rate, S
                shared_ptr<SubtitleImage> ii = dynamic_pointer_cast<SubtitleImage>(i);
                if (ii) {
                        text.reset ();
-                       ImageUUIDMap::const_iterator uuid = _image_subtitle_uuid.find(ii);
-                       DCP_ASSERT (uuid != _image_subtitle_uuid.end());
                        subtitle->children.push_back (
-                               shared_ptr<order::Image> (new order::Image (subtitle, uuid->second, ii->png_image(), ii->h_align(), ii->h_position(), ii->v_align(), ii->v_position()))
+                               shared_ptr<order::Image> (new order::Image (subtitle, ii->id(), ii->png_image(), ii->h_align(), ii->h_position(), ii->v_align(), ii->v_position()))
                                );
                }
        }

diff --git a/src/subtitle_asset.h b/src/subtitle_asset.h
index 31cca18584b1f9fa69980de6aeddc574e987c81c..59ed1165475a41d5ca7957f9428ced598c2508b5 100644 (file)
--- a/src/subtitle_asset.h
+++ b/src/subtitle_asset.h
@@ -126,13 +126,20 @@ protected:
                boost::optional<Time> out;
                boost::optional<Time> fade_up_time;
                boost::optional<Time> fade_down_time;
+               enum Type {
+                       TEXT,
+                       IMAGE
+               };
+               boost::optional<Type> type;
        };
 
        void parse_subtitles (xmlpp::Element const * node, std::list<ParseState>& state, boost::optional<int> tcr, Standard standard);
        ParseState font_node_state (xmlpp::Element const * node, Standard standard) const;
        ParseState text_node_state (xmlpp::Element const * node) const;
+       ParseState image_node_state (xmlpp::Element const * node) const;
        ParseState subtitle_node_state (xmlpp::Element const * node, boost::optional<int> tcr) const;
        Time fade_time (xmlpp::Element const * node, std::string name, boost::optional<int> tcr) const;
+       void position_align (ParseState& ps, xmlpp::Element const * node) const;
 
        void subtitles_as_xml (xmlpp::Element* root, int time_code_rate, Standard standard) const;
 
@@ -165,16 +172,12 @@ protected:
        /** TTF font data that we need */
        std::list<Font> _fonts;
 
-       /** Map of image subtitles to UUIDs */
-       typedef std::map<boost::shared_ptr<dcp::SubtitleImage>, std::string> ImageUUIDMap;
-       ImageUUIDMap _image_subtitle_uuid;
-
 private:
        friend struct ::pull_fonts_test1;
        friend struct ::pull_fonts_test2;
        friend struct ::pull_fonts_test3;
 
-       void maybe_add_subtitle (std::string text, std::list<ParseState> const & parse_state);
+       void maybe_add_subtitle (std::string text, std::list<ParseState> const & parse_state, Standard standard);
 
        static void pull_fonts (boost::shared_ptr<order::Part> part);
 };

diff --git a/src/subtitle_asset_internal.h b/src/subtitle_asset_internal.h
index b0034743421e4f393767cb561cd2c3a5229bdbfd..366123c51afaaa8a4f9f1068fd22db51459109fd 100644 (file)
--- a/src/subtitle_asset_internal.h
+++ b/src/subtitle_asset_internal.h
@@ -181,7 +181,7 @@ public:
 
 private:
        Data _png_data;
-       std::string _id; ///< the ID of this image (index for Interop, UUID for SMPTE)
+       std::string _id; ///< the ID of this image
        HAlign _h_align;
        float _h_position;
        VAlign _v_align;

diff --git a/src/subtitle_image.cc b/src/subtitle_image.cc
index 974b496f5f700343c546062f73c4f777095fe307..49c00f77789536141186336f4d47c4d4bf39cfd9 100644 (file)
--- a/src/subtitle_image.cc
+++ b/src/subtitle_image.cc
@@ -32,8 +32,10 @@
 */
 
 #include "subtitle_image.h"
+#include "util.h"
 
 using std::ostream;
+using std::string;
 using namespace dcp;
 
 SubtitleImage::SubtitleImage (
@@ -49,6 +51,26 @@ SubtitleImage::SubtitleImage (
        )
        : Subtitle (in, out, h_position, h_align, v_position, v_align, fade_up_time, fade_down_time)
        , _png_image (png_image)
+       , _id (make_uuid ())
+{
+
+}
+
+SubtitleImage::SubtitleImage (
+       Data png_image,
+       string id,
+       Time in,
+       Time out,
+       float h_position,
+       HAlign h_align,
+       float v_position,
+       VAlign v_align,
+       Time fade_up_time,
+       Time fade_down_time
+       )
+       : Subtitle (in, out, h_position, h_align, v_position, v_align, fade_up_time, fade_down_time)
+       , _png_image (png_image)
+       , _id (id)
 {
 
 }
@@ -57,7 +79,8 @@ bool
 dcp::operator== (SubtitleImage const & a, SubtitleImage const & b)
 {
        return (
-               a.png_image() == b.png_image(),
+               a.png_image() == b.png_image() &&
+               a.id() == b.id() &&
                a.in() == b.in() &&
                a.out() == b.out() &&
                a.h_position() == b.h_position() &&

diff --git a/src/subtitle_image.h b/src/subtitle_image.h
index d0c4732894c929298d23dad1a3f899d2959c89f3..c398ef78213f931b321ee307ad3274d9d1725a44 100644 (file)
--- a/src/subtitle_image.h
+++ b/src/subtitle_image.h
@@ -65,12 +65,30 @@ public:
                Time fade_down_time
                );
 
+       SubtitleImage (
+               Data png_image,
+               std::string id,
+               Time in,
+               Time out,
+               float h_position,
+               HAlign h_align,
+               float v_position,
+               VAlign v_align,
+               Time fade_up_time,
+               Time fade_down_time
+               );
+
        Data png_image () const {
                return _png_image;
        }
 
+       std::string id () const {
+               return _id;
+       }
+
 private:
        Data _png_image;
+       std::string _id;
 };
 
 bool operator== (SubtitleImage const & a, SubtitleImage const & b);

diff --git a/test/read_interop_subtitle_test.cc b/test/read_interop_subtitle_test.cc
index c91ba03c4cadb862381ad07e603db377d2f54f20..6997a9a3cf05e6d90390c77528891052975ec587 100644 (file)
--- a/test/read_interop_subtitle_test.cc
+++ b/test/read_interop_subtitle_test.cc
@@ -595,3 +595,9 @@ BOOST_AUTO_TEST_CASE (read_interop_subtitle_test2)
                                   dcp::Time (0, 0, 0, 0, 250)
                                   ));
 }
+
+/** And one with bitmap subtitles */
+BOOST_AUTO_TEST_CASE (read_interop_subtitle_test3)
+{
+       dcp::InteropSubtitleAsset subs ("test/data/subs3.xml");
+}