Pass MainSoundConfiguration object rather than a string.

I guess originally it was a string mostly because it's not very well defined,
and Interop seemingly does whatever it wants.  This basic change also means
that things are checked more carefully, and so we must be more relaxed with
some things seen in the wild that I can't find contradictions for in the
standard (and also with the case of channel IDs, which does seem to be
mentioned in the standard).

diff --git a/src/cpl.cc b/src/cpl.cc
index c862a85386d1c3e23e55397ad13b0b05650b9072..86b738d99fe843d0850488f7fb57babb2203fab5 100644 (file)
--- a/src/cpl.cc
+++ b/src/cpl.cc
@@ -288,7 +288,16 @@ CPL::read_composition_metadata_asset (cxml::ConstNodePtr node)
                _luminance = Luminance (lum);
        }
 
-       _main_sound_configuration = node->optional_string_child("MainSoundConfiguration");
+       if (auto msc = node->optional_string_child("MainSoundConfiguration")) {
+               try {
+                       _main_sound_configuration = MainSoundConfiguration(*msc);
+               } catch (MainSoundConfigurationError& e) {
+                       /* With Interop DCPs this node may not make any sense, but that's OK */
+                       if (_standard == dcp::Standard::SMPTE) {
+                               throw e;
+                       }
+               }
+       }
 
        auto sr = node->optional_string_child("MainSoundSampleRate");
        if (sr) {
@@ -492,7 +501,9 @@ CPL::maybe_write_composition_metadata_asset(xmlpp::Element* node, bool include_m
                _luminance->as_xml (meta, "meta");
        }
 
-       meta->add_child("MainSoundConfiguration", "meta")->add_child_text(*_main_sound_configuration);
+       if (_main_sound_configuration) {
+               meta->add_child("MainSoundConfiguration", "meta")->add_child_text(_main_sound_configuration->to_string());
+       }
        meta->add_child("MainSoundSampleRate", "meta")->add_child_text(raw_convert<string>(*_main_sound_sample_rate) + " 1");
 
        auto stored = meta->add_child("MainPictureStoredArea", "meta");

diff --git a/src/cpl.h b/src/cpl.h
index 71ac8545a9bd99280d081008a188eb20596caae6..686954b244795ed0bc4bc9bcf1eca7bd1bd3bfe9 100644 (file)
--- a/src/cpl.h
+++ b/src/cpl.h
@@ -283,11 +283,11 @@ public:
                _luminance = l;
        }
 
-       boost::optional<std::string> main_sound_configuration () const {
+       boost::optional<dcp::MainSoundConfiguration> main_sound_configuration () const {
                return _main_sound_configuration;
        }
 
-       void set_main_sound_configuration (std::string c) {
+       void set_main_sound_configuration(dcp::MainSoundConfiguration c) {
                _main_sound_configuration = c;
        }
 
@@ -377,7 +377,7 @@ private:
        boost::optional<std::string> _distributor;
        boost::optional<std::string> _facility;
        boost::optional<Luminance> _luminance;
-       boost::optional<std::string> _main_sound_configuration;
+       boost::optional<MainSoundConfiguration> _main_sound_configuration;
        boost::optional<int> _main_sound_sample_rate;
        boost::optional<dcp::Size> _main_picture_stored_area;
        boost::optional<dcp::Size> _main_picture_active_area;

diff --git a/src/types.cc b/src/types.cc
index 153a73afb180187f6511e3d9257ec388831284cd..9b86819797c6fefe955a4fb30f88ae102c58dbb7 100644 (file)
--- a/src/types.cc
+++ b/src/types.cc
@@ -473,8 +473,8 @@ MainSoundConfiguration::MainSoundConfiguration (string s)
 {
        vector<string> parts;
        boost::split (parts, s, boost::is_any_of("/"));
-       if (parts.size() != 2) {
-               throw MainSoundConfigurationError (s);
+       if (parts.empty()) {
+               throw MainSoundConfigurationError(s);
        }
 
        if (parts[0] == "51") {
@@ -482,7 +482,14 @@ MainSoundConfiguration::MainSoundConfiguration (string s)
        } else if (parts[0] == "71") {
                _field = MCASoundField::SEVEN_POINT_ONE;
        } else {
-               throw MainSoundConfigurationError (s);
+               _field = MCASoundField::OTHER;
+       }
+
+       if (parts.size() < 2) {
+               /* I think it's OK to just have the sound field descriptor with no channels, though
+                * to me it's not clear and I might be wrong.
+                */
+               return;
        }
 
        vector<string> channels;
@@ -590,31 +597,33 @@ dcp::string_to_status (string s)
 Channel
 dcp::mca_id_to_channel (string id)
 {
-       if (id == "L") {
+       transform(id.begin(), id.end(), id.begin(), ::tolower);
+
+       if (id == "l") {
                return Channel::LEFT;
-       } else if (id == "R") {
+       } else if (id == "r") {
                return Channel::RIGHT;
-       } else if (id == "C") {
+       } else if (id == "c") {
                return Channel::CENTRE;
-       } else if (id == "LFE") {
+       } else if (id == "lfe") {
                return Channel::LFE;
-       } else if (id == "Ls" || id == "Lss") {
+       } else if (id == "ls" || id == "lss") {
                return Channel::LS;
-       } else if (id == "Rs" || id == "Rss") {
+       } else if (id == "rs" || id == "rss") {
                return Channel::RS;
-       } else if (id == "HI") {
+       } else if (id == "hi") {
                return Channel::HI;
-       } else if (id == "VIN") {
+       } else if (id == "vin") {
                return Channel::VI;
-       } else if (id == "Lrs") {
+       } else if (id == "lrs") {
                return Channel::BSL;
-       } else if (id == "Rrs") {
+       } else if (id == "rrs") {
                return Channel::BSR;
-       } else if (id == "DBOX") {
+       } else if (id == "dbox") {
                return Channel::MOTION_DATA;
-       } else if (id == "FSKSync") {
+       } else if (id == "sync" || id == "fsksync") {
                return Channel::SYNC_SIGNAL;
-       } else if (id == "SLVS") {
+       } else if (id == "slvs") {
                return Channel::SIGN_LANGUAGE;
        }
 

diff --git a/src/types.h b/src/types.h
index 5f955c4b119f4b07697757fc6e6538ecc630642f..a670fdd592a4218fd0b8613efe5a6351fd25e856 100644 (file)
--- a/src/types.h
+++ b/src/types.h
@@ -119,7 +119,8 @@ std::vector<dcp::Channel> used_audio_channels ();
 enum class MCASoundField
 {
        FIVE_POINT_ONE,
-       SEVEN_POINT_ONE
+       SEVEN_POINT_ONE,
+       OTHER
 };
 
 
@@ -445,7 +446,7 @@ bool operator== (Luminance const& a, Luminance const& b);
 class MainSoundConfiguration
 {
 public:
-       MainSoundConfiguration (std::string);
+       explicit MainSoundConfiguration(std::string);
        MainSoundConfiguration (MCASoundField field_, int channels);
 
        MCASoundField field () const {

diff --git a/test/combine_test.cc b/test/combine_test.cc
index 1ef4b8823e550775f172777d52bce6b3804beb09..23d4869c933f2fb72c08b80e0214dfa9d81d4a1b 100644 (file)
--- a/test/combine_test.cc
+++ b/test/combine_test.cc
@@ -343,7 +343,7 @@ BOOST_AUTO_TEST_CASE (combine_two_dcps_with_shared_asset)
        cpl->set_content_version (
                dcp::ContentVersion("urn:uuid:75ac29aa-42ac-1234-ecae-49251abefd11","content-version-label-text")
                );
-       cpl->set_main_sound_configuration("51/L,C,R,LFE,-,-");
+       cpl->set_main_sound_configuration(dcp::MainSoundConfiguration("51/L,C,R,LFE,-,-"));
        cpl->set_main_sound_sample_rate (48000);
        cpl->set_main_picture_stored_area (dcp::Size(1998, 1080));
        cpl->set_main_picture_active_area (dcp::Size(1440, 1080));
@@ -388,7 +388,7 @@ BOOST_AUTO_TEST_CASE (combine_two_dcps_with_duplicated_asset)
        cpl->set_content_version (
                dcp::ContentVersion("urn:uuid:75ac29aa-42ac-1234-ecae-49251abefd11","content-version-label-text")
                );
-       cpl->set_main_sound_configuration("51/L,C,R,LFE,-,-");
+       cpl->set_main_sound_configuration(dcp::MainSoundConfiguration("51/L,C,R,LFE,-,-"));
        cpl->set_main_sound_sample_rate (48000);
        cpl->set_main_picture_stored_area (dcp::Size(1998, 1080));
        cpl->set_main_picture_active_area (dcp::Size(1440, 1080));

diff --git a/test/cpl_metadata_test.cc b/test/cpl_metadata_test.cc
index 414cdc224cf5b7969cd3203f326d928de36d5f94..0ebf9078af6fb00b3be1656c2a1cc622691f646b 100644 (file)
--- a/test/cpl_metadata_test.cc
+++ b/test/cpl_metadata_test.cc
@@ -154,6 +154,26 @@ BOOST_AUTO_TEST_CASE (main_sound_configuration_test5)
 }
 
 
+/* 482-12 says that implementations may use case-insensitive comparisons for the channel identifiers,
+ * and there is one DCP in the private test suite (made by Disney) that uses LS for left surround.
+ */
+BOOST_AUTO_TEST_CASE(main_sound_configuration_test_case_insensitive)
+{
+       dcp::MainSoundConfiguration msc("51/L,-,C,LFE,LS,RS,HI,VIN");
+       BOOST_CHECK_EQUAL(msc.to_string(), "51/L,-,C,LFE,Ls,Rs,HI,VIN");
+       BOOST_CHECK_EQUAL(msc.channels(), 8);
+       BOOST_CHECK_EQUAL(msc.field(), dcp::MCASoundField::FIVE_POINT_ONE);
+       BOOST_CHECK_EQUAL(msc.mapping(0).get(), dcp::Channel::LEFT);
+       BOOST_CHECK(!msc.mapping(1));
+       BOOST_CHECK_EQUAL(msc.mapping(2).get(), dcp::Channel::CENTRE);
+       BOOST_CHECK_EQUAL(msc.mapping(3).get(), dcp::Channel::LFE);
+       BOOST_CHECK_EQUAL(msc.mapping(4).get(), dcp::Channel::LS);
+       BOOST_CHECK_EQUAL(msc.mapping(5).get(), dcp::Channel::RS);
+       BOOST_CHECK_EQUAL(msc.mapping(6).get(), dcp::Channel::HI);
+       BOOST_CHECK_EQUAL(msc.mapping(7).get(), dcp::Channel::VI);
+}
+
+
 BOOST_AUTO_TEST_CASE (luminance_test1)
 {
        BOOST_CHECK_NO_THROW (dcp::Luminance(4, dcp::Luminance::Unit::CANDELA_PER_SQUARE_METRE));
@@ -283,7 +303,7 @@ BOOST_AUTO_TEST_CASE (cpl_metadata_write_test1)
        msc.set_mapping (2, dcp::Channel::CENTRE);
        msc.set_mapping (3, dcp::Channel::LFE);
        msc.set_mapping (13, dcp::Channel::SYNC_SIGNAL);
-       cpl.set_main_sound_configuration (msc.to_string());
+       cpl.set_main_sound_configuration(msc);
 
        cpl.set_main_sound_sample_rate (48000);
        cpl.set_main_picture_stored_area (dcp::Size(1998, 1080));
@@ -354,7 +374,7 @@ BOOST_AUTO_TEST_CASE (cpl_metadata_write_test2)
        msc.set_mapping (2, dcp::Channel::CENTRE);
        msc.set_mapping (3, dcp::Channel::LFE);
        msc.set_mapping (13, dcp::Channel::SYNC_SIGNAL);
-       cpl.set_main_sound_configuration (msc.to_string());
+       cpl.set_main_sound_configuration(msc);
 
        cpl.set_main_sound_sample_rate (48000);
        cpl.set_main_picture_stored_area (dcp::Size(1998, 1080));

diff --git a/test/mca_test.cc b/test/mca_test.cc
index 308d66026bd28328fd14152d6fea2e4fca3d8528..bdfc9484cc33b20a9022280e6c38a89dca4a5e2d 100644 (file)
--- a/test/mca_test.cc
+++ b/test/mca_test.cc
@@ -69,7 +69,7 @@ BOOST_AUTO_TEST_CASE (parse_mca_descriptors_from_mxf_test)
 
                dcp::CPL cpl("", dcp::ContentKind::FEATURE, dcp::Standard::SMPTE);
                cpl.add (reel);
-               cpl.set_main_sound_configuration("51/L,R,C,LFE,Ls,Rs");
+               cpl.set_main_sound_configuration(dcp::MainSoundConfiguration("51/L,R,C,LFE,Ls,Rs"));
                cpl.set_main_sound_sample_rate(48000);
                cpl.set_main_picture_stored_area(dcp::Size(1998, 1080));
                cpl.set_main_picture_active_area(dcp::Size(1998, 1080));
@@ -122,7 +122,7 @@ BOOST_AUTO_TEST_CASE (write_mca_descriptors_to_mxf_test)
 
        dcp::CPL cpl("", dcp::ContentKind::FEATURE, dcp::Standard::SMPTE);
        cpl.add (reel);
-       cpl.set_main_sound_configuration("51/L,R,C,LFE,Ls,Rs");
+       cpl.set_main_sound_configuration(dcp::MainSoundConfiguration("51/L,R,C,LFE,Ls,Rs"));
        cpl.set_main_sound_sample_rate(48000);
        cpl.set_main_picture_stored_area(dcp::Size(1998, 1080));
        cpl.set_main_picture_active_area(dcp::Size(1998, 1080));
@@ -197,7 +197,7 @@ check_mca_descriptors(int suffix, vector<dcp::Channel> extra_active_channels, ve
 
        dcp::CPL cpl("", dcp::ContentKind::FEATURE, dcp::Standard::SMPTE);
        cpl.add(reel);
-       cpl.set_main_sound_configuration("51/L,R,C,LFE,Ls,Rs");
+       cpl.set_main_sound_configuration(dcp::MainSoundConfiguration("51/L,R,C,LFE,Ls,Rs"));
        cpl.set_main_sound_sample_rate(48000);
        cpl.set_main_picture_stored_area(dcp::Size(1998, 1080));
        cpl.set_main_picture_active_area(dcp::Size(1998, 1080));

diff --git a/test/test.cc b/test/test.cc
index a8efa5c359794ac80246c626f5457778b1470344..2dac199fe316f62aa9115e5cfb683e0a27eb5ee3 100644 (file)
--- a/test/test.cc
+++ b/test/test.cc
@@ -347,7 +347,7 @@ make_simple (boost::filesystem::path path, int reels, int frames, dcp::Standard
        cpl->set_content_version (
                dcp::ContentVersion("urn:uuid:75ac29aa-42ac-1234-ecae-49251abefd11", "content-version-label-text")
                );
-       cpl->set_main_sound_configuration("51/L,R,C,LFE,Ls,Rs");
+       cpl->set_main_sound_configuration(dcp::MainSoundConfiguration("51/L,R,C,LFE,Ls,Rs"));
        cpl->set_main_sound_sample_rate(sample_rate);
        cpl->set_main_picture_stored_area(dcp::Size(1998, 1080));
        cpl->set_main_picture_active_area(dcp::Size(1998, 1080));

diff --git a/test/verify_test.cc b/test/verify_test.cc
index b6293a051c870528632881f661cdae6f0b6e2d85..d853bc810d896462d61c2b82fd0e8336eb55bfc7 100644 (file)
--- a/test/verify_test.cc
+++ b/test/verify_test.cc
@@ -995,7 +995,7 @@ BOOST_AUTO_TEST_CASE (verify_valid_cpl_metadata)
 
        auto cpl = make_shared<dcp::CPL>("hello", dcp::ContentKind::TRAILER, dcp::Standard::SMPTE);
        cpl->add (reel);
-       cpl->set_main_sound_configuration("51/L,C,R,LFE,-,-");
+       cpl->set_main_sound_configuration(dcp::MainSoundConfiguration("51/L,C,R,LFE,-,-"));
        cpl->set_main_sound_sample_rate (48000);
        cpl->set_main_picture_stored_area (dcp::Size(1998, 1080));
        cpl->set_main_picture_active_area (dcp::Size(1440, 1080));
@@ -1052,7 +1052,7 @@ BOOST_AUTO_TEST_CASE (verify_invalid_cpl_metadata_bad_tag)
        reel->add (black_picture_asset(dir));
        auto cpl = make_shared<dcp::CPL>("hello", dcp::ContentKind::TRAILER, dcp::Standard::SMPTE);
        cpl->add (reel);
-       cpl->set_main_sound_configuration("51/L,C,R,LFE,-,-");
+       cpl->set_main_sound_configuration(dcp::MainSoundConfiguration("51/L,C,R,LFE,-,-"));
        cpl->set_main_sound_sample_rate (48000);
        cpl->set_main_picture_stored_area (dcp::Size(1998, 1080));
        cpl->set_main_picture_active_area (dcp::Size(1440, 1080));
@@ -1102,7 +1102,7 @@ BOOST_AUTO_TEST_CASE (verify_invalid_cpl_metadata_missing_tag)
        reel->add (black_picture_asset(dir));
        auto cpl = make_shared<dcp::CPL>("hello", dcp::ContentKind::TRAILER, dcp::Standard::SMPTE);
        cpl->add (reel);
-       cpl->set_main_sound_configuration("51/L,C,R,LFE,-,-");
+       cpl->set_main_sound_configuration(dcp::MainSoundConfiguration("51/L,C,R,LFE,-,-"));
        cpl->set_main_sound_sample_rate (48000);
        cpl->set_main_picture_stored_area (dcp::Size(1998, 1080));
        cpl->set_main_picture_active_area (dcp::Size(1440, 1080));
@@ -1190,7 +1190,7 @@ BOOST_AUTO_TEST_CASE (verify_invalid_language3)
        cpl->add (reel);
        cpl->_additional_subtitle_languages.push_back("this-is-wrong");
        cpl->_additional_subtitle_languages.push_back("andso-is-this");
-       cpl->set_main_sound_configuration("51/L,C,R,LFE,-,-");
+       cpl->set_main_sound_configuration(dcp::MainSoundConfiguration("51/L,C,R,LFE,-,-"));
        cpl->set_main_sound_sample_rate (48000);
        cpl->set_main_picture_stored_area (dcp::Size(1998, 1080));
        cpl->set_main_picture_active_area (dcp::Size(1440, 1080));
@@ -1241,7 +1241,7 @@ check_picture_size (int width, int height, int frame_rate, bool three_d)
        auto cpl = make_shared<dcp::CPL>("A Test DCP", dcp::ContentKind::TRAILER, dcp::Standard::SMPTE);
        cpl->set_annotation_text ("A Test DCP");
        cpl->set_issue_date ("2012-07-17T04:45:18+00:00");
-       cpl->set_main_sound_configuration("51/L,C,R,LFE,-,-");
+       cpl->set_main_sound_configuration(dcp::MainSoundConfiguration("51/L,C,R,LFE,-,-"));
        cpl->set_main_sound_sample_rate (48000);
        cpl->set_main_picture_stored_area(dcp::Size(width, height));
        cpl->set_main_picture_active_area(dcp::Size(width, height));
@@ -3050,7 +3050,7 @@ BOOST_AUTO_TEST_CASE (verify_partially_encrypted)
        cpl->set_issuer ("OpenDCP 0.0.25");
        cpl->set_creator ("OpenDCP 0.0.25");
        cpl->set_issue_date ("2012-07-17T04:45:18+00:00");
-       cpl->set_main_sound_configuration("51/L,C,R,LFE,-,-");
+       cpl->set_main_sound_configuration(dcp::MainSoundConfiguration("51/L,C,R,LFE,-,-"));
        cpl->set_main_sound_sample_rate (48000);
        cpl->set_main_picture_stored_area (dcp::Size(1998, 1080));
        cpl->set_main_picture_active_area (dcp::Size(1440, 1080));