Merge branch '1.0' of ssh://main.carlh.net/home/carl/git/libdcp into 1.0

39 files changed, 250 insertions, 102 deletions

diff --git a/examples/make_dcp.cc b/examples/make_dcp.cc
index 7b15e5e0..ae7890a5 100644
--- a/examples/make_dcp.cc
+++ b/examples/make_dcp.cc
@@ -105,8 +105,6 @@ main ()
 	/* Write the DCP */
 	dcp::DCP dcp ("DCP");
 	dcp.add (cpl);
-	dcp.add (picture_asset);
-	dcp.add (sound_asset);
 	dcp.write_xml (dcp::SMPTE);
 	
 	return 0;
diff --git a/run/test/rewrite_subs b/run/test/rewrite_subs
index a4c411d7..6458350f 100755
--- a/run/test/rewrite_subs
+++ b/run/test/rewrite_subs
@@ -1,6 +1,6 @@
 #!/bin/bash
 
-export LD_LIBRARY_PATH=build/src
+export LD_LIBRARY_PATH=build/src:build/asdcplib/src:$LD_LIBRARY_PATH
 if [ "$1" == "--debug" ]; then
     shift
     gdb --args build/test/rewrite_subs "$@"
diff --git a/run/test/subs_in_out b/run/test/subs_in_out
index cd464dac..3fc57ab1 100755
--- a/run/test/subs_in_out
+++ b/run/test/subs_in_out
@@ -1,6 +1,6 @@
 #!/bin/bash
 
-export LD_LIBRARY_PATH=build/src
+export LD_LIBRARY_PATH=build/src:build/asdcplib/src:$LD_LIBRARY_PATH
 if [ "$1" == "--debug" ]; then
     shift
     gdb --args build/test/subs_in_out "$@"
diff --git a/run/tests b/run/tests
index 8f12e536..68af40a8 100755
--- a/run/tests
+++ b/run/tests
@@ -111,8 +111,4 @@ fi
 # and check that they are right
 $dcpinfo -s $private/data/JourneyToJah_TLR-1_F_EN-DE-FR_CH_51_2K_LOK_20140225_DGL_SMPTE_OV >> $work/jah.log
 
-# Rewrite the subs of JourneyToJah... 
-#cp -r $private/data/JourneyToJah_TLR-1_F_EN-DE-FR_CH_51_2K_LOK_20140225_DGL_SMPTE_OV $work/
-#$work/rewrite_subs $WORK/Jou
-
 echo "PASS"
diff --git a/src/dcp.cc b/src/dcp.cc
index abeafdfb..da12e4f9 100644
--- a/src/dcp.cc
+++ b/src/dcp.cc
@@ -39,6 +39,7 @@
 #include "decrypted_kdm.h"
 #include "decrypted_kdm_key.h"
 #include "dcp_assert.h"
+#include "reel_asset.h"
 #include <xmlsec/xmldsig.h>
 #include <xmlsec/app.h>
 #include <libxml++/libxml++.h>
@@ -114,6 +115,12 @@ DCP::read (bool keep_going, ReadErrors* errors)
 	/* XXX: I think we should be looking at the PKL here to decide type, not
 	   the extension of the file.
 	*/
+
+	/* Make a list of non-CPL assets so that we can resolve the references
+	   from the CPL.
+	*/
+	list<shared_ptr<Asset> > other_assets;
+	
 	for (map<string, boost::filesystem::path>::const_iterator i = paths.begin(); i != paths.end(); ++i) {
 		boost::filesystem::path path = _directory / i->second;
 
@@ -122,7 +129,7 @@ DCP::read (bool keep_going, ReadErrors* errors)
 			continue;
 		}
 		
