Rework subtitle writing to fix incorrect output with

in-line font changes (#952).

6 files changed, 519 insertions, 129 deletions

diff --git a/src/object.h b/src/object.h
index 31fb5275..0804f4cf 100644
--- a/src/object.h
+++ b/src/object.h
@@ -43,6 +43,7 @@
 
 class write_interop_subtitle_test;
 class write_smpte_subtitle_test;
+class write_smpte_subtitle_test2;
 
 namespace dcp {
 
@@ -64,6 +65,7 @@ public:
 protected:
 	friend class ::write_interop_subtitle_test;
 	friend class ::write_smpte_subtitle_test;
+	friend class ::write_smpte_subtitle_test2;
 
 	/** ID */
 	std::string _id;
diff --git a/src/subtitle_asset.cc b/src/subtitle_asset.cc
index ac87148d..19f7fa37 100644
--- a/src/subtitle_asset.cc
+++ b/src/subtitle_asset.cc
@@ -1,5 +1,5 @@
 /*
-    Copyright (C) 2012-2015 Carl Hetherington <cth@carlh.net>
+    Copyright (C) 2012-2016 Carl Hetherington <cth@carlh.net>
 
     This file is part of libdcp.
 
@@ -34,6 +34,7 @@
 #include "raw_convert.h"
 #include "compose.hpp"
 #include "subtitle_asset.h"
+#include "subtitle_asset_internal.h"
 #include "util.h"
 #include "xml.h"
 #include "subtitle_string.h"
@@ -51,6 +52,7 @@ using std::list;
 using std::cout;
 using std::cerr;
 using std::map;
+using std::distance;
 using boost::shared_ptr;
 using boost::shared_array;
 using boost::optional;
@@ -403,7 +405,8 @@ SubtitleAsset::equals (shared_ptr<const Asset> other_asset, EqualityOptions opti
 	return true;
 }
 
-struct SubtitleSorter {
+struct SubtitleSorter
+{
 	bool operator() (SubtitleString const & a, SubtitleString const & b) {
 		if (a.in() != b.in()) {
 			return a.in() < b.in();
@@ -412,165 +415,144 @@ struct SubtitleSorter {
 	}
 };
 
+void
+SubtitleAsset::pull_fonts (shared_ptr<order::Part> part)
+{
+	if (part->children.empty ()) {
+		return;
+	}
+
+	/* Pull up from children */
+	BOOST_FOREACH (shared_ptr<order::Part> i, part->children) {
+		pull_fonts (i);
+	}
+
+	if (part->parent) {
+		/* Establish the common font features that each of part's children have;
+		   these features go into part's font.
+		*/
+		part->font = part->children.front()->font;
+		BOOST_FOREACH (shared_ptr<order::Part> i, part->children) {
+			part->font.take_intersection (i->font);
+		}
+
+		/* Remove common values from part's children's fonts */
+		BOOST_FOREACH (shared_ptr<order::Part> i, part->children) {
+			i->font.take_difference (part->font);
+		}
+	}
+
+	/* Merge adjacent children with the same font */
+	list<shared_ptr<order::Part> >::const_iterator i = part->children.begin();
+	list<shared_ptr<order::Part> > merged;
+
+	while (i != part->children.end()) {
+
+		if ((*i)->font.empty ()) {
+			merged.push_back (*i);
+			++i;
+		} else {
+			list<shared_ptr<order::Part> >::const_iterator j = i;
+			++j;
+			while (j != part->children.end() && (*i)->font == (*j)->font) {
+				++j;
+			}
+			if (distance (i, j) == 1) {
+				merged.push_back (*i);
+				++i;
+			} else {
+				shared_ptr<order::Part> group (new order::Part (part, (*i)->font));
+				for (list<shared_ptr<order::Part> >::const_iterator k = i; k != j; ++k) {
+					(*k)->font.clear ();
+					group->children.push_back (*k);
+				}
+				merged.push_back (group);
+				i = j;
+			}
+		}
+	}
+
+	part->children = merged;
+}
+
 /** @param standard Standard (INTEROP or SMPTE); this is used rather than putting things in the child
  *  class because the differences between the two are fairly subtle.
  */
 void
-SubtitleAsset::subtitles_as_xml (xmlpp::Element* root, int time_code_rate, Standard standard) const
+SubtitleAsset::subtitles_as_xml (xmlpp::Element* xml_root, int time_code_rate, Standard standard) const
 {
 	list<SubtitleString> sorted = _subtitles;
 	sorted.sort (SubtitleSorter ());
 
-	string const xmlns = standard == SMPTE ? "dcst" : "";
+	/* Gather our subtitles into a hierarchy of Subtitle/Text/String objects, writing
+	   font information into the bottom level (String) objects.
+	*/
 
-	/* XXX: script not supported */
+	shared_ptr<order::Part> root (new order::Part (shared_ptr<order::Part> ()));
+	shared_ptr<order::Subtitle> subtitle;
+	shared_ptr<order::Text> text;
 
-	optional<string> font;
-	bool italic = false;
-	bool bold = false;
-	bool underline = false;
-	Colour colour;
-	int size = 0;
-	float aspect_adjust = 1.0;
-	Effect effect = NONE;
-	Colour effect_colour;
-	int spot_number = 1;
 	Time last_in;
 	Time last_out;
 	Time last_fade_up_time;
 	Time last_fade_down_time;
-
-	xmlpp::Element* font_element = 0;
-	xmlpp::Element* subtitle_element = 0;
+	HAlign last_h_align;
+	float last_h_position;
+	VAlign last_v_align;
+	float last_v_position;
+	Direction last_direction;
 
 	BOOST_FOREACH (SubtitleString const & i, sorted) {
-
-		/* We will start a new <Font>...</Font> whenever some font property changes.
-		   I suppose we should really make an optimal hierarchy of <Font> tags, but
-		   that seems hard.
-		*/
-
-		bool const font_changed =
-			font          != i.font()          ||
-			italic        != i.italic()        ||
-			bold          != i.bold()          ||
-			underline     != i.underline()     ||
-			colour        != i.colour()        ||
-			size          != i.size()          ||
-			fabs (aspect_adjust - i.aspect_adjust()) > ASPECT_ADJUST_EPSILON ||
-			effect        != i.effect()        ||
-			effect_colour != i.effect_colour();
-
-		if (font_changed) {
-			font = i.font ();
-			italic = i.italic ();
-			bold = i.bold ();
-			underline = i.underline ();
-			colour = i.colour ();
-			size = i.size ();
-			aspect_adjust = i.aspect_adjust ();
-			effect = i.effect ();
-			effect_colour = i.effect_colour ();
-		}
-
-		if (!font_element || font_changed) {
-			font_element = root->add_child ("Font", xmlns);
-			if (font) {
-				if (standard == SMPTE) {
-					font_element->set_attribute ("ID", font.get ());
-				} else {
-					font_element->set_attribute ("Id", font.get ());
-				}
-			}
-			font_element->set_attribute ("Italic", italic ? "yes" : "no");
-			font_element->set_attribute ("Color", colour.to_argb_string());
-			font_element->set_attribute ("Size", raw_convert<string> (size));
-			if (fabs (aspect_adjust - 1.0) > ASPECT_ADJUST_EPSILON) {
-				font_element->set_attribute ("AspectAdjust", raw_convert<string> (aspect_adjust));
-			}
-			font_element->set_attribute ("Effect", effect_to_string (effect));
-			font_element->set_attribute ("EffectColor", effect_colour.to_argb_string());
-			font_element->set_attribute ("Script", "normal");
-			if (standard == SMPTE) {
-				font_element->set_attribute ("Underline", underline ? "yes" : "no");
-			} else {
-				font_element->set_attribute ("Underlined", underline ? "yes" : "no");
-			}
-			font_element->set_attribute ("Weight", bold ? "bold" : "normal");
-		}
-
-		if (!subtitle_element || font_changed ||
+		if (!subtitle ||
 		    (last_in != i.in() ||
 		     last_out != i.out() ||
 		     last_fade_up_time != i.fade_up_time() ||
-		     last_fade_down_time != i.fade_down_time()
-			    )) {
-
-			subtitle_element = font_element->add_child ("Subtitle", xmlns);
-			subtitle_element->set_attribute ("SpotNumber", raw_convert<string> (spot_number++));
-			subtitle_element->set_attribute ("TimeIn", i.in().rebase(time_code_rate).as_string(standard));
-			subtitle_element->set_attribute ("TimeOut", i.out().rebase(time_code_rate).as_string(standard));
-			if (standard == SMPTE) {
-				subtitle_element->set_attribute ("FadeUpTime", i.fade_up_time().rebase(time_code_rate).as_string(standard));
-				subtitle_element->set_attribute ("FadeDownTime", i.fade_down_time().rebase(time_code_rate).as_string(standard));
-			} else {
-				subtitle_element->set_attribute ("FadeUpTime", raw_convert<string> (i.fade_up_time().as_editable_units(time_code_rate)));
-				subtitle_element->set_attribute ("FadeDownTime", raw_convert<string> (i.fade_down_time().as_editable_units(time_code_rate)));
-			}
+		     last_fade_down_time != i.fade_down_time())
+			) {
+
+			subtitle.reset (new order::Subtitle (root, i.in(), i.out(), i.fade_up_time(), i.fade_down_time()));
+			root->children.push_back (subtitle);
 
 			last_in = i.in ();
 			last_out = i.out ();
 			last_fade_up_time = i.fade_up_time ();
 			last_fade_down_time = i.fade_down_time ();
+			text.reset ();
 		}
 
-		xmlpp::Element* text = subtitle_element->add_child ("Text", xmlns);
-
-		if (i.h_align() != HALIGN_CENTER) {
-			if (standard == SMPTE) {
-				text->set_attribute ("Halign", halign_to_string (i.h_align ()));
-			} else {
-				text->set_attribute ("HAlign", halign_to_string (i.h_align ()));
-			}
+		if (!text ||
+		    last_h_align != i.h_align() ||
+		    fabs(last_h_position - i.h_position()) > ALIGN_EPSILON ||
+		    last_v_align != i.v_align() ||
+		    fabs(last_v_position - i.v_position()) > ALIGN_EPSILON ||
+		    last_direction != i.direction()
+			) {
+
+			text.reset (new order::Text (subtitle, i.h_align(), i.h_position(), i.v_align(), i.v_position(), i.direction()));
+			subtitle->children.push_back (text);
+
+			last_h_align = i.h_align ();
+			last_h_position = i.h_position ();
+			last_v_align = i.v_align ();
+			last_v_position = i.v_position ();
+			last_direction = i.direction ();
 		}
 
-		if (i.h_position() > ALIGN_EPSILON) {
-			if (standard == SMPTE) {
-				text->set_attribute ("Hposition", raw_convert<string> (i.h_position() * 100, 6));
-			} else {
-				text->set_attribute ("HPosition", raw_convert<string> (i.h_position() * 100, 6));
-			}
-		}
+		text->children.push_back (shared_ptr<order::String> (new order::String (text, order::Font (i, standard), i.text())));
+	}
 
-		if (standard == SMPTE) {
-			text->set_attribute ("Valign", valign_to_string (i.v_align()));
-		} else {
-			text->set_attribute ("VAlign", valign_to_string (i.v_align()));
-		}
+	/* Pull font changes as high up the hierarchy as we can */
 
-		if (i.v_position() > ALIGN_EPSILON) {
-			if (standard == SMPTE) {
-				text->set_attribute ("Vposition", raw_convert<string> (i.v_position() * 100, 6));
-			} else {
-				text->set_attribute ("VPosition", raw_convert<string> (i.v_position() * 100, 6));
-			}
-		} else {
-			if (standard == SMPTE) {
-				text->set_attribute ("Vposition", "0");
-			} else {
-				text->set_attribute ("VPosition", "0");
-			}
-		}
+	pull_fonts (root);
 
-		/* Interop only supports "horizontal" or "vertical" for direction, so only write this
-		   for SMPTE.
-		*/
-		if (i.direction() != DIRECTION_LTR && standard == SMPTE) {
-			text->set_attribute ("Direction", direction_to_string (i.direction ()));
-		}
+	/* Write XML */
 
-		text->add_child_text (i.text());
-	}
+	order::Context context;
+	context.time_code_rate = time_code_rate;
+	context.standard = standard;
+	context.spot_number = 1;
+
+	root->write_xml (xml_root, context);
 }
 
 map<string, Data>
diff --git a/src/subtitle_asset.h b/src/subtitle_asset.h
index 62314263..e40118c4 100644
--- a/src/subtitle_asset.h
+++ b/src/subtitle_asset.h
@@ -48,6 +48,9 @@ namespace xmlpp {
 
 struct interop_dcp_font_test;
 struct smpte_dcp_font_test;
+struct pull_fonts_test1;
+struct pull_fonts_test2;
+struct pull_fonts_test3;
 
 namespace dcp
 {
@@ -58,6 +61,11 @@ class TextNode;
 class SubtitleNode;
 class LoadFontNode;
 
+namespace order {
+	class Part;
+	class Context;
+}
+
 /** @class SubtitleAsset
  *  @brief A parent for classes representing a file containing subtitles.
  *
@@ -156,7 +164,13 @@ protected:
 	std::list<Font> _fonts;
 
 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);
+
+	static void pull_fonts (boost::shared_ptr<order::Part> part);
 };
 
 }
diff --git a/src/subtitle_asset_internal.cc b/src/subtitle_asset_internal.cc
new file mode 100644
index 00000000..bd28cdcb
--- /dev/null
+++ b/src/subtitle_asset_internal.cc
@@ -0,0 +1,224 @@
+/*
+    Copyright (C) 2012-2016 Carl Hetherington <cth@carlh.net>
+
+    This file is part of libdcp.
+
+    libdcp is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 2 of the License, or
+    (at your option) any later version.
+
+    libdcp is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with libdcp.  If not, see <http://www.gnu.org/licenses/>.
+
+    In addition, as a special exception, the copyright holders give
+    permission to link the code of portions of this program with the
+    OpenSSL library under certain conditions as described in each
+    individual source file, and distribute linked combinations
+    including the two.
+
+    You must obey the GNU General Public License in all respects
+    for all of the code used other than OpenSSL.  If you modify
+    file(s) with this exception, you may extend this exception to your
+    version of the file(s), but you are not obligated to do so.  If you
+    do not wish to do so, delete this exception statement from your
+    version.  If you delete this exception statement from all source
+    files in the program, then also delete it here.
+*/
+
+#include "subtitle_asset_internal.h"
+#include "subtitle_string.h"
+
+using std::string;
+using std::map;
+using namespace dcp;
+
+string
+order::Context::xmlns () const
+{
+	return standard == SMPTE ? "dcst" : "";
+}
+
+order::Font::Font (SubtitleString const & s, Standard standard)
+{
+	if (s.font()) {
+		if (standard == SMPTE) {
+			_values["ID"] = s.font().get ();
+		} else {
+			_values["Id"] = s.font().get ();
+		}
+	}
+	_values["Italic"] = s.italic() ? "yes" : "no";
+	_values["Color"] = s.colour().to_argb_string();
+	_values["Size"] = raw_convert<string> (s.size());
+	_values["AspectAdjust"] = raw_convert<string>(s.aspect_adjust(), 1, true);
+	_values["Effect"] = effect_to_string (s.effect());
+	_values["EffectColor"] = s.effect_colour().to_argb_string();
+	_values["Script"] = "normal";
+	if (standard == SMPTE) {
+		_values["Underline"] = s.underline() ? "yes" : "no";
+	} else {
+		_values["Underlined"] = s.underline() ? "yes" : "no";
+	}
+	_values["Weight"] = s.bold() ? "bold" : "normal";
+}
+
+xmlpp::Element*
+order::Font::as_xml (xmlpp::Element* parent, Context& context) const
+{
+	xmlpp::Element* e = parent->add_child ("Font", context.xmlns());
+	for (map<string, string>::const_iterator i = _values.begin(); i != _values.end(); ++i) {
+		e->set_attribute (i->first, i->second);
+	}
+	return e;
+}
+
+/** Modify our values so that they contain only those that are common to us and
+ *  other.
+ */
+void
+order::Font::take_intersection (Font other)
+{
+	map<string, string> inter;
+
+	for (map<string, string>::const_iterator i = other._values.begin(); i != other._values.end(); ++i) {
+		map<string, string>::iterator t = _values.find (i->first);
+		if (t != _values.end() && t->second == i->second) {
+			inter.insert (*i);
+		}
+	}
+
+	_values = inter;
+}
+
+/** Modify our values so that it contains only those keys that are not in other */
+void
+order::Font::take_difference (Font other)
+{
+	map<string, string> diff;
+	for (map<string, string>::const_iterator i = _values.begin(); i != _values.end(); ++i) {
+		if (other._values.find (i->first) == other._values.end ()) {
+			diff.insert (*i);
+		}
+	}
+
+	_values = diff;
+}
+
+bool
+order::Font::empty () const
+{
+	return _values.empty ();
+}
+
+xmlpp::Element*
+order::Part::as_xml (xmlpp::Element* parent, Context &) const
+{
+	return parent;
+}
+
+xmlpp::Element*
+order::String::as_xml (xmlpp::Element* parent, Context &) const
+{
+	parent->add_child_text (text);
+	return 0;
+}
+
+void
+order::Part::write_xml (xmlpp::Element* parent, order::Context& context) const
+{
+	if (!font.empty ()) {
+		parent = font.as_xml (parent, context);
+	}
+
+	parent = as_xml (parent, context);
+
+	BOOST_FOREACH (boost::shared_ptr<order::Part> i, children) {
+		i->write_xml (parent, context);
+	}
+}
+
+xmlpp::Element*
+order::Text::as_xml (xmlpp::Element* parent, Context& context) const
+{
+	xmlpp::Element* e = parent->add_child ("Text", context.xmlns());
+
+	if (_h_align != HALIGN_CENTER) {
+		if (context.standard == SMPTE) {
+			e->set_attribute ("Halign", halign_to_string (_h_align));
+		} else {
+			e->set_attribute ("HAlign", halign_to_string (_h_align));
+		}
+	}
+
+	if (_h_position > ALIGN_EPSILON) {
+		if (context.standard == SMPTE) {
+			e->set_attribute ("Hposition", raw_convert<string> (_h_position * 100, 6));
+		} else {
+			e->set_attribute ("HPosition", raw_convert<string> (_h_position * 100, 6));
+		}
+	}
+
+	if (context.standard == SMPTE) {
+		e->set_attribute ("Valign", valign_to_string (_v_align));
+	} else {
+		e->set_attribute ("VAlign", valign_to_string (_v_align));
+	}
+
+	if (_v_position > ALIGN_EPSILON) {
+		if (context.standard == SMPTE) {
+			e->set_attribute ("Vposition", raw_convert<string> (_v_position * 100, 6));
+		} else {
+			e->set_attribute ("VPosition", raw_convert<string> (_v_position * 100, 6));
+		}
+	} else {
+		if (context.standard == SMPTE) {
+			e->set_attribute ("Vposition", "0");
+		} else {
+			e->set_attribute ("VPosition", "0");
+		}
+	}
+
+	/* Interop only supports "horizontal" or "vertical" for direction, so only write this
+	   for SMPTE.
+	*/
+	if (_direction != DIRECTION_LTR && context.standard == SMPTE) {
+		e->set_attribute ("Direction", direction_to_string (_direction));
+	}
+
+	return e;
+}
+
+xmlpp::Element*
+order::Subtitle::as_xml (xmlpp::Element* parent, Context& context) const
+{
+	xmlpp::Element* e = parent->add_child ("Subtitle", context.xmlns());
+	e->set_attribute ("SpotNumber", raw_convert<string> (context.spot_number++));
+	e->set_attribute ("TimeIn", _in.rebase(context.time_code_rate).as_string(context.standard));
+	e->set_attribute ("TimeOut", _out.rebase(context.time_code_rate).as_string(context.standard));
+	if (context.standard == SMPTE) {
+		e->set_attribute ("FadeUpTime", _fade_up.rebase(context.time_code_rate).as_string(context.standard));
+		e->set_attribute ("FadeDownTime", _fade_down.rebase(context.time_code_rate).as_string(context.standard));
+	} else {
+		e->set_attribute ("FadeUpTime", raw_convert<string> (_fade_up.as_editable_units(context.time_code_rate)));
+		e->set_attribute ("FadeDownTime", raw_convert<string> (_fade_down.as_editable_units(context.time_code_rate)));
+	}
+	return e;
+}
+
+bool
+order::Font::operator== (Font const & other) const
+{
+	return _values == other._values;
+}
+
+void
+order::Font::clear ()
+{
+	_values.clear ();
+}
diff --git a/src/subtitle_asset_internal.h b/src/subtitle_asset_internal.h
new file mode 100644
index 00000000..cea230af
--- /dev/null
+++ b/src/subtitle_asset_internal.h
@@ -0,0 +1,167 @@
+/*
+    Copyright (C) 2012-2016 Carl Hetherington <cth@carlh.net>
+
+    This file is part of libdcp.
+
+    libdcp is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 2 of the License, or
+    (at your option) any later version.
+
+    libdcp is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with libdcp.  If not, see <http://www.gnu.org/licenses/>.
+
+    In addition, as a special exception, the copyright holders give
+    permission to link the code of portions of this program with the
+    OpenSSL library under certain conditions as described in each
+    individual source file, and distribute linked combinations
+    including the two.
+
+    You must obey the GNU General Public License in all respects
+    for all of the code used other than OpenSSL.  If you modify
+    file(s) with this exception, you may extend this exception to your
+    version of the file(s), but you are not obligated to do so.  If you
+    do not wish to do so, delete this exception statement from your
+    version.  If you delete this exception statement from all source
+    files in the program, then also delete it here.
+*/
+
+#ifndef LIBDCP_SUBTITLE_ASSET_INTERNAL_H
+#define LIBDCP_SUBTITLE_ASSET_INTERNAL_H
+
+#include "raw_convert.h"
+#include "types.h"
+#include "dcp_time.h"
+#include <libxml++/libxml++.h>
+#include <boost/foreach.hpp>
+
+struct take_intersection_test;
+struct take_difference_test;
+struct pull_fonts_test1;
+struct pull_fonts_test2;
+struct pull_fonts_test3;
+
+namespace dcp {
+
+class SubtitleString;
+
+namespace order {
+
+struct Context
+{
+	std::string xmlns () const;
+
+	int time_code_rate;
+	Standard standard;
+	int spot_number;
+};
+
+class Font
+{
+public:
+	Font () {}
+
+	Font (SubtitleString const & s, Standard standard);
+
+	xmlpp::Element* as_xml (xmlpp::Element* parent, Context& context) const;
+
+	void take_intersection (Font other);
+	void take_difference (Font other);
+	bool empty () const;
+	void clear ();
+	bool operator== (Font const & other) const;
+
+private:
+	friend struct ::take_intersection_test;
+	friend struct ::take_difference_test;
+	friend struct ::pull_fonts_test1;
+	friend struct ::pull_fonts_test2;
+	friend struct ::pull_fonts_test3;
+
+	std::map<std::string, std::string> _values;
+};
+
+class Part
+{
+public:
+	Part (boost::shared_ptr<Part> parent_)
+		: parent (parent_)
+	{}
+
+	Part (boost::shared_ptr<Part> parent_, Font font_)
+		: parent (parent_)
+		, font (font_)
+	{}
+
+	virtual xmlpp::Element* as_xml (xmlpp::Element* parent, Context &) const;
+	void write_xml (xmlpp::Element* parent, order::Context& context) const;
+
+	boost::shared_ptr<Part> parent;
+	Font font;
+	std::list<boost::shared_ptr<Part> > children;
+};
+
+class String : public Part
+{
+public:
+	String (boost::shared_ptr<Part> parent, Font font, std::string text_)
+		: Part (parent, font)
+		, text (text_)
+	{}
+
+	virtual xmlpp::Element* as_xml (xmlpp::Element* parent, Context &) const;
+
+	std::string text;
+};
+
+class Text : public Part
+{
+public:
+	Text (boost::shared_ptr<Part> parent, HAlign h_align, float h_position, VAlign v_align, float v_position, Direction direction)
+		: Part (parent)
+		, _h_align (h_align)
+		, _h_position (h_position)
+		, _v_align (v_align)
+		, _v_position (v_position)
+		, _direction (direction)
+	{}
+
+	xmlpp::Element* as_xml (xmlpp::Element* parent, Context& context) const;
+
+private:
+	HAlign _h_align;
+	float _h_position;
+	VAlign _v_align;
+	float _v_position;
+	Direction _direction;
+};
+
+class Subtitle : public Part
+{
+public:
+	Subtitle (boost::shared_ptr<Part> parent, Time in, Time out, Time fade_up, Time fade_down)
+		: Part (parent)
+		, _in (in)
+		, _out (out)
+		, _fade_up (fade_up)
+		, _fade_down (fade_down)
+	{}
+
+	xmlpp::Element* as_xml (xmlpp::Element* parent, Context& context) const;
+
+private:
+	Time _in;
+	Time _out;
+	Time _fade_up;
+	Time _fade_down;
+};
+
+}
+}
+
+#endif
diff --git a/src/wscript b/src/wscript
index 365f3df3..8c35c62c 100644
--- a/src/wscript
+++ b/src/wscript
@@ -92,6 +92,7 @@ def build(bld):
              stereo_picture_asset_writer.cc
              stereo_picture_frame.cc
              subtitle_asset.cc
+             subtitle_asset_internal.cc
              subtitle_string.cc
              transfer_function.cc
              types.cc