From 582bdd054dd5cc6e0874459d2ad426dc5669fb78 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Mon, 17 Jan 2022 18:45:19 +0100 Subject: [PATCH] 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). --- src/reel_asset.cc | 11 +++++++---- src/reel_asset.h | 8 ++++++-- test/reel_asset_test.cc | 4 ++-- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/reel_asset.cc b/src/reel_asset.cc index d233ee64..f9742628 100644 --- a/src/reel_asset.cc +++ b/src/reel_asset.cc @@ -1,5 +1,5 @@ /* - Copyright (C) 2014-2021 Carl Hetherington + Copyright (C) 2014-2022 Carl Hetherington This file is part of libdcp. @@ -72,7 +72,7 @@ ReelAsset::ReelAsset (shared_ptr node) : Object (remove_urn_uuid (node->string_child ("Id"))) , _intrinsic_duration (node->number_child ("IntrinsicDuration")) , _duration (node->optional_number_child("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("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 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 (_intrinsic_duration)); if (_entry_point) { @@ -124,7 +127,7 @@ bool ReelAsset::asset_equals (shared_ptr 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 ab06434e..200c49ff 100644 --- a/src/reel_asset.h +++ b/src/reel_asset.h @@ -120,7 +120,7 @@ public: /** @return , or - if is not present */ int64_t actual_duration () const; - std::string annotation_text () const { + boost::optional 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, EqualityOptions, NoteHandler) const; protected: @@ -147,7 +151,7 @@ protected: boost::optional _duration; ///< The <Duration> from the reel's entry for this asset, if present private: - std::string _annotation_text; ///< The <AnnotationText> from the reel's entry for this asset + boost::optional _annotation_text; ///< The <AnnotationText> from the reel's entry for this asset Fraction _edit_rate; ///< The <EditRate> from the reel's entry for this asset boost::optional _entry_point; ///< The <EntryPoint> from the reel's entry for this asset }; diff --git a/test/reel_asset_test.cc b/test/reel_asset_test.cc index 94f9c265..9dd2f8c3 100644 --- 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); -- 2.30.2