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

7 files changed, 159 insertions, 63 deletions

diff --git a/src/interop_subtitle_asset.cc b/src/interop_subtitle_asset.cc
index 2ef82c90..1cd99563 100644
--- 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 267ff27e..91afec14 100644
--- 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 5a096f20..31cb30c9 100644
--- 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 31cca185..59ed1165 100644
--- 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 b0034743..366123c5 100644
--- 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 974b496f..49c00f77 100644
--- 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 d0c47328..c398ef78 100644
--- 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);