Use optional for ReelAsset _annotation_text.

Not only is this tag optional in Interop and SMPTE, but it would
appear that if it is present but empty a DCP will not play back
on Sony SRX320 / LMT3000 systems (DoM bug #2124).

Here we use optional<>, as seems to make sense, and also refuse
to write empty tags (instead omitting the tag entirely).

diff --git a/src/reel_asset.cc b/src/reel_asset.cc
index d233ee649ee20a7e8e598e69940bb15491586d8b..f9742628843446ee28668ac2c68e1b7bcf38ae87 100644 (file)
--- a/src/reel_asset.cc
+++ b/src/reel_asset.cc
@@ -1,5 +1,5 @@
 /*
-    Copyright (C) 2014-2021 Carl Hetherington <cth@carlh.net>
+    Copyright (C) 2014-2022 Carl Hetherington <cth@carlh.net>
 
     This file is part of libdcp.
 
@@ -72,7 +72,7 @@ ReelAsset::ReelAsset (shared_ptr<const cxml::Node> node)
        : Object (remove_urn_uuid (node->string_child ("Id")))
        , _intrinsic_duration (node->number_child<int64_t> ("IntrinsicDuration"))
        , _duration (node->optional_number_child<int64_t>("Duration"))
-       , _annotation_text (node->optional_string_child ("AnnotationText").get_value_or (""))
+       , _annotation_text (node->optional_string_child("AnnotationText"))
        , _edit_rate (Fraction (node->string_child ("EditRate")))
        , _entry_point (node->optional_number_child<int64_t>("EntryPoint"))
 {
@@ -93,7 +93,10 @@ ReelAsset::write_to_cpl (xmlpp::Node* node, Standard standard) const
                a->set_namespace_declaration (ns.first, ns.second);
        }
        a->add_child("Id")->add_child_text ("urn:uuid:" + _id);
-       a->add_child("AnnotationText")->add_child_text (_annotation_text);
+       /* Empty <AnnotationText> tags cause refusal to play on some Sony SRX320 / LMT3000 systems (DoM bug #2124) */
+       if (_annotation_text && !_annotation_text->empty()) {
+               a->add_child("AnnotationText")->add_child_text(*_annotation_text);
+       }
        a->add_child("EditRate")->add_child_text (_edit_rate.as_string());
        a->add_child("IntrinsicDuration")->add_child_text (raw_convert<string> (_intrinsic_duration));
        if (_entry_point) {
@@ -124,7 +127,7 @@ bool
 ReelAsset::asset_equals (shared_ptr<const ReelAsset> other, EqualityOptions opt, NoteHandler note) const
 {
        if (_annotation_text != other->_annotation_text) {
-               string const s = "Reel: annotation texts differ (" + _annotation_text + " vs " + other->_annotation_text + ")\n";
+               string const s = "Reel: annotation texts differ (" + _annotation_text.get_value_or("") + " vs " + other->_annotation_text.get_value_or("") + ")\n";
                if (!opt.reel_annotation_texts_can_differ) {
                        note (NoteType::ERROR, s);
                        return false;

diff --git a/src/reel_asset.h b/src/reel_asset.h
index ab06434ebe5f2f14774def1dc267e4afad623128..200c49ffdaad096b6435db9b466b3c2b45b2e4dd 100644 (file)
--- a/src/reel_asset.h
+++ b/src/reel_asset.h
@@ -120,7 +120,7 @@ public:
        /** @return <Duration>, or <IntrinsicDuration> - <EntryPoint> if <Duration> is not present */
        int64_t actual_duration () const;
 
-       std::string annotation_text () const {
+       boost::optional<std::string> annotation_text () const {
                return _annotation_text;
        }
 
@@ -128,6 +128,10 @@ public:
                _annotation_text = at;
        }
 
+       void unset_annotation_text () {
+               _annotation_text = boost::none;
+       }
+
        bool asset_equals (std::shared_ptr<const ReelAsset>, EqualityOptions, NoteHandler) const;
 
 protected:
@@ -147,7 +151,7 @@ protected:
        boost::optional<int64_t> _duration;    ///< The &lt;Duration&gt; from the reel's entry for this asset, if present
 
 private:
-       std::string _annotation_text;          ///< The &lt;AnnotationText&gt; from the reel's entry for this asset
+       boost::optional<std::string> _annotation_text; ///< The &lt;AnnotationText&gt; from the reel's entry for this asset
        Fraction _edit_rate;                   ///< The &lt;EditRate&gt; from the reel's entry for this asset
        boost::optional<int64_t> _entry_point; ///< The &lt;EntryPoint&gt; from the reel's entry for this asset
 };

diff --git a/test/reel_asset_test.cc b/test/reel_asset_test.cc
index 94f9c2653927f2ade7fbb52e28708cb5f5f1972e..9dd2f8c3fa10fce4cb59f66f4263ef707e186384 100644 (file)
--- a/test/reel_asset_test.cc
+++ b/test/reel_asset_test.cc
@@ -67,7 +67,7 @@ BOOST_AUTO_TEST_CASE (reel_picture_asset_test)
 
        dcp::ReelMonoPictureAsset pa (doc);
        BOOST_CHECK_EQUAL (pa.id(), "06ac1ca7-9c46-4107-8864-a6448e24b04b");
-       BOOST_CHECK_EQUAL (pa.annotation_text(), "Hello world!");
+       BOOST_CHECK_EQUAL (pa.annotation_text().get_value_or(""), "Hello world!");
        BOOST_CHECK_EQUAL (pa.edit_rate(), dcp::Fraction(24, 1));
        BOOST_CHECK_EQUAL (pa.intrinsic_duration(), 187048);
        BOOST_CHECK_EQUAL (pa.entry_point().get(), 42L);
@@ -99,7 +99,7 @@ BOOST_AUTO_TEST_CASE (reel_smpte_subtitle_asset_test)
 
        dcp::ReelSMPTESubtitleAsset ps (doc);
        BOOST_CHECK_EQUAL (ps.id(), "8bca1489-aab1-9259-a4fd-8150abc1de12");
-       BOOST_CHECK_EQUAL (ps.annotation_text(), "Goodbye world!");
+       BOOST_CHECK_EQUAL (ps.annotation_text().get_value_or(""), "Goodbye world!");
        BOOST_CHECK_EQUAL (ps.edit_rate(), dcp::Fraction(25, 1));
        BOOST_CHECK_EQUAL (ps.intrinsic_duration(), 1870);
        BOOST_CHECK_EQUAL (ps.entry_point().get(), 0L);