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).

9 files changed, 78 insertions, 37 deletions

diff --git a/src/cpl.cc b/src/cpl.cc
index c862a853..86b738d9 100644
--- 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 71ac8545..686954b2 100644
--- 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 153a73af..9b868197 100644
--- 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 5f955c4b..a670fdd5 100644
--- 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 1ef4b882..23d4869c 100644
--- 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 414cdc22..0ebf9078 100644
--- 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 308d6602..bdfc9484 100644
--- 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 a8efa5c3..2dac199f 100644
--- 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 b6293a05..d853bc81 100644
--- 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));