Use a different ID for the XML inside a SMPTE subtitle MXF than the

ID for the MXF itself.  Apparently not doing so is an error (according
to Doremi's release notes for 2.8.18).

diff --git a/src/smpte_subtitle_asset.cc b/src/smpte_subtitle_asset.cc
index e52cd2effa684f20ca08a620d691d2ddeea117b2..c85afc2e268cd2ba8588b6b406047f095b67a053 100644 (file)
--- a/src/smpte_subtitle_asset.cc
+++ b/src/smpte_subtitle_asset.cc
@@ -66,6 +66,7 @@ SMPTESubtitleAsset::SMPTESubtitleAsset ()
        : _intrinsic_duration (0)
        , _edit_rate (24, 1)
        , _time_code_rate (24)
+       , _xml_id (make_uuid ())
 {
 
 }
@@ -99,7 +100,7 @@ SMPTESubtitleAsset::SMPTESubtitleAsset (boost::filesystem::path file)
                        xml.reset (new cxml::Document ("SubtitleReel"));
                        xml->read_file (file);
                        parse_xml (xml);
-                       _id = remove_urn_uuid (xml->string_child ("Id"));
+                       _id = _xml_id = remove_urn_uuid (xml->string_child ("Id"));
                } catch (cxml::Error& e) {
                        boost::throw_exception (
                                DCPReadError (
@@ -116,6 +117,7 @@ SMPTESubtitleAsset::SMPTESubtitleAsset (boost::filesystem::path file)
 void
 SMPTESubtitleAsset::parse_xml (shared_ptr<cxml::Document> xml)
 {
+       _xml_id = remove_urn_uuid(xml->string_child("Id"));
        _load_font_nodes = type_children<dcp::SMPTELoadFontNode> (xml, "LoadFont");
 
        _content_title_text = xml->string_child ("ContentTitleText");
@@ -258,7 +260,7 @@ SMPTESubtitleAsset::xml_as_string () const
        root->set_namespace_declaration ("http://www.smpte-ra.org/schemas/428-7/2010/DCST", "dcst");
        root->set_namespace_declaration ("http://www.w3.org/2001/XMLSchema", "xs");
 
-       root->add_child("Id", "dcst")->add_child_text ("urn:uuid:" + _id);
+       root->add_child("Id", "dcst")->add_child_text ("urn:uuid:" + _xml_id);
        root->add_child("ContentTitleText", "dcst")->add_child_text (_content_title_text);
        if (_annotation_text) {
                root->add_child("AnnotationText", "dcst")->add_child_text (_annotation_text.get ());

diff --git a/src/smpte_subtitle_asset.h b/src/smpte_subtitle_asset.h
index 004531a2f18a5514b834662a499ef0661d7823e5..cf7e714de1eb9477e6d9715a3efae4f27458c700 100644 (file)
--- a/src/smpte_subtitle_asset.h
+++ b/src/smpte_subtitle_asset.h
@@ -151,6 +151,10 @@ public:
                return _start_time;
        }
 
+       std::string xml_id () const {
+               return _xml_id;
+       }
+
        static bool valid_mxf (boost::filesystem::path);
 
 protected:
@@ -160,6 +164,9 @@ protected:
        }
 
 private:
+       friend struct ::write_smpte_subtitle_test;
+       friend struct ::write_smpte_subtitle_test2;
+
        void read_fonts (boost::shared_ptr<ASDCP::TimedText::MXFReader>);
        void parse_xml (boost::shared_ptr<cxml::Document> xml);
        void read_mxf_descriptor (boost::shared_ptr<ASDCP::TimedText::MXFReader> reader, boost::shared_ptr<DecryptionContext> dec);
@@ -179,6 +186,10 @@ private:
        boost::optional<Time> _start_time;
 
        std::list<boost::shared_ptr<SMPTELoadFontNode> > _load_font_nodes;
+       /** UUID for the XML inside the MXF, which should be different to the ID of the MXF according to
+        *  Doremi's 2.8.18 release notes.
+        */
+       std::string _xml_id;
 };
 
 }

diff --git a/test/smpte_subtitle_test.cc b/test/smpte_subtitle_test.cc
new file mode 100644 (file)
index 0000000..b094ea0
--- /dev/null
+++ b/test/smpte_subtitle_test.cc
@@ -0,0 +1,66 @@
+/*
+    Copyright (C) 2018 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 <boost/test/unit_test.hpp>
+#include "smpte_subtitle_asset.h"
+
+using std::string;
+using boost::optional;
+
+BOOST_AUTO_TEST_CASE (smpte_subtitle_id_test)
+{
+       dcp::SMPTESubtitleAsset subs;
+       subs.add(dcp::SubtitleString(
+                        optional<string>(),
+                        false, false, false,
+                        dcp::Colour(),
+                        64,
+                        1,
+                        dcp::Time(0, 1, 2, 3, 24),
+                        dcp::Time(0, 2, 2, 3, 24),
+                        0.5,
+                        dcp::HALIGN_CENTER,
+                        0.5,
+                        dcp::VALIGN_CENTER,
+                        dcp::DIRECTION_LTR,
+                        "Hello",
+                        dcp::NONE,
+                        dcp::Colour(),
+                        dcp::Time(0, 0, 0, 0, 24),
+                        dcp::Time(0, 0, 0, 0, 24)
+                        ));
+       subs.write("build/test/smpte_subtitle_id_test.mxf");
+
+       dcp::SMPTESubtitleAsset check("build/test/smpte_subtitle_id_test.mxf");
+       BOOST_CHECK(check.id() != check.xml_id());
+}

diff --git a/test/write_subtitle_test.cc b/test/write_subtitle_test.cc
index d9240a3062af33c19c684028daba8a4e27200f23..1ca7840e77f121c935a62014cae42884585896f6 100644 (file)
--- a/test/write_subtitle_test.cc
+++ b/test/write_subtitle_test.cc
@@ -356,7 +356,7 @@ BOOST_AUTO_TEST_CASE (write_smpte_subtitle_test)
                        )
                );
 
-       c._id = "a6c58cff-3e1e-4b38-acec-a42224475ef6";
+       c._xml_id = "a6c58cff-3e1e-4b38-acec-a42224475ef6";
 
        check_xml (
                "<?xml version=\"1.0\" encoding=\"UTF-8\"?>"
@@ -541,7 +541,7 @@ BOOST_AUTO_TEST_CASE (write_smpte_subtitle_test2)
                        )
                );
 
-       c._id = "a6c58cff-3e1e-4b38-acec-a42224475ef6";
+       c._xml_id = "a6c58cff-3e1e-4b38-acec-a42224475ef6";
 
        check_xml (
                c.xml_as_string (),

diff --git a/test/wscript b/test/wscript
index ff0a908a8d7096fa2cd228dbe80de3093f36416f..1afce80521d4c915248b8acbe84d04295a647df8 100644 (file)
--- a/test/wscript
+++ b/test/wscript
@@ -1,5 +1,5 @@
 #
-#    Copyright (C) 2012-2016 Carl Hetherington <cth@carlh.net>
+#    Copyright (C) 2012-2018 Carl Hetherington <cth@carlh.net>
 #
 #    This file is part of libdcp.
 #
@@ -90,6 +90,7 @@ def build(bld):
                  rgb_xyz_test.cc
                  round_trip_test.cc
                  smpte_load_font_test.cc
+                 smpte_subtitle_test.cc
                  sound_frame_test.cc
                  test.cc
                  util_test.cc