From fac4b92813714d5b6bdaaa4425bf0bf81dbd1a45 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Sun, 3 Oct 2021 22:18:56 +0200 Subject: Make the former dcst namespace default for SMPTE subtitles. In DoM bug #2061 it is reported that Easy DCP gives the error "XML Document has default root namespace prefix: dcst. Default namespace should not use prefix for root or root namespace child nodes." with SMPTE subtitle files written by DCP-o-matic, and that the correct fix is to make the former dcst: namespace the default and then remove the dcst: qualifiers from everything. I'm not sure I agree with the error; AFAICS the subtitle files written by previous versions did not have a default root namespace prefix, since it was specified by xmlns:dcst and not just xmlns alone, so I think they were valid. However, using a default NS also seems fine, slightly simplifies the code and produces more compact subtitle files, so we change that here. It should also stop Easy DCP complaining, which is always marginally preferable to sticking to our guns and getting the blame for it. --- src/smpte_subtitle_asset.cc | 26 ++++++------ src/subtitle_asset_internal.cc | 21 ++++------ src/subtitle_asset_internal.h | 2 - test/smpte_subtitle_test.cc | 92 +++++++++++++++++++++--------------------- test/verify_test.cc | 4 +- 5 files changed, 68 insertions(+), 77 deletions(-) diff --git a/src/smpte_subtitle_asset.cc b/src/smpte_subtitle_asset.cc index 11450ef3..f09d99f3 100644 --- a/src/smpte_subtitle_asset.cc +++ b/src/smpte_subtitle_asset.cc @@ -343,36 +343,36 @@ string SMPTESubtitleAsset::xml_as_string () const { xmlpp::Document doc; - auto root = doc.create_root_node ("dcst:SubtitleReel"); - root->set_namespace_declaration (subtitle_smpte_ns, "dcst"); + auto root = doc.create_root_node ("SubtitleReel"); + root->set_namespace_declaration (subtitle_smpte_ns); root->set_namespace_declaration ("http://www.w3.org/2001/XMLSchema", "xs"); DCP_ASSERT (_xml_id); - root->add_child("Id", "dcst")->add_child_text ("urn:uuid:" + *_xml_id); - root->add_child("ContentTitleText", "dcst")->add_child_text (_content_title_text); + root->add_child("Id")->add_child_text("urn:uuid:" + *_xml_id); + root->add_child("ContentTitleText")->add_child_text(_content_title_text); if (_annotation_text) { - root->add_child("AnnotationText", "dcst")->add_child_text (_annotation_text.get ()); + root->add_child("AnnotationText")->add_child_text(_annotation_text.get()); } - root->add_child("IssueDate", "dcst")->add_child_text (_issue_date.as_string (true)); + root->add_child("IssueDate")->add_child_text(_issue_date.as_string(true)); if (_reel_number) { - root->add_child("ReelNumber", "dcst")->add_child_text (raw_convert (_reel_number.get ())); + root->add_child("ReelNumber")->add_child_text(raw_convert(_reel_number.get())); } if (_language) { - root->add_child("Language", "dcst")->add_child_text (_language.get ()); + root->add_child("Language")->add_child_text(_language.get()); } - root->add_child("EditRate", "dcst")->add_child_text (_edit_rate.as_string ()); - root->add_child("TimeCodeRate", "dcst")->add_child_text (raw_convert (_time_code_rate)); + root->add_child("EditRate")->add_child_text(_edit_rate.as_string()); + root->add_child("TimeCodeRate")->add_child_text(raw_convert(_time_code_rate)); if (_start_time) { - root->add_child("StartTime", "dcst")->add_child_text(_start_time.get().as_string(Standard::SMPTE)); + root->add_child("StartTime")->add_child_text(_start_time.get().as_string(Standard::SMPTE)); } for (auto i: _load_font_nodes) { - auto load_font = root->add_child("LoadFont", "dcst"); + auto load_font = root->add_child("LoadFont"); load_font->add_child_text ("urn:uuid:" + i->urn); load_font->set_attribute ("ID", i->id); } - subtitles_as_xml (root->add_child("SubtitleList", "dcst"), _time_code_rate, Standard::SMPTE); + subtitles_as_xml (root->add_child("SubtitleList"), _time_code_rate, Standard::SMPTE); return doc.write_to_string ("UTF-8"); } diff --git a/src/subtitle_asset_internal.cc b/src/subtitle_asset_internal.cc index a9200618..bf73fcc3 100644 --- a/src/subtitle_asset_internal.cc +++ b/src/subtitle_asset_internal.cc @@ -49,13 +49,6 @@ using std::shared_ptr; using namespace dcp; -string -order::Context::xmlns () const -{ - return standard == Standard::SMPTE ? "dcst" : ""; -} - - order::Font::Font (shared_ptr s, Standard standard) { if (s->font()) { @@ -82,11 +75,11 @@ order::Font::Font (shared_ptr s, Standard standard) xmlpp::Element* -order::Font::as_xml (xmlpp::Element* parent, Context& context) const +order::Font::as_xml (xmlpp::Element* parent, Context&) const { - xmlpp::Element* e = parent->add_child ("Font", context.xmlns()); - for (map::const_iterator i = _values.begin(); i != _values.end(); ++i) { - e->set_attribute (i->first, i->second); + auto e = parent->add_child("Font"); + for (const auto& i: _values) { + e->set_attribute (i.first, i.second); } return e; } @@ -207,7 +200,7 @@ position_align (xmlpp::Element* e, order::Context& context, HAlign h_align, floa xmlpp::Element* order::Text::as_xml (xmlpp::Element* parent, Context& context) const { - auto e = parent->add_child ("Text", context.xmlns()); + auto e = parent->add_child ("Text"); position_align (e, context, _h_align, _h_position, _v_align, _v_position); @@ -225,7 +218,7 @@ order::Text::as_xml (xmlpp::Element* parent, Context& context) const xmlpp::Element* order::Subtitle::as_xml (xmlpp::Element* parent, Context& context) const { - auto e = parent->add_child ("Subtitle", context.xmlns()); + auto e = parent->add_child ("Subtitle"); e->set_attribute ("SpotNumber", raw_convert (context.spot_number++)); e->set_attribute ("TimeIn", _in.rebase(context.time_code_rate).as_string(context.standard)); e->set_attribute ("TimeOut", _out.rebase(context.time_code_rate).as_string(context.standard)); @@ -257,7 +250,7 @@ order::Font::clear () xmlpp::Element * order::Image::as_xml (xmlpp::Element* parent, Context& context) const { - auto e = parent->add_child ("Image", context.xmlns()); + auto e = parent->add_child ("Image"); position_align (e, context, _h_align, _h_position, _v_align, _v_position); if (context.standard == Standard::SMPTE) { diff --git a/src/subtitle_asset_internal.h b/src/subtitle_asset_internal.h index 9b5bb2da..f24ed58a 100644 --- a/src/subtitle_asset_internal.h +++ b/src/subtitle_asset_internal.h @@ -69,8 +69,6 @@ namespace order { struct Context { - std::string xmlns () const; - int time_code_rate; Standard standard; int spot_number; diff --git a/test/smpte_subtitle_test.cc b/test/smpte_subtitle_test.cc index cc6190c2..e93efefb 100644 --- a/test/smpte_subtitle_test.cc +++ b/test/smpte_subtitle_test.cc @@ -222,27 +222,27 @@ BOOST_AUTO_TEST_CASE (write_smpte_subtitle_test) check_xml ( "" - "" - "urn:uuid:a6c58cff-3e1e-4b38-acec-a42224475ef6" - "Test" - "2016-04-01T03:52:00.000+00:00" - "1" - "en" - "24 1" - "24" - "" - "" - "" - "Hello world" - "" - "" - "" - "" - "What's going on" - "" - "" - "" - "", + "" + "urn:uuid:a6c58cff-3e1e-4b38-acec-a42224475ef6" + "Test" + "2016-04-01T03:52:00.000+00:00" + "1" + "en" + "24 1" + "24" + "" + "" + "" + "Hello world" + "" + "" + "" + "" + "What's going on" + "" + "" + "" + "", c.xml_as_string (), vector() ); @@ -408,31 +408,31 @@ BOOST_AUTO_TEST_CASE (write_smpte_subtitle_test2) check_xml ( c.xml_as_string(), "" - "" - "urn:uuid:a6c58cff-3e1e-4b38-acec-a42224475ef6" - "Test" - "2016-04-01T03:52:00.000+00:00" - "1" - "en" - "24 1" - "24" - "" - "" - "" - "" - "Testing is " - "really" - " fun" - "" - "" - "This is the " - "second" - " line" - "" - "" - "" - "" - "", + "" + "urn:uuid:a6c58cff-3e1e-4b38-acec-a42224475ef6" + "Test" + "2016-04-01T03:52:00.000+00:00" + "1" + "en" + "24 1" + "24" + "" + "" + "" + "" + "Testing is " + "really" + " fun" + "" + "" + "This is the " + "second" + " line" + "" + "" + "" + "" + "", vector() ); } diff --git a/test/verify_test.cc b/test/verify_test.cc index c68152b3..62bb7f31 100644 --- a/test/verify_test.cc +++ b/test/verify_test.cc @@ -1284,7 +1284,7 @@ BOOST_AUTO_TEST_CASE (verify_invalid_closed_caption_xml_size_in_bytes) { dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::INVALID_CLOSED_CAPTION_XML_SIZE_IN_BYTES, - string("413262"), + string("372207"), canonical(dir / "subs.mxf") }, { dcp::VerificationNote::Type::WARNING, dcp::VerificationNote::Code::INVALID_SUBTITLE_FIRST_TEXT_TIME }, @@ -1324,7 +1324,7 @@ verify_timed_text_asset_too_large (string name) check_verify_result ( { dir }, { - { dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::INVALID_TIMED_TEXT_SIZE_IN_BYTES, string("121696411"), canonical(dir / "subs.mxf") }, + { dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::INVALID_TIMED_TEXT_SIZE_IN_BYTES, string("121695136"), canonical(dir / "subs.mxf") }, { dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::INVALID_TIMED_TEXT_FONT_SIZE_IN_BYTES, string("121634816"), canonical(dir / "subs.mxf") }, { dcp::VerificationNote::Type::BV21_ERROR, dcp::VerificationNote::Code::MISSING_SUBTITLE_START_TIME, canonical(dir / "subs.mxf") }, { dcp::VerificationNote::Type::WARNING, dcp::VerificationNote::Code::INVALID_SUBTITLE_FIRST_TEXT_TIME }, -- cgit v1.2.3