More various fixes to subtitle XML writing.

diff --git a/run-tests.sh b/run-tests.sh
index e02589b6c6f374496c74fed79d26008b143a0c7a..b6064db3143431cfe6e14e4f02464e9b4693f87e 100755 (executable)
--- 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 (executable)
index 0000000..a4c411d
--- /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 6141b2c510ee9b1e61c5b2306f5a26855781d106..9ba85a43b143fa54e17c952ed6f413dcb75ff5a6 100644 (file)
--- 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 5d996c7ad95b7323716f913234bb442dfe9873bd..71ae42fc426002d44abac1bdad302a3a03e6d015 100644 (file)
--- 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 (file)
index 0000000..651b207
--- /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 f24683b7bb7ef8e9ba8051095a85b60579571e3e..43ca7d732767b8d738fc975fc1c5994bd34e5e30 100644 (file)
--- 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 = ''