-		if (boost::algorithm::ends_with (path.string(), ".xml")) {
+		if (boost::filesystem::extension (path) == ".xml") {
 			xmlpp::DomParser* p = new xmlpp::DomParser;
 			try {
 				p->parse_file (path.string());
@@ -135,11 +142,11 @@ DCP::read (bool keep_going, ReadErrors* errors)
 			delete p;
 			
 			if (root == "CompositionPlaylist") {
-				_assets.push_back (shared_ptr<CPL> (new CPL (path)));
+				_cpls.push_back (shared_ptr<CPL> (new CPL (path)));
 			} else if (root == "DCSubtitle") {
-				_assets.push_back (shared_ptr<InteropSubtitleAsset> (new InteropSubtitleAsset (path)));
+				other_assets.push_back (shared_ptr<InteropSubtitleAsset> (new InteropSubtitleAsset (path)));
 			}
-		} else if (boost::algorithm::ends_with (path.string(), ".mxf")) {
+		} else if (boost::filesystem::extension (path) == ".mxf") {
 			ASDCP::EssenceType_t type;
 			if (ASDCP::EssenceType (path.string().c_str(), type) != ASDCP::RESULT_OK) {
 				throw DCPReadError ("Could not find essence type");
@@ -149,26 +156,26 @@ DCP::read (bool keep_going, ReadErrors* errors)
 				case ASDCP::ESS_MPEG2_VES:
 					throw DCPReadError ("MPEG2 video essences are not supported");
 				case ASDCP::ESS_JPEG_2000:
-					_assets.push_back (shared_ptr<MonoPictureAsset> (new MonoPictureAsset (path)));
+					other_assets.push_back (shared_ptr<MonoPictureAsset> (new MonoPictureAsset (path)));
 					break;
 				case ASDCP::ESS_PCM_24b_48k:
 				case ASDCP::ESS_PCM_24b_96k:
-					_assets.push_back (shared_ptr<SoundAsset> (new SoundAsset (path)));
+					other_assets.push_back (shared_ptr<SoundAsset> (new SoundAsset (path)));
 					break;
 				case ASDCP::ESS_JPEG_2000_S:
-					_assets.push_back (shared_ptr<StereoPictureAsset> (new StereoPictureAsset (path)));
+					other_assets.push_back (shared_ptr<StereoPictureAsset> (new StereoPictureAsset (path)));
 					break;
 				case ASDCP::ESS_TIMED_TEXT:
-					_assets.push_back (shared_ptr<SMPTESubtitleAsset> (new SMPTESubtitleAsset (path)));
+					other_assets.push_back (shared_ptr<SMPTESubtitleAsset> (new SMPTESubtitleAsset (path)));
 					break;
 				default:
 					throw DCPReadError ("Unknown MXF essence type");
-				}
+			}
 		}
 	}
 
 	BOOST_FOREACH (shared_ptr<CPL> i, cpls ()) {
-		i->resolve_refs (list_of_type<Asset, Object> (assets ()));
+		i->resolve_refs (list_of_type<Asset, Object> (other_assets));
 	}
 }
 
@@ -200,9 +207,9 @@ DCP::equals (DCP const & other, EqualityOptions opt, NoteHandler note) const
 }
 
 void
-DCP::add (boost::shared_ptr<Asset> asset)
+DCP::add (boost::shared_ptr<CPL> cpl)
 {
-	_assets.push_back (asset);
+	_cpls.push_back (cpl);
 }
 
 bool
@@ -260,7 +267,7 @@ DCP::write_pkl (Standard standard, string pkl_uuid, XMLMetadata metadata, shared
 	pkl->add_child("Creator")->add_child_text (metadata.creator);
 
 	xmlpp::Element* asset_list = pkl->add_child("AssetList");
-	BOOST_FOREACH (shared_ptr<Asset> i, _assets) {
+	BOOST_FOREACH (shared_ptr<Asset> i, assets ()) {
 		i->write_to_pkl (asset_list, standard);
 	}
 
@@ -370,7 +377,7 @@ DCP::write_assetmap (Standard standard, string pkl_uuid, int pkl_length, XMLMeta
 	chunk->add_child("Offset")->add_child_text ("0");
 	chunk->add_child("Length")->add_child_text (raw_convert<string> (pkl_length));
 
-	BOOST_FOREACH (shared_ptr<Asset> i, _assets) {
+	BOOST_FOREACH (shared_ptr<Asset> i, assets ()) {
 		i->write_to_assetmap (asset_list, _directory);
 	}
 
@@ -405,5 +412,20 @@ DCP::write_xml (
 list<shared_ptr<CPL> >
 DCP::cpls () const
 {
-	return list_of_type<Asset, CPL> (_assets);
+	return _cpls;
+}
+
+/** @return All assest (including CPLs) */
+list<shared_ptr<Asset> >
+DCP::assets () const
+{
+	list<shared_ptr<Asset> > assets;
+	BOOST_FOREACH (shared_ptr<CPL> i, cpls ()) {
+		assets.push_back (i);
+		BOOST_FOREACH (shared_ptr<const ReelAsset> j, i->reel_assets ()) {
+			assets.push_back (j->asset_ref().object ());
+		}
+	}
+
+	return assets;
 }
diff --git a/src/dcp.h b/src/dcp.h
index 46ae2b34..03f0a802 100644
--- a/src/dcp.h
+++ b/src/dcp.h
@@ -82,14 +82,10 @@ public:
 	 */
 	bool equals (DCP const & other, EqualityOptions options, NoteHandler note) const;
 
-	void add (boost::shared_ptr<Asset> asset);
+	void add (boost::shared_ptr<CPL> cpl);
 
 	std::list<boost::shared_ptr<CPL> > cpls () const;
-
-	/** @return All this DCP's assets (note that CPLs are assets) */
-	std::list<boost::shared_ptr<Asset> > assets () const {
-		return _assets;
-	}
+	std::list<boost::shared_ptr<Asset> > assets () const;
 
 	bool encrypted () const;
 
@@ -123,8 +119,8 @@ private:
 
 	/** the directory that we are writing to */
 	boost::filesystem::path _directory;
-	/** the assets that make up this DCP */
-	std::list<boost::shared_ptr<Asset> > _assets;
+	/** the CPLs that make up this DCP */
+	std::list<boost::shared_ptr<CPL> > _cpls;
 };
 
 }
diff --git a/src/interop_subtitle_asset.cc b/src/interop_subtitle_asset.cc
index 96ccb68a..231dda16 100644
--- a/src/interop_subtitle_asset.cc
+++ b/src/interop_subtitle_asset.cc
@@ -56,10 +56,9 @@ InteropSubtitleAsset::InteropSubtitleAsset (boost::filesystem::path file)
 	parse_subtitles (xml, font_nodes);
 }
 
-InteropSubtitleAsset::InteropSubtitleAsset (string movie_title, string language)
-	: _movie_title (movie_title)
+InteropSubtitleAsset::InteropSubtitleAsset ()
 {
-	_language = language;
+	
 }
 
 Glib::ustring
diff --git a/src/interop_subtitle_asset.h b/src/interop_subtitle_asset.h
index 56350421..378bacc7 100644
--- a/src/interop_subtitle_asset.h
+++ b/src/interop_subtitle_asset.h
@@ -36,7 +36,7 @@ class InteropLoadFontNode;
 class InteropSubtitleAsset : public SubtitleAsset
 {
 public:
-	InteropSubtitleAsset (std::string movie_title, std::string language);
+	InteropSubtitleAsset ();
 	InteropSubtitleAsset (boost::filesystem::path file);
 
 	bool equals (
diff --git a/src/load_font_node.h b/src/load_font_node.h
index 58e5920a..3ff70497 100644
--- a/src/load_font_node.h
+++ b/src/load_font_node.h
@@ -35,6 +35,8 @@ public:
 	LoadFontNode (std::string id_)
 		: id (id_)
 	{}
+
+	virtual ~LoadFontNode () {}
 	
 	std::string id;
 };
diff --git a/src/local_time.cc b/src/local_time.cc
index 32e8e093..09729543 100644
--- a/src/local_time.cc
+++ b/src/local_time.cc
@@ -29,6 +29,7 @@
 #include <cstdio>
 
 using std::string;
+using std::ostream;
 using boost::lexical_cast;
 using namespace dcp;
 
@@ -156,3 +157,24 @@ LocalTime::time_of_day (bool with_millisecond) const
 	}
 	return buffer;
 }
+
+bool
+LocalTime::operator== (LocalTime const & other) const
+{
+	return _year == other._year && _month == other._month && _day == other._day &&
+		_hour == other._hour && _second == other._second && _millisecond == other._millisecond &&
+		_tz_hour == other._tz_hour && _tz_minute == other._tz_minute;
+}
+
+bool
+LocalTime::operator!= (LocalTime const & other) const
+{
+	return !(*this == other);
+}
+
+ostream&
+dcp::operator<< (ostream& s, LocalTime const & t)
+{
+	s << t.as_string ();
+	return s;
+}
diff --git a/src/local_time.h b/src/local_time.h
index b1751f8b..5112307c 100644
--- a/src/local_time.h
+++ b/src/local_time.h
@@ -51,6 +51,9 @@ public:
 	std::string date () const;
 	std::string time_of_day (bool with_millisecond = false) const;
 
+	bool operator== (LocalTime const & other) const;
+	bool operator!= (LocalTime const & other) const;
+	
 private:
 	friend class ::local_time_test;
 
@@ -69,6 +72,9 @@ private:
 	int _tz_minute; ///< minutes by which this time is offset from UTC
 };
 
+std::ostream&
+operator<< (std::ostream& s, LocalTime const & t);
+
 }
 
 #endif
diff --git a/src/mxf.cc b/src/mxf.cc
index 0cde395c..5d90d60e 100644
--- a/src/mxf.cc
+++ b/src/mxf.cc
@@ -36,6 +36,7 @@
 #include <iostream>
 
 using std::string;
+using std::cout;
 using std::list;
 using std::pair;
 using boost::shared_ptr;
@@ -105,8 +106,10 @@ MXF::read_writer_info (ASDCP::WriterInfo const & info)
 {
 	char buffer[64];
 
-	Kumu::bin2UUIDhex (info.CryptographicKeyID, ASDCP::UUIDlen, buffer, sizeof (buffer));
-	_key_id = buffer;
+	if (info.EncryptedEssence) {
+		Kumu::bin2UUIDhex (info.CryptographicKeyID, ASDCP::UUIDlen, buffer, sizeof (buffer));
+		_key_id = buffer;
+	}
 
 	_metadata.read (info);
 	
diff --git a/src/reel_asset.h b/src/reel_asset.h
index 47bae675..e9704acd 100644
--- a/src/reel_asset.h
+++ b/src/reel_asset.h
@@ -59,10 +59,15 @@ public:
 	virtual bool equals (boost::shared_ptr<const ReelAsset>, EqualityOptions, NoteHandler) const;
 
 	/** @return a Ref to our actual asset */
-	Ref<Asset>& asset_ref () {
+	Ref<Asset> const & asset_ref () const {
 		return _asset_ref;
 	}
 
+	/** @return a Ref to our actual asset */
+	Ref<Asset>& asset_ref () {
+		return _asset_ref;
+	}
+	
 	int64_t entry_point () const {
 		return _entry_point;
 	}
diff --git a/src/smpte_subtitle_asset.cc b/src/smpte_subtitle_asset.cc
index eef28e51..69e2712f 100644
--- a/src/smpte_subtitle_asset.cc
+++ b/src/smpte_subtitle_asset.cc
@@ -43,6 +43,12 @@ using boost::split;
 using boost::is_any_of;
 using namespace dcp;
 
+SMPTESubtitleAsset::SMPTESubtitleAsset ()
+	: _time_code_rate (0)
+{
+	
+}
+
 /** Construct a SMPTESubtitleAsset by reading an XML or MXF file.
  *  @param file Filename.
  *  @param mxf true if file is an MXF, false if it is XML.
@@ -177,16 +183,19 @@ SMPTESubtitleAsset::write (boost::filesystem::path p) const
 	memcpy (descriptor.AssetID, writer_info.AssetUUID, ASDCP::UUIDlen);
 	descriptor.ContainerDuration = latest_subtitle_out().as_editable_units (_edit_rate.numerator / _edit_rate.denominator);
 
-	/* XXX: fonts into descriptor? */
+	/* XXX: should write fonts into the file somehow */
 	
 	ASDCP::TimedText::MXFWriter writer;
-	Kumu::Result_t r = writer.OpenWrite (p.string().c_str(), writer_info, descriptor);
+	ASDCP::Result_t r = writer.OpenWrite (p.string().c_str(), writer_info, descriptor);
 	if (ASDCP_FAILURE (r)) {
 		boost::throw_exception (FileError ("could not open subtitle MXF for writing", p.string(), r));
 	}
 
 	/* XXX: no encryption */
-	writer.WriteTimedTextResource (xml_as_string ());
+	r = writer.WriteTimedTextResource (xml_as_string ());
+	if (ASDCP_FAILURE (r)) {
+		boost::throw_exception (MXFFileError ("could not write XML to timed text resource", p.string(), r));
+	}
 
 	writer.Finalize ();
 
diff --git a/src/smpte_subtitle_asset.h b/src/smpte_subtitle_asset.h
index c796e1d4..3fb2571a 100644
--- a/src/smpte_subtitle_asset.h
+++ b/src/smpte_subtitle_asset.h
@@ -36,6 +36,8 @@ class SMPTELoadFontNode;
 class SMPTESubtitleAsset : public SubtitleAsset, public MXF
 {
 public:
+	SMPTESubtitleAsset ();
+	
 	/** @param file File name
 	 *  @param mxf true if `file' is a MXF, or false if it is an XML file.
 	 */
@@ -52,6 +54,22 @@ public:
 	Glib::ustring xml_as_string () const;
 	void write (boost::filesystem::path path) const;
 
+	void set_content_title_text (std::string t) {
+		_content_title_text = t;
+	}
+
+	void set_language (std::string l) {
+		_language = l;
+	}
+
+	void set_edit_rate (Fraction e) {
+		_edit_rate = e;
+	}
+
+	void set_time_code_rate (int t) {
+		_time_code_rate = t;
+	}
+
 	/** @return title of the film that these subtitles are for,
 	 *  to be presented to the user.
 	 */
@@ -74,6 +92,10 @@ public:
 		return _issue_date;
 	}
 
+	boost::optional<int> reel_number () const {
+		return _reel_number;
+	}
+
 	Fraction edit_rate () const {
 		return _edit_rate;
 	}
diff --git a/src/types.cc b/src/types.cc
index 37666701..f32d90bd 100644
--- a/src/types.cc
+++ b/src/types.cc
@@ -62,6 +62,13 @@ dcp::operator!= (Fraction const & a, Fraction const & b)
 	return (a.numerator != b.numerator || a.denominator != b.denominator);
 }
 
+ostream&
+dcp::operator<< (ostream& s, Fraction const & f)
+{
+	s << f.numerator << "/" << f.denominator;
+	return s;
+}
+
 /** Construct a Colour, initialising it to black. */
 Colour::Colour ()
 	: r (0)
diff --git a/src/types.h b/src/types.h
index 16908952..f364572b 100644
--- a/src/types.h
+++ b/src/types.h
@@ -147,6 +147,7 @@ public:
 
 extern bool operator== (Fraction const & a, Fraction const & b);
 extern bool operator!= (Fraction const & a, Fraction const & b);
+extern std::ostream& operator<< (std::ostream& s, Fraction const & f);	
 
 /** @struct EqualityOptions
  *  @brief  A class to describe what "equality" means for a particular test.
diff --git a/test/certificates_test.cc b/test/certificates_test.cc
index dc08a193..374dbc13 100644
--- a/test/certificates_test.cc
+++ b/test/certificates_test.cc
@@ -26,6 +26,7 @@ using std::list;
 using std::string;
 using boost::shared_ptr;
 
+/** Check that loading certificates from files via strings works */
 BOOST_AUTO_TEST_CASE (certificates)
 {
 	dcp::CertificateChain c;
diff --git a/test/colour_conversion_test.cc b/test/colour_conversion_test.cc
index 2eb5edf5..7ff93468 100644
--- a/test/colour_conversion_test.cc
+++ b/test/colour_conversion_test.cc
@@ -54,6 +54,7 @@ check_modified_gamma (shared_ptr<const TransferFunction> tf, int bit_depth, bool
 	}
 }
 
+/** Check that the gamma correction LUTs are right for sRGB */
 BOOST_AUTO_TEST_CASE (colour_conversion_test1)
 {
 	ColourConversion cc = ColourConversion::srgb_to_xyz ();
@@ -67,6 +68,7 @@ BOOST_AUTO_TEST_CASE (colour_conversion_test1)
 	check_gamma (cc.out(), 16, true, 1 / 2.6);
 }
 
+/** Check that the gamma correction LUTs are right for REC709 */
 BOOST_AUTO_TEST_CASE (colour_conversion_test2)
 {
 	ColourConversion cc = ColourConversion::rec709_to_xyz ();
diff --git a/test/colour_test.cc b/test/colour_test.cc
index f2be8ead..cebe95ab 100644
--- a/test/colour_test.cc
+++ b/test/colour_test.cc
@@ -23,7 +23,7 @@
 
 using std::stringstream;
 
-/* Check that dcp::Colour works */
+/** Check that dcp::Colour works */
 BOOST_AUTO_TEST_CASE (colour)
 {
 	dcp::Colour z;
diff --git a/test/cpl_sar_test.cc b/test/cpl_sar_test.cc
index 691439db..cafa589f 100644
--- a/test/cpl_sar_test.cc
+++ b/test/cpl_sar_test.cc
@@ -25,9 +25,9 @@
 
 using boost::shared_ptr;
 
-/* Test for a reported bug where <ScreenAspectRatio> in Interop files uses
-   excessive decimal places and (sometimes) the wrong decimal point character.
-*/
+/** Test for a reported bug where <ScreenAspectRatio> in Interop files uses
+ *  excessive decimal places and (sometimes) the wrong decimal point character.
+ */
 BOOST_AUTO_TEST_CASE (cpl_sar)
 {
 	shared_ptr<dcp::ReelMonoPictureAsset> pa (
diff --git a/test/dcp_test.cc b/test/dcp_test.cc
index c4950704..0338e1f8 100644
--- a/test/dcp_test.cc
+++ b/test/dcp_test.cc
@@ -101,8 +101,6 @@ BOOST_AUTO_TEST_CASE (dcp_test1)
 			  ));
 		  
 	d.add (cpl);
-	d.add (mp);
-	d.add (ms);
 
 	d.write_xml (dcp::SMPTE, xml_meta);
 
@@ -175,8 +173,6 @@ BOOST_AUTO_TEST_CASE (dcp_test2)
 			  ));
 		  
 	d.add (cpl);
-	d.add (mp);
-	d.add (ms);
 
 	d.write_xml (dcp::SMPTE, xml_meta);
 
diff --git a/test/encryption_test.cc b/test/encryption_test.cc
index 299e5f9b..7786292d 100644
--- a/test/encryption_test.cc
+++ b/test/encryption_test.cc
@@ -41,9 +41,9 @@
 
 using boost::shared_ptr;
 
-/* Load a certificate chain from build/test/data/ *.pem and then build
-   an encrypted DCP and a KDM using it.
-*/
+/** Load a certificate chain from build/test/data/ *.pem and then build
+ *  an encrypted DCP and a KDM using it.
+ */
 BOOST_AUTO_TEST_CASE (encryption_test)
 {
 	boost::filesystem::remove_all ("build/test/signer");
@@ -124,8 +124,6 @@ BOOST_AUTO_TEST_CASE (encryption_test)
 	cpl->set_metadata (xml_metadata);
 	
 	d.add (cpl);
-	d.add (mp);
-	d.add (ms);
 
 	d.write_xml (dcp::SMPTE, xml_metadata, signer);
 
diff --git a/test/exception_test.cc b/test/exception_test.cc
index 740154dd..dd9608ee 100644
--- a/test/exception_test.cc
+++ b/test/exception_test.cc
@@ -22,6 +22,7 @@
 
 using std::string;
 
+/** Test exception classes */
 BOOST_AUTO_TEST_CASE (exception_test)
 {
 	BOOST_CHECK_EQUAL (string (dcp::FileError ("foo", "bar", 42).what()), "foo (bar) (error 42)");
diff --git a/test/fraction_test.cc b/test/fraction_test.cc
index 445805d1..efdba664 100644
--- a/test/fraction_test.cc
+++ b/test/fraction_test.cc
@@ -21,6 +21,7 @@
 #include "exceptions.h"
 #include <boost/test/unit_test.hpp>
 
+/** Test dcp::Fraction */
 BOOST_AUTO_TEST_CASE (fraction_test)
 {
 	dcp::Fraction f (42, 26);
diff --git a/test/interop_load_font_test.cc b/test/interop_load_font_test.cc
index 9940c5e0..04520080 100644
--- a/test/interop_load_font_test.cc
+++ b/test/interop_load_font_test.cc
@@ -22,6 +22,7 @@
 #include <libxml++/libxml++.h>
 #include <boost/test/unit_test.hpp>
 
+/** Test dcp::InteropLoadFont's simple constructor */
 BOOST_AUTO_TEST_CASE (interop_load_font_test1)
 {
 	dcp::InteropLoadFontNode lf ("my-great-id", "my-great-uri");
@@ -29,6 +30,7 @@ BOOST_AUTO_TEST_CASE (interop_load_font_test1)
 	BOOST_CHECK_EQUAL (lf.uri, "my-great-uri");
 }
 
+/** Test dcp::InteropLoadFont's XML constructor */
 BOOST_AUTO_TEST_CASE (interop_load_font_test2)
 {
 	xmlpp::Document doc;
diff --git a/test/kdm_test.cc b/test/kdm_test.cc
index c2d40b3b..4051a44a 100644
--- a/test/kdm_test.cc
+++ b/test/kdm_test.cc
@@ -27,6 +27,7 @@ using std::list;
 using std::stringstream;
 using boost::shared_ptr;
 
+/** Check reading and decryption of a KDM */
 BOOST_AUTO_TEST_CASE (kdm_test)
 {
 	dcp::DecryptedKDM kdm (
@@ -49,7 +50,7 @@ BOOST_AUTO_TEST_CASE (kdm_test)
 	BOOST_CHECK_EQUAL (keys.back().key().hex(), "5327fb7ec2e807bd57059615bf8a169d");
 }
 
-/* Check that we can read in a KDM and then write it back out again the same */
+/** Check that we can read in a KDM and then write it back out again the same */
 BOOST_AUTO_TEST_CASE (kdm_passthrough_test)
 {
 	dcp::EncryptedKDM kdm (
diff --git a/test/read_dcp_test.cc b/test/read_dcp_test.cc
index 6aed62a4..097d0aa9 100644
--- a/test/read_dcp_test.cc
+++ b/test/read_dcp_test.cc
@@ -24,7 +24,7 @@
 using std::list;
 using boost::shared_ptr;
 
-/* Read DCP that is in git and make sure that basic stuff is read in correctly */
+/** Read DCP that is in git and make sure that basic stuff is read in correctly */
 BOOST_AUTO_TEST_CASE (read_dcp)
 {
 	dcp::DCP d ("test/ref/DCP/dcp_test1");
diff --git a/test/read_subtitle_test.cc b/test/read_interop_subtitle_test.cc
index c49ddfdf..b268d629 100644
--- a/test/read_subtitle_test.cc
+++ b/test/read_interop_subtitle_test.cc
@@ -18,20 +18,32 @@
 */
 
 #include "interop_subtitle_asset.h"
+#include "interop_load_font_node.h"
 #include "subtitle_string.h"
 #include <boost/test/unit_test.hpp>
 
 using std::list;
 using std::string;
 using boost::shared_ptr;
+using boost::dynamic_pointer_cast;
 
-/* Load some subtitle content from XML and check that it is read correctly */
-BOOST_AUTO_TEST_CASE (read_subtitle_test1)
+/** Load some subtitle content from Interop XML and check that it is read correctly */
+BOOST_AUTO_TEST_CASE (read_interop_subtitle_test1)
 {
 	dcp::InteropSubtitleAsset subs ("test/data/subs1.xml");
 
+	BOOST_CHECK_EQUAL (subs.id(), "cab5c268-222b-41d2-88ae-6d6999441b17");
+	BOOST_CHECK_EQUAL (subs.movie_title(), "Movie Title");
+	BOOST_CHECK_EQUAL (subs.reel_number(), "1");
 	BOOST_CHECK_EQUAL (subs.language(), "French");
 
+	list<shared_ptr<dcp::LoadFontNode> > lfn = subs.load_font_nodes ();
+	BOOST_REQUIRE_EQUAL (lfn.size(), 1);
+	shared_ptr<dcp::InteropLoadFontNode> interop_lfn = dynamic_pointer_cast<dcp::InteropLoadFontNode> (lfn.front ());
+	BOOST_REQUIRE (interop_lfn);
+	BOOST_CHECK_EQUAL (interop_lfn->id, "theFontId");
+	BOOST_CHECK_EQUAL (interop_lfn->uri, "arial.ttf");
+
 	list<dcp::SubtitleString> s = subs.subtitles_during (dcp::Time (0, 0, 6, 1, 250), dcp::Time (0, 0, 6, 2, 250));
 	BOOST_REQUIRE_EQUAL (s.size(), 1);
 	BOOST_CHECK_EQUAL (s.front(), dcp::SubtitleString (
@@ -136,7 +148,7 @@ BOOST_AUTO_TEST_CASE (read_subtitle_test1)
 }
 
 /** And similarly for another one */
-BOOST_AUTO_TEST_CASE (read_subtitle_test2)
+BOOST_AUTO_TEST_CASE (read_interop_subtitle_test2)
 {
 	dcp::InteropSubtitleAsset subs ("test/data/subs2.xml");
 
diff --git a/test/read_smpte_subtitle_test.cc b/test/read_smpte_subtitle_test.cc
new file mode 100644
index 00000000..81749a15
--- /dev/null
+++ b/test/read_smpte_subtitle_test.cc
@@ -0,0 +1,60 @@
+/*
+    Copyright (C) 2015 Carl Hetherington <cth@carlh.net>
+
+    This program 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.
+
+    This program 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 this program; if not, write to the Free Software
+    Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+
+*/
+
+#include "smpte_subtitle_asset.h"
+#include "test.h"
+#include "local_time.h"
+#include "smpte_load_font_node.h"
+#include <boost/test/unit_test.hpp>
+
+using std::list;
+using boost::shared_ptr;
+using boost::dynamic_pointer_cast;
+
+/** Check reading of a SMPTE subtitle file */
+BOOST_AUTO_TEST_CASE (read_smpte_subtitle_test)
+{
+	dcp::SMPTESubtitleAsset sc (private_test / "8dfafe11-2bd1-4206-818b-afc109cfe7f6_reel1.xml", false);
+
+	BOOST_CHECK_EQUAL (sc.id(), "8dfafe11-2bd1-4206-818b-afc109cfe7f6");
+	BOOST_CHECK_EQUAL (sc.content_title_text(), "Violet");
+	BOOST_REQUIRE (sc.annotation_text());
+	BOOST_CHECK_EQUAL (sc.annotation_text().get(), "Violet");
+	BOOST_CHECK_EQUAL (sc.issue_date(), dcp::LocalTime ("2014-12-23T22:30:07.000-00:00"));
+	BOOST_REQUIRE (sc.reel_number());
+	BOOST_CHECK_EQUAL (sc.reel_number().get(), 1);
+	BOOST_REQUIRE (sc.language ());
+	BOOST_CHECK_EQUAL (sc.language().get (), "Dutch");
+	BOOST_CHECK_EQUAL (sc.edit_rate(), dcp::Fraction (24, 1));
+	BOOST_CHECK_EQUAL (sc.time_code_rate(), 24);
+	BOOST_CHECK_EQUAL (sc.start_time(), dcp::Time (0, 0, 0, 23, 24));
+	list<shared_ptr<dcp::LoadFontNode> > lfn = sc.load_font_nodes ();
+	BOOST_REQUIRE_EQUAL (lfn.size(), 1);
+	shared_ptr<dcp::SMPTELoadFontNode> smpte_lfn = dynamic_pointer_cast<dcp::SMPTELoadFontNode> (lfn.front ());
+	BOOST_REQUIRE (smpte_lfn);
+	BOOST_CHECK_EQUAL (smpte_lfn->id, "theFontId");
+	BOOST_CHECK_EQUAL (smpte_lfn->urn, "3dec6dc0-39d0-498d-97d0-928d2eb78391");
+	BOOST_REQUIRE_EQUAL (sc.subtitles().size(), 159);
+	BOOST_CHECK_EQUAL (sc.subtitles().front().text(), "Jonas ?");
+	BOOST_CHECK_EQUAL (sc.subtitles().front().in(), dcp::Time (0, 7, 6, 20, 24));
+	BOOST_CHECK_EQUAL (sc.subtitles().front().out(), dcp::Time (0, 7, 7, 20, 24));
+	BOOST_CHECK_EQUAL (sc.subtitles().back().text(), "Come on.");
+	BOOST_CHECK_EQUAL (sc.subtitles().back().in(), dcp::Time (1, 13, 37, 11, 24));
+	BOOST_CHECK_EQUAL (sc.subtitles().back().out(), dcp::Time (1, 13, 38, 11, 24));
+}
diff --git a/test/recovery_test.cc b/test/recovery_test.cc
index 257c512d..754a49e2 100644
--- a/test/recovery_test.cc
+++ b/test/recovery_test.cc
@@ -26,7 +26,7 @@
 using std::string;
 using boost::shared_ptr;
 
-/* Check that recovery from a partially-written MXF works */
+/** Check that recovery from a partially-written MXF works */
 BOOST_AUTO_TEST_CASE (recovery)
 {
 	Kumu::libdcp_test = true;
diff --git a/test/rewrite_subs.cc b/test/rewrite_subs.cc
index 5bfe126f..6aa259a9 100644
--- a/test/rewrite_subs.cc
+++ b/test/rewrite_subs.cc
@@ -32,6 +32,7 @@ using std::string;
 using boost::shared_ptr;
 using namespace dcp;
 
+/** Load a DCP then re-write its subtitle XML or MXF in-place */
 int
 main (int argc, char* argv[])
 {
diff --git a/test/round_trip_test.cc b/test/round_trip_test.cc
index a719ff45..39db697b 100644
--- a/test/round_trip_test.cc
+++ b/test/round_trip_test.cc
@@ -43,7 +43,7 @@ using std::list;
 using boost::shared_ptr;
 using boost::scoped_array;
 
-/* Build an encrypted picture asset and a KDM for it and check that the KDM can be decrypted */
+/** Build an encrypted picture asset and a KDM for it and check that the KDM can be decrypted */
 BOOST_AUTO_TEST_CASE (round_trip_test)
 {
 	shared_ptr<dcp::Signer> signer (new dcp::Signer ("openssl"));
diff --git a/test/smpte_load_font_test.cc b/test/smpte_load_font_test.cc
index 77020b98..da52ed97 100644
--- a/test/smpte_load_font_test.cc
+++ b/test/smpte_load_font_test.cc
@@ -22,6 +22,7 @@
 #include <libxml++/libxml++.h>
 #include <boost/test/unit_test.hpp>
 
+/** Test dcp::SMPTELoadFontNode */
 BOOST_AUTO_TEST_CASE (smpte_load_font_test1)
 {
 	xmlpp::Document doc;
diff --git a/test/smpte_subtitle_test.cc b/test/smpte_subtitle_test.cc
deleted file mode 100644
index 2b68de3d..00000000
--- a/test/smpte_subtitle_test.cc
+++ /dev/null
@@ -1,33 +0,0 @@
-/*
-    Copyright (C) 2015 Carl Hetherington <cth@carlh.net>
-
-    This program 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.
-
-    This program 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 this program; if not, write to the Free Software
-    Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
-
-*/
-
-#include "smpte_subtitle_asset.h"
-#include "test.h"
-#include <boost/test/unit_test.hpp>
-
-/** Load a SMPTE XML subtitle file */
-BOOST_AUTO_TEST_CASE (smpte_subtitle_test)
-{
-	dcp::SMPTESubtitleAsset sc (private_test / "8dfafe11-2bd1-4206-818b-afc109cfe7f6_reel1.xml", false);
-
-	BOOST_REQUIRE_EQUAL (sc.id(), "8dfafe11-2bd1-4206-818b-afc109cfe7f6");
-	BOOST_REQUIRE_EQUAL (sc.subtitles().size(), 159);
-	BOOST_REQUIRE_EQUAL (sc.subtitles().front().text(), "Jonas ?");
-	BOOST_REQUIRE_EQUAL (sc.subtitles().back().text(), "Come on.");
-}
diff --git a/test/subs_in_out.cc b/test/subs_in_out.cc
index 2a662262..fce0bfec 100644
--- a/test/subs_in_out.cc
+++ b/test/subs_in_out.cc
@@ -18,18 +18,30 @@
 */
 
 #include "interop_subtitle_asset.h"
+#include "smpte_subtitle_asset.h"
 #include <iostream>
 
 using namespace std;
 
-int main (int argc, char* argv[])
+int
+main (int argc, char* argv[])
 {
-	if (argc < 2) {
+	if (argc != 2) {
 		cerr << "Syntax: " << argv[0] << " <subtitle file>\n";
 		exit (EXIT_FAILURE);
 	}
-	
-	dcp::InteropSubtitleAsset s (argv[1]);
-	cout << s.xml_as_string ();
+
+	try {
+		dcp::InteropSubtitleAsset sc (argv[1]);
+		cout << sc.xml_as_string ();
+	} catch (exception& e) {
+		cerr << "Could not load as interop: " << e.what() << "\n";
+		try {
+			dcp::SMPTESubtitleAsset sc (argv[1]);
+			cout << sc.xml_as_string().raw ();
+		} catch (exception& e) {
+			cerr << "Could not load as SMPTE (" << e.what() << ")\n";
+		}
+	}
 	return 0;
 }
diff --git a/test/util_test.cc b/test/util_test.cc
index 34dc0c18..d7c41a6a 100644
--- a/test/util_test.cc
+++ b/test/util_test.cc
@@ -24,6 +24,7 @@
 using std::ifstream;
 using std::string;
 
+/** Test dcp::base64_decode */
 BOOST_AUTO_TEST_CASE (base64_decode_test)
 {
 	int const N = 256;
@@ -55,6 +56,7 @@ BOOST_AUTO_TEST_CASE (base64_decode_test)
 	}
 }
 
+/** Test dcp::content_kind_from_string */
 BOOST_AUTO_TEST_CASE (content_kind_test)
 {
 	BOOST_CHECK_EQUAL (dcp::content_kind_from_string ("feature"), dcp::FEATURE);
@@ -71,6 +73,7 @@ BOOST_AUTO_TEST_CASE (content_kind_test)
 	BOOST_CHECK_EQUAL (dcp::content_kind_from_string ("advertisement"), dcp::ADVERTISEMENT);
 }
 
+/** Test dcp::relative_to_root */
 BOOST_AUTO_TEST_CASE (relative_to_root_test)
 {
 	{
diff --git a/test/write_subtitle_test.cc b/test/write_subtitle_test.cc
index 958fc981..577eadec 100644
--- a/test/write_subtitle_test.cc
+++ b/test/write_subtitle_test.cc
@@ -29,8 +29,10 @@ using boost::shared_ptr;
 /* Write some subtitle content as Interop XML and check that it is right */
 BOOST_AUTO_TEST_CASE (write_subtitle_test)
 {
-	dcp::InteropSubtitleAsset c ("Test", "EN");
+	dcp::InteropSubtitleAsset c;
 	c.set_reel_number ("1");
+	c.set_language ("EN");
+	c.set_movie_title ("Test");
 
 	c.add (
 		dcp::SubtitleString (
diff --git a/test/wscript b/test/wscript
index 30015939..f640f6fc 100644
--- a/test/wscript
+++ b/test/wscript
@@ -40,12 +40,12 @@ def build(bld):
                  local_time_test.cc
                  kdm_test.cc
                  read_dcp_test.cc
-                 read_subtitle_test.cc
+                 read_interop_subtitle_test.cc
+                 read_smpte_subtitle_test.cc
                  recovery_test.cc
                  rgb_xyz_test.cc
                  round_trip_test.cc
                  smpte_load_font_test.cc
-                 smpte_subtitle_test.cc
                  test.cc 
                  text_test.cc
                  util_test.cc