Hopefully-correct PKL and AssetMap when using Interop PNG subtitles.

13 files changed, 127 insertions, 19 deletions

diff --git a/src/asset.cc b/src/asset.cc
index 24fdfe6c..5b64b6cc 100644
--- a/src/asset.cc
+++ b/src/asset.cc
@@ -101,10 +101,15 @@ void
 Asset::write_to_assetmap (xmlpp::Node* node, boost::filesystem::path root) const
 {
 	DCP_ASSERT (_file);
+	write_file_to_assetmap (node, root, _file.get(), _id);
+}
 
+void
+Asset::write_file_to_assetmap (xmlpp::Node* node, boost::filesystem::path root, boost::filesystem::path file, string id)
+{
 	optional<boost::filesystem::path> path = relative_to_root (
 		boost::filesystem::canonical (root),
-		boost::filesystem::canonical (_file.get())
+		boost::filesystem::canonical (file)
 		);
 
 	if (!path) {
@@ -115,14 +120,14 @@ Asset::write_to_assetmap (xmlpp::Node* node, boost::filesystem::path root) const
 	}
 
 	xmlpp::Node* asset = node->add_child ("Asset");
-	asset->add_child("Id")->add_child_text ("urn:uuid:" + _id);
+	asset->add_child("Id")->add_child_text ("urn:uuid:" + id);
 	xmlpp::Node* chunk_list = asset->add_child ("ChunkList");
 	xmlpp::Node* chunk = chunk_list->add_child ("Chunk");
 
 	chunk->add_child("Path")->add_child_text (path.get().generic_string());
 	chunk->add_child("VolumeIndex")->add_child_text ("1");
 	chunk->add_child("Offset")->add_child_text ("0");
-	chunk->add_child("Length")->add_child_text (raw_convert<string> (boost::filesystem::file_size (_file.get())));
+	chunk->add_child("Length")->add_child_text (raw_convert<string> (boost::filesystem::file_size (file)));
 }
 
 string
diff --git a/src/asset.h b/src/asset.h
index f69be5a1..b6ab8fd2 100644
--- a/src/asset.h
+++ b/src/asset.h
@@ -76,9 +76,9 @@ public:
 	/** Write details of the asset to a ASSETMAP.
 	 *  @param node Parent node.
 	 */
-	void write_to_assetmap (xmlpp::Node* node, boost::filesystem::path root) const;
+	virtual void write_to_assetmap (xmlpp::Node* node, boost::filesystem::path root) const;
 
-	void add_to_pkl (boost::shared_ptr<PKL> pkl, boost::filesystem::path root) const;
+	virtual void add_to_pkl (boost::shared_ptr<PKL> pkl, boost::filesystem::path root) const;
 
 	/** @return the most recent disk file used to read or write this asset, if there is one */
 	boost::optional<boost::filesystem::path> file () const {
@@ -97,6 +97,8 @@ protected:
 	/** The most recent disk file used to read or write this asset */
 	mutable boost::optional<boost::filesystem::path> _file;
 
+	static void write_file_to_assetmap (xmlpp::Node* node, boost::filesystem::path root, boost::filesystem::path file, std::string id);
+
 private:
 	friend struct ::asset_test;
 
diff --git a/src/dcp.cc b/src/dcp.cc
index 58d05651..fa8ffabf 100644
--- a/src/dcp.cc
+++ b/src/dcp.cc
@@ -162,7 +162,6 @@ DCP::read (bool keep_going, ReadErrors* errors, bool ignore_incorrect_picture_mx
 			break;
 		}
 		}
-
 	}
 
 	if (!pkl_path) {
@@ -261,6 +260,8 @@ DCP::read (bool keep_going, ReadErrors* errors, bool ignore_incorrect_picture_mx
 			}
 		} else if (pkl_type == FontAsset::static_pkl_type(*_standard)) {
 			other_assets.push_back (shared_ptr<FontAsset> (new FontAsset (i->first, path)));
+		} else if (pkl_type == "image/png") {
+			/* It's an Interop PNG subtitle; let it go */
 		} else {
 			throw DCPReadError (String::compose("Unknown asset type %1 in PKL", pkl_type));
 		}
diff --git a/src/interop_subtitle_asset.cc b/src/interop_subtitle_asset.cc
index e2873fb6..d250b752 100644
--- a/src/interop_subtitle_asset.cc
+++ b/src/interop_subtitle_asset.cc
@@ -83,7 +83,7 @@ InteropSubtitleAsset::InteropSubtitleAsset (boost::filesystem::path file)
 	BOOST_FOREACH (shared_ptr<Subtitle> i, _subtitles) {
 		shared_ptr<SubtitleImage> si = dynamic_pointer_cast<SubtitleImage>(i);
 		if (si) {
-			si->set_png_image (Data (file.parent_path() / String::compose("%1.png", si->id())));
+			si->read_png_file (file.parent_path() / String::compose("%1.png", si->id()));
 		}
 	}
 }
@@ -189,7 +189,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) {
-			im->png_image().write (p.parent_path() / String::compose("%1.png", im->id()));
+			im->write_png_file(p.parent_path() / String::compose("%1.png", im->id()));
 		}
 	}
 
@@ -249,3 +249,31 @@ InteropSubtitleAsset::add_font_assets (list<shared_ptr<Asset> >& assets)
 		assets.push_back (shared_ptr<FontAsset> (new FontAsset (i.uuid, i.file.get ())));
 	}
 }
+
+void
+InteropSubtitleAsset::write_to_assetmap (xmlpp::Node* node, boost::filesystem::path root) const
+{
+	Asset::write_to_assetmap (node, root);
+
+	BOOST_FOREACH (shared_ptr<dcp::Subtitle> i, _subtitles) {
+		shared_ptr<dcp::SubtitleImage> im = dynamic_pointer_cast<dcp::SubtitleImage> (i);
+		if (im) {
+			DCP_ASSERT (im->file());
+			write_file_to_assetmap (node, root, im->file().get(), im->id());
+		}
+	}
+}
+
+void
+InteropSubtitleAsset::add_to_pkl (shared_ptr<PKL> pkl, boost::filesystem::path root) const
+{
+	Asset::add_to_pkl (pkl, root);
+
+	BOOST_FOREACH (shared_ptr<dcp::Subtitle> i, _subtitles) {
+		shared_ptr<dcp::SubtitleImage> im = dynamic_pointer_cast<dcp::SubtitleImage> (i);
+		if (im) {
+			Data png_image = im->png_image ();
+			pkl->add_asset (im->id(), optional<string>(), make_digest(png_image), png_image.size(), "image/png");
+		}
+	}
+}
diff --git a/src/interop_subtitle_asset.h b/src/interop_subtitle_asset.h
index 703229a7..ba8b5edf 100644
--- a/src/interop_subtitle_asset.h
+++ b/src/interop_subtitle_asset.h
@@ -59,6 +59,9 @@ public:
 		NoteHandler note
 		) const;
 
+	void write_to_assetmap (xmlpp::Node* node, boost::filesystem::path root) const;
+	void add_to_pkl (boost::shared_ptr<PKL> pkl, boost::filesystem::path root) const;
+
 	std::list<boost::shared_ptr<LoadFontNode> > load_font_nodes () const;
 
 	void add_font (std::string load_id, boost::filesystem::path file);
diff --git a/src/subtitle_image.cc b/src/subtitle_image.cc
index 49c00f77..e1f123bc 100644
--- a/src/subtitle_image.cc
+++ b/src/subtitle_image.cc
@@ -75,6 +75,20 @@ SubtitleImage::SubtitleImage (
 
 }
 
+void
+SubtitleImage::read_png_file (boost::filesystem::path file)
+{
+	_file = file;
+	_png_image = Data (file);
+}
+
+void
+SubtitleImage::write_png_file (boost::filesystem::path file) const
+{
+	_file = file;
+	png_image().write (file);
+}
+
 bool
 dcp::operator== (SubtitleImage const & a, SubtitleImage const & b)
 {
diff --git a/src/subtitle_image.h b/src/subtitle_image.h
index 062df56b..3c4b3fe8 100644
--- a/src/subtitle_image.h
+++ b/src/subtitle_image.h
@@ -82,17 +82,26 @@ public:
 		return _png_image;
 	}
 
-	void set_png_image (Data d) {
-		_png_image = d;
+	void set_png_image (Data png) {
+		_png_image = png;
 	}
 
+	void read_png_file (boost::filesystem::path file);
+	void write_png_file (boost::filesystem::path file) const;
+
 	std::string id () const {
 		return _id;
 	}
 
+	/** @return the most recent disk file used to read or write this asset, if there is one */
+	boost::optional<boost::filesystem::path> file () const {
+		return _file;
+	}
+
 private:
 	Data _png_image;
 	std::string _id;
+	mutable boost::optional<boost::filesystem::path> _file;
 };
 
 bool operator== (SubtitleImage const & a, SubtitleImage const & b);
diff --git a/src/util.cc b/src/util.cc
index ac2246c3..3d90ccaa 100644
--- a/src/util.cc
+++ b/src/util.cc
@@ -88,6 +88,17 @@ dcp::make_uuid ()
 	return string (buffer);
 }
 
+string
+dcp::make_digest (Data data)
+{
+	SHA_CTX sha;
+	SHA1_Init (&sha);
+	SHA1_Update (&sha, data.data().get(), data.size());
+	byte_t byte_buffer[SHA_DIGEST_LENGTH];
+	SHA1_Final (byte_buffer, &sha);
+	char digest[64];
+	return Kumu::base64encode (byte_buffer, SHA_DIGEST_LENGTH, digest, 64);
+}
 
 /** Create a digest for a file.
  *  @param filename File name.
diff --git a/src/util.h b/src/util.h
index 20ea5ac5..9803ad03 100644
--- a/src/util.h
+++ b/src/util.h
@@ -60,6 +60,7 @@ class OpenJPEGImage;
 
 extern std::string make_uuid ();
 extern std::string make_digest (boost::filesystem::path filename, boost::function<void (float)>);
+extern std::string make_digest (Data data);
 extern std::string content_kind_to_string (ContentKind kind);
 extern ContentKind content_kind_from_string (std::string kind);
 extern bool empty_or_white_space (std::string s);
diff --git a/test/ref/write_interop_subtitle_test3/ASSETMAP b/test/ref/write_interop_subtitle_test3/ASSETMAP
new file mode 100644
index 00000000..82fe845a
--- /dev/null
+++ b/test/ref/write_interop_subtitle_test3/ASSETMAP
@@ -0,0 +1,2 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<AssetMap xmlns="http://www.digicine.com/PROTO-ASDCP-AM-20040311#"><Id>urn:uuid:5bdc7da4-eaf1-43e6-854f-7a1175edf94a</Id><AnnotationText>Created by libdcp1.6.1devel</AnnotationText><VolumeCount>1</VolumeCount><IssueDate>2018-09-02T04:45:18+00:00</IssueDate><Issuer>libdcp1.6.1devel</Issuer><Creator>libdcp1.6.1devel</Creator><AssetList><Asset><Id>urn:uuid:8d98d2e1-d2a1-458f-b96b-295e5b5d0860</Id><PackingList>true</PackingList><ChunkList><Chunk><Path>pkl_8d98d2e1-d2a1-458f-b96b-295e5b5d0860.xml</Path><VolumeIndex>1</VolumeIndex><Offset>0</Offset><Length>993</Length></Chunk></ChunkList></Asset><Asset><Id>urn:uuid:9f134b47-ab8d-4c3c-adbf-f494b1544283</Id><ChunkList><Chunk><Path>cpl_9f134b47-ab8d-4c3c-adbf-f494b1544283.xml</Path><VolumeIndex>1</VolumeIndex><Offset>0</Offset><Length>970</Length></Chunk></ChunkList></Asset><Asset><Id>urn:uuid:a6c58cff-3e1e-4b38-acec-a42224475ef6</Id><ChunkList><Chunk><Path>subs.xml</Path><VolumeIndex>1</VolumeIndex><Offset>0</Offset><Length>414</Length></Chunk></ChunkList></Asset><Asset><Id>urn:uuid:822bd341-c751-45b1-94d2-410e4ffcff1b</Id><ChunkList><Chunk><Path>822bd341-c751-45b1-94d2-410e4ffcff1b.png</Path><VolumeIndex>1</VolumeIndex><Offset>0</Offset><Length>44935</Length></Chunk></ChunkList></Asset></AssetList></AssetMap>
diff --git a/test/ref/write_interop_subtitle_test3/pkl.xml b/test/ref/write_interop_subtitle_test3/pkl.xml
new file mode 100644
index 00000000..995fabdb
--- /dev/null
+++ b/test/ref/write_interop_subtitle_test3/pkl.xml
@@ -0,0 +1,2 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<PackingList xmlns="http://www.digicine.com/PROTO-ASDCP-PKL-20040311#"><Id>urn:uuid:8d98d2e1-d2a1-458f-b96b-295e5b5d0860</Id><AnnotationText>Created by libdcp1.6.1devel</AnnotationText><IssueDate>2018-09-02T04:45:18+00:00</IssueDate><Issuer>libdcp1.6.1devel</Issuer><Creator>libdcp1.6.1devel</Creator><AssetList><Asset><Id>urn:uuid:9f134b47-ab8d-4c3c-adbf-f494b1544283</Id><AnnotationText>9f134b47-ab8d-4c3c-adbf-f494b1544283</AnnotationText><Hash>X3ymjtLWzUZ0Av4L7y2/2X3wlB0=</Hash><Size>970</Size><Type>text/xml;asdcpKind=CPL</Type></Asset><Asset><Id>urn:uuid:a6c58cff-3e1e-4b38-acec-a42224475ef6</Id><AnnotationText>a6c58cff-3e1e-4b38-acec-a42224475ef6</AnnotationText><Hash>+k1Lnon0dJab5s8+wucSdGVy3Go=</Hash><Size>414</Size><Type>text/xml;asdcpKind=Subtitle</Type></Asset><Asset><Id>urn:uuid:822bd341-c751-45b1-94d2-410e4ffcff1b</Id><Hash>w0Xc4TUYpao08I0yPSDfFkaEwdg=</Hash><Size>44935</Size><Type>image/png</Type></Asset></AssetList></PackingList>
diff --git a/test/data/write_interop_subtitle_test3.xml b/test/ref/write_interop_subtitle_test3/subs.xml
index 7ba72e47..7ba72e47 100644
--- a/test/data/write_interop_subtitle_test3.xml
+++ b/test/ref/write_interop_subtitle_test3/subs.xml
diff --git a/test/write_subtitle_test.cc b/test/write_subtitle_test.cc
index cf595faf..e57b0e7f 100644
--- a/test/write_subtitle_test.cc
+++ b/test/write_subtitle_test.cc
@@ -22,6 +22,10 @@
 #include "subtitle_string.h"
 #include "subtitle_image.h"
 #include "subtitle_asset_internal.h"
+#include "reel_subtitle_asset.h"
+#include "reel.h"
+#include "cpl.h"
+#include "dcp.h"
 #include "test.h"
 #include "util.h"
 #include <boost/test/unit_test.hpp>
@@ -312,12 +316,12 @@ BOOST_AUTO_TEST_CASE (write_interop_subtitle_test2)
 /* Write some subtitle content as Interop XML using bitmaps and check that it is right */
 BOOST_AUTO_TEST_CASE (write_interop_subtitle_test3)
 {
-	dcp::InteropSubtitleAsset c;
-	c.set_reel_number ("1");
-	c.set_language ("EN");
-	c.set_movie_title ("Test");
+	shared_ptr<dcp::InteropSubtitleAsset> c (new dcp::InteropSubtitleAsset());
+	c->set_reel_number ("1");
+	c->set_language ("EN");
+	c->set_movie_title ("Test");
 
-	c.add (
+	c->add (
 		shared_ptr<dcp::Subtitle> (
 			new dcp::SubtitleImage (
 				dcp::Data ("test/data/sub.png"),
@@ -333,17 +337,43 @@ BOOST_AUTO_TEST_CASE (write_interop_subtitle_test3)
 			)
 		);
 
-	c._id = "a6c58cff-3e1e-4b38-acec-a42224475ef6";
-
+	c->_id = "a6c58cff-3e1e-4b38-acec-a42224475ef6";
 	boost::filesystem::create_directories ("build/test/write_interop_subtitle_test3");
-	c.write ("build/test/write_interop_subtitle_test3/subs.xml");
+	c->write ("build/test/write_interop_subtitle_test3/subs.xml");
+
+	shared_ptr<dcp::Reel> reel (new dcp::Reel());
+	reel->add(shared_ptr<dcp::ReelSubtitleAsset>(new dcp::ReelSubtitleAsset(c, dcp::Fraction(24, 1), 6046, 0)));
+
+	dcp::XMLMetadata xml_meta;
+	xml_meta.issue_date = "2018-09-02T04:45:18+00:00";
+
+	shared_ptr<dcp::CPL> cpl (new dcp::CPL ("My film", dcp::FEATURE));
+	cpl->add (reel);
+	cpl->set_metadata (xml_meta);
+	cpl->set_content_version_label_text ("foo");
+
+	dcp::DCP dcp ("build/test/write_interop_subtitle_test3");
+	dcp.add (cpl);
+	dcp.write_xml (dcp::INTEROP, xml_meta);
 
 	check_xml (
-		dcp::file_to_string("test/data/write_interop_subtitle_test3.xml"),
+		dcp::file_to_string("test/ref/write_interop_subtitle_test3/subs.xml"),
 		dcp::file_to_string("build/test/write_interop_subtitle_test3/subs.xml"),
 		list<string>()
 		);
 	check_file ("build/test/write_interop_subtitle_test3/822bd341-c751-45b1-94d2-410e4ffcff1b.png", "test/data/sub.png");
+
+	check_xml (
+		dcp::file_to_string("test/ref/write_interop_subtitle_test3/ASSETMAP"),
+		dcp::file_to_string("build/test/write_interop_subtitle_test3/ASSETMAP"),
+		list<string>()
+		);
+
+	check_xml (
+		dcp::file_to_string("test/ref/write_interop_subtitle_test3/pkl.xml"),
+		dcp::file_to_string("build/test/write_interop_subtitle_test3/pkl_8d98d2e1-d2a1-458f-b96b-295e5b5d0860.xml"),
+		list<string>()
+		);
 }
 
 /* Write some subtitle content as SMPTE XML and check that it is right */