More various fixes to subtitle XML writing.

6 files changed, 130 insertions, 21 deletions

diff --git a/run-tests.sh b/run-tests.sh
index e02589b6..b6064db3 100755
--- a/run-tests.sh
+++ b/run-tests.sh
@@ -21,25 +21,42 @@ fi
 
 rm -f build/test/info.log
 
-if [ -e "../libdcp-test" ]; then
-  for d in `find ../libdcp-test -mindepth 1 -maxdepth 1 -type d`; do
-    if [ `basename $d` != ".git" ]; then
-      LD_LIBRARY_PATH=build/src:build/asdcplib/src build/tools/dcpinfo -s $d >> build/test/info.log
-      if [ "$?" != "0" ]; then
-        echo "FAIL: dcpinfo failed for $d"
-        exit 1
-      fi
-    fi
-  done
-else
+if [ ! -e "../libdcp-test" ]; then
   echo "Test corpus not found"
   exit 1
 fi
 
+for d in `find ../libdcp-test -mindepth 1 -maxdepth 1 -type d`; do
+  if [ `basename $d` != ".git" ]; then
+    LD_LIBRARY_PATH=build/src:build/asdcplib/src build/tools/dcpinfo -s $d >> build/test/info.log
+    if [ "$?" != "0" ]; then
+      echo "FAIL: dcpinfo failed for $d"
+      exit 1
+    fi
+  fi
+done
+
 diff -q build/test/info.log ../libdcp-test/info.log
 if [ "$?" != "0" ]; then
   echo "FAIL: dcpinfo output incorrect"
   exit 1
 fi
 
+rm -f build/test/info2.log
+rm -rf build/test/libdcp-test
+
+cp -r ../libdcp-test build/test
+for d in `find build/test/libdcp-test -mindepth 1 -maxdepth 1 -type d`; do
+  if [ `basename $d` != ".git" ]; then
+    LD_LIBRARY_PATH=build/src:build/asdcplib/src build/test/rewrite_subs $d
+    LD_LIBRARY_PATH=build/src:build/asdcplib/src build/tools/dcpinfo -s $d >> build/test/info2.log
+  fi
+done
+
+diff -q build/test/info2.log ../libdcp-test/info.log
+if [ "$?" != "0" ]; then
+  echo "FAIL: dcpinfo output from rewrite incorrect"
+  exit 1
+fi
+    
 echo "PASS"
diff --git a/run/test/rewrite_subs b/run/test/rewrite_subs
new file mode 100755
index 00000000..a4c411d7
--- /dev/null
+++ b/run/test/rewrite_subs
@@ -0,0 +1,12 @@
+#!/bin/bash
+
+export LD_LIBRARY_PATH=build/src
+if [ "$1" == "--debug" ]; then
+    shift
+    gdb --args build/test/rewrite_subs "$@"
+elif [ "$1" == "--valgrind" ]; then
+    shift
+    valgrind --tool="memcheck" --leak-check=full --show-reachable=yes build/test/rewrite_subs "$@"
+else
+    build/test/rewrite_subs "$@"
+fi
diff --git a/src/subtitle_asset.cc b/src/subtitle_asset.cc
index 6141b2c5..9ba85a43 100644
--- a/src/subtitle_asset.cc
+++ b/src/subtitle_asset.cc
@@ -19,6 +19,7 @@
 
 #include <fstream>
 #include <boost/lexical_cast.hpp>
+#include <boost/algorithm/string.hpp>
 #include "subtitle_asset.h"
 #include "util.h"
 
@@ -33,6 +34,7 @@ using namespace libdcp;
 
 SubtitleAsset::SubtitleAsset (string directory, string xml_file)
 	: Asset (directory, xml_file)
+	, _need_sort (false)
 {
 	read_xml (path().string());
 }
@@ -42,6 +44,7 @@ SubtitleAsset::SubtitleAsset (string directory, string movie_title, string langu
 	, _movie_title (movie_title)
 	, _reel_number ("1")
 	, _language (language)
+	, _need_sort (false)
 {
 
 }
@@ -368,6 +371,7 @@ void
 SubtitleAsset::add (shared_ptr<Subtitle> s)
 {
 	_subtitles.push_back (s);
+	_need_sort = true;
 }
 
 void
@@ -392,24 +396,34 @@ struct SubtitleSorter {
 };
 
 void
-SubtitleAsset::write_xml ()
+SubtitleAsset::write_xml () const
 {
 	ofstream f (path().string().c_str());
 	write_xml (f);
 }
 
 void
-SubtitleAsset::write_xml (ostream& s)
+SubtitleAsset::write_xml (ostream& s) const
 {
 	s << "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
 	  << "<DCSubtitle Version=\"1.0\">\n"
 	  << "  <SubtitleID>" << _uuid << "</SubtitleID>\n"
 	  << "  <MovieTitle>" << _movie_title << "</MovieTitle>\n"
 	  << "  <ReelNumber>" << _reel_number << "</ReelNumber>\n"
-	  << "  <Language>" << _language << "</Language>\n"
-	  << "  <LoadFont Id=\"theFontId\" URI=\"arial.ttf\"/>\n";
+	  << "  <Language>" << _language << "</Language>\n";
 
-	_subtitles.sort (SubtitleSorter ());
+	if (_load_font_nodes.size() > 1) {
+		throw MiscError ("multiple LoadFont nodes not supported");
+	}
+
+	if (!_load_font_nodes.empty ()) {
+		s << "  <LoadFont Id=\"" << _load_font_nodes.front()->id << "\" URI=\"" << _load_font_nodes.front()->uri << "\"/>\n";
+	}
+
+	list<shared_ptr<Subtitle> > sorted = _subtitles;
+	if (_need_sort) {
+		sorted.sort (SubtitleSorter ());
+	}
 
 	/* XXX: multiple fonts not supported */
 	/* XXX: script, underlined, weight not supported */
@@ -426,7 +440,7 @@ SubtitleAsset::write_xml (ostream& s)
 	Time last_fade_up_time;
 	Time last_fade_down_time;
 
-	for (list<shared_ptr<Subtitle> >::iterator i = _subtitles.begin(); i != _subtitles.end(); ++i) {
+	for (list<shared_ptr<Subtitle> >::iterator i = sorted.begin(); i != sorted.end(); ++i) {
 
 		/* We will start a new <Font>...</Font> whenever some font property changes.
 		   I suppose should really make an optimal hierarchy of <Font> tags, but
@@ -470,7 +484,13 @@ SubtitleAsset::write_xml (ostream& s)
 				if (!first) {
 					s << "  </Font>\n";
 				}
-				s << "  <Font Id=\"theFontId\" " << a.str() << ">\n";
+
+				string id = "theFontId";
+				if (!_load_font_nodes.empty()) {
+					id = _load_font_nodes.front()->id;
+				}
+				
+				s << "  <Font Id=\"" << id << "\" " << a.str() << ">\n";
 			}
 
 			s << "  <Subtitle "
@@ -490,7 +510,7 @@ SubtitleAsset::write_xml (ostream& s)
 		s << "      <Text "
 		  << "VAlign=\"" << valign_to_string ((*i)->v_align()) << "\" "
 		  << "VPosition=\"" << (*i)->v_position() << "\""
-		  << ">" << (*i)->text() << "</Text>\n";
+		  << ">" << escape ((*i)->text()) << "</Text>\n";
 
 		first = false;
 	}
@@ -499,3 +519,11 @@ SubtitleAsset::write_xml (ostream& s)
 	s << "  </Font>\n";
 	s << "</DCSubtitle>\n";
 }
+
+/** XXX: Another reason why we should be writing with libxml++ */
+string
+SubtitleAsset::escape (string s) const
+{
+	boost::replace_all (s, "&", "&amp;");
+	return s;
+}
diff --git a/src/subtitle_asset.h b/src/subtitle_asset.h
index 5d996c7a..71ae42fc 100644
--- a/src/subtitle_asset.h
+++ b/src/subtitle_asset.h
@@ -202,11 +202,12 @@ public:
 	void add (boost::shared_ptr<Subtitle>);
 
 	void read_xml (std::string);
-	void write_xml ();
-	void write_xml (std::ostream& s);
+	void write_xml () const;
+	void write_xml (std::ostream& s) const;
 
 private:
 	std::string font_id_to_name (std::string id) const;
+	std::string escape (std::string) const;
 
 	struct ParseState {
 		std::list<boost::shared_ptr<FontNode> > font_nodes;
@@ -235,6 +236,7 @@ private:
 	std::list<boost::shared_ptr<LoadFontNode> > _load_font_nodes;
 
 	std::list<boost::shared_ptr<Subtitle> > _subtitles;
+	bool _need_sort;
 };
 
 }
diff --git a/test/rewrite_subs.cc b/test/rewrite_subs.cc
new file mode 100644
index 00000000..651b207a
--- /dev/null
+++ b/test/rewrite_subs.cc
@@ -0,0 +1,42 @@
+#include <iostream>
+#include "dcp.h"
+#include "reel.h"
+#include "subtitle_asset.h"
+
+using std::cout;
+using std::cerr;
+using std::list;
+using boost::shared_ptr;
+using namespace libdcp;
+
+int
+main (int argc, char* argv[])
+try
+{
+	if (argc < 2) {
+		cerr << "Syntax: " << argv[0] << " <dcp>\n";
+		exit (EXIT_FAILURE);
+	}
+
+	DCP* dcp = new DCP (argv[1]);
+	dcp->read (false);
+	
+	list<shared_ptr<const CPL> > cpls = dcp->cpls ();
+	for (list<boost::shared_ptr<const CPL> >::iterator i = cpls.begin(); i != cpls.end(); ++i) {
+
+		list<shared_ptr<const Reel> > reels = (*i)->reels ();
+		for (list<shared_ptr<const Reel> >::iterator j = reels.begin(); j != reels.end(); ++j) {
+
+			if ((*j)->main_subtitle()) {
+				(*j)->main_subtitle()->write_xml ();
+			}
+		}
+	}
+
+	return 0;
+}
+catch (FileError& e)
+{
+	cerr << e.what() << " (" << e.filename() << ")\n";
+	exit (EXIT_FAILURE);
+}
diff --git a/test/wscript b/test/wscript
index f24683b7..43ca7d73 100644
--- a/test/wscript
+++ b/test/wscript
@@ -31,3 +31,11 @@ def build(bld):
     obj.source = 'subs_in_out.cc'
     obj.target = 'subs_in_out'
     obj.install_path = ''
+
+    obj = bld(features = 'cxx cxxprogram')
+    obj.name   = 'rewrite_subs'
+    obj.uselib = 'BOOST_TEST OPENJPEG'
+    obj.use    = 'libdcp'
+    obj.source = 'rewrite_subs.cc'
+    obj.target = 'rewrite_subs'
+    obj.install_path = ''