Allow specification of channels that need a MCASubDescriptor.

6 files changed, 174 insertions, 20 deletions

diff --git a/src/cpl.cc b/src/cpl.cc
index 33a690f3..c862a853 100644
--- a/src/cpl.cc
+++ b/src/cpl.cc
@@ -385,6 +385,7 @@ CPL::write_mca_subdescriptors(xmlpp::Element* parent, shared_ptr<const SoundAsse
 			sf->add_child("RFC5646SpokenLanguage", "r1")->add_child_text(buffer);
 		}
 
+		/* Find the MCA subdescriptors in the MXF so that we can also write them here */
 		list<ASDCP::MXF::InterchangeObject*> channels;
 		auto r = reader->reader()->OP1aHeader().GetMDObjectsByType(
 			asdcp_smpte_dict->ul(ASDCP::MDD_AudioChannelLabelSubDescriptor),
@@ -393,9 +394,6 @@ CPL::write_mca_subdescriptors(xmlpp::Element* parent, shared_ptr<const SoundAsse
 
 		for (auto i: channels) {
 			auto channel = reinterpret_cast<ASDCP::MXF::AudioChannelLabelSubDescriptor*>(i);
-			if (static_cast<int>(channel->MCAChannelID) > asset->channels()) {
-				continue;
-			}
 			auto ch = mca_subs->add_child("AudioChannelLabelSubDescriptor", "r0");
 			channel->InstanceUID.EncodeString(buffer, sizeof(buffer));
 			ch->add_child("InstanceID", "r1")->add_child_text("urn:uuid:" + string(buffer));
diff --git a/src/sound_asset.cc b/src/sound_asset.cc
index 56d13951..7c94b549 100644
--- a/src/sound_asset.cc
+++ b/src/sound_asset.cc
@@ -223,6 +223,7 @@ SoundAsset::equals (shared_ptr<const Asset> other, EqualityOptions opt, NoteHand
 shared_ptr<SoundAssetWriter>
 SoundAsset::start_write(
 	boost::filesystem::path file,
+	vector<dcp::Channel> extra_active_channels,
 	AtmosSync atmos_sync,
 	MCASubDescriptors include_mca_subdescriptors
 	)
@@ -232,7 +233,7 @@ SoundAsset::start_write(
 	}
 
 	return shared_ptr<SoundAssetWriter>(
-		new SoundAssetWriter(this, file, atmos_sync == AtmosSync::ENABLED, include_mca_subdescriptors == MCASubDescriptors::ENABLED)
+		new SoundAssetWriter(this, file, extra_active_channels, atmos_sync == AtmosSync::ENABLED, include_mca_subdescriptors == MCASubDescriptors::ENABLED)
 		);
 }
 
diff --git a/src/sound_asset.h b/src/sound_asset.h
index e51f1854..55669c2f 100644
--- a/src/sound_asset.h
+++ b/src/sound_asset.h
@@ -85,8 +85,12 @@ public:
 		DISABLED
 	};
 
+	/** @param extra_active_channels list of channels that are active in the asset, other than the basic 5.1
+	 *  which are assumed always to be active.
+	 */
 	std::shared_ptr<SoundAssetWriter> start_write(
 		boost::filesystem::path file,
+		std::vector<dcp::Channel> extra_active_channels,
 		AtmosSync atmos_sync,
 		MCASubDescriptors mca_subdescriptors
 		);
diff --git a/src/sound_asset_writer.cc b/src/sound_asset_writer.cc
index 0300407c..1185616a 100644
--- a/src/sound_asset_writer.cc
+++ b/src/sound_asset_writer.cc
@@ -69,16 +69,36 @@ struct SoundAssetWriter::ASDCPState
 };
 
 
-SoundAssetWriter::SoundAssetWriter (SoundAsset* asset, boost::filesystem::path file, bool sync, bool include_mca_subdescriptors)
+SoundAssetWriter::SoundAssetWriter(SoundAsset* asset, boost::filesystem::path file, vector<dcp::Channel> extra_active_channels, bool sync, bool include_mca_subdescriptors)
 	: AssetWriter (asset, file)
 	, _state (new SoundAssetWriter::ASDCPState)
 	, _asset (asset)
+	, _extra_active_channels(extra_active_channels)
 	, _sync (sync)
 	, _include_mca_subdescriptors(include_mca_subdescriptors)
 {
 	DCP_ASSERT (!_sync || _asset->channels() >= 14);
 	DCP_ASSERT (!_sync || _asset->standard() == Standard::SMPTE);
 
+	/* None of these are allowed in extra_active_channels; some are implicit, and (it seems) should never have a descriptor
+	 * written for them.
+	 */
+	vector<Channel> disallowed_extra = {
+		Channel::LEFT,
+		Channel::RIGHT,
+		Channel::CENTRE,
+		Channel::LFE,
+		Channel::LS,
+		Channel::RS,
+		Channel::MOTION_DATA,
+		Channel::SYNC_SIGNAL,
+		Channel::SIGN_LANGUAGE,
+		Channel::CHANNEL_COUNT
+	};
+	for (auto disallowed: disallowed_extra) {
+		DCP_ASSERT(std::find(extra_active_channels.begin(), extra_active_channels.end(), disallowed) == extra_active_channels.end());
+	}
+
 	/* Derived from ASDCP::Wav::SimpleWaveHeader::FillADesc */
 	_state->desc.EditRate = ASDCP::Rational (_asset->edit_rate().numerator, _asset->edit_rate().denominator);
 	_state->desc.AudioSamplingRate = ASDCP::Rational (_asset->sampling_rate(), 1);
@@ -146,7 +166,11 @@ SoundAssetWriter::start ()
 			soundfield->RFC5646SpokenLanguage = *lang;
 		}
 
-		const MCASoundField field = _asset->channels() > 10 ? MCASoundField::SEVEN_POINT_ONE : MCASoundField::FIVE_POINT_ONE;
+		MCASoundField const field =
+			(
+				find(_extra_active_channels.begin(), _extra_active_channels.end(), dcp::Channel::BSL) != _extra_active_channels.end() ||
+				find(_extra_active_channels.begin(), _extra_active_channels.end(), dcp::Channel::BSR) != _extra_active_channels.end()
+			) ? MCASoundField::SEVEN_POINT_ONE : MCASoundField::FIVE_POINT_ONE;
 
 		if (field == MCASoundField::SEVEN_POINT_ONE) {
 			soundfield->MCATagSymbol = "sg71";
@@ -165,22 +189,25 @@ LIBDCP_ENABLE_WARNINGS
 		_state->mxf_writer.OP1aHeader().AddChildObject(soundfield);
 		essence_descriptor->SubDescriptors.push_back(soundfield->InstanceUID);
 
-		/* We must describe at least the number of channels in `field', even if they aren't
-		 * in the asset (I think)
-		 */
-		int descriptors = max(_asset->channels(), field == MCASoundField::FIVE_POINT_ONE ? 6 : 8);
-
-		auto const used = used_audio_channels();
-
-		for (auto i = 0; i < descriptors; ++i) {
-			auto dcp_channel = static_cast<dcp::Channel>(i);
-			if (find(used.begin(), used.end(), dcp_channel) == used.end()) {
-				continue;
-			}
+		std::vector<dcp::Channel> dcp_channels = {
+			Channel::LEFT,
+			Channel::RIGHT,
+			Channel::CENTRE,
+			Channel::LFE,
+			Channel::LS,
+			Channel::RS
+		};
+
+		std::copy(_extra_active_channels.begin(), _extra_active_channels.end(), back_inserter(dcp_channels));
+		std::sort(dcp_channels.begin(), dcp_channels.end());
+		auto last = std::unique(dcp_channels.begin(), dcp_channels.end());
+		dcp_channels.erase(last, dcp_channels.end());
+
+		for (auto dcp_channel: dcp_channels) {
 			auto channel = new ASDCP::MXF::AudioChannelLabelSubDescriptor(asdcp_smpte_dict);
 			GenRandomValue (channel->MCALinkID);
 			channel->SoundfieldGroupLinkID = soundfield->MCALinkID;
-			channel->MCAChannelID = i + 1;
+			channel->MCAChannelID = static_cast<int>(dcp_channel) + 1;
 			channel->MCATagSymbol = "ch" + channel_to_mca_id(dcp_channel, field);
 			channel->MCATagName = channel_to_mca_name(dcp_channel, field);
 			if (auto lang = _asset->language()) {
diff --git a/src/sound_asset_writer.h b/src/sound_asset_writer.h
index d8ebdc7b..495a4582 100644
--- a/src/sound_asset_writer.h
+++ b/src/sound_asset_writer.h
@@ -162,7 +162,7 @@ private:
 		}
 	}
 
-	SoundAssetWriter(SoundAsset *, boost::filesystem::path, bool sync, bool include_mca_subdescriptors);
+	SoundAssetWriter(SoundAsset *, boost::filesystem::path, std::vector<dcp::Channel> extra_active_channels, bool sync, bool include_mca_subdescriptors);
 
 	void start ();
 	void write_current_frame ();
@@ -178,6 +178,7 @@ private:
 	SoundAsset* _asset = nullptr;
 	int _frame_buffer_offset = 0;
 
+	std::vector<dcp::Channel> _extra_active_channels;
 	/** true to ignore any signal passed to write() on channel 14 and instead write a sync track */
 	bool _sync = false;
 	/** index of the sync packet (0-3) which starts the next edit unit */
diff --git a/test/mca_test.cc b/test/mca_test.cc
index fc6a76d3..308d6602 100644
--- a/test/mca_test.cc
+++ b/test/mca_test.cc
@@ -42,6 +42,7 @@
 #include "warnings.h"
 #include <libcxml/cxml.h>
 LIBDCP_DISABLE_WARNINGS
+#include <asdcp/Metadata.h>
 #include <libxml++/libxml++.h>
 LIBDCP_ENABLE_WARNINGS
 #include <boost/test/unit_test.hpp>
@@ -142,3 +143,125 @@ BOOST_AUTO_TEST_CASE (write_mca_descriptors_to_mxf_test)
 		);
 }
 
+
+static
+void
+check_mca_descriptors(int suffix, vector<dcp::Channel> extra_active_channels, vector<string> expected_mca_tag_symbols)
+{
+	auto const dir = boost::filesystem::path(dcp::String::compose("build/test/check_mca_descriptors_%1", suffix));
+	boost::filesystem::remove_all(dir);
+	boost::filesystem::create_directories(dir);
+
+	auto sound_asset = make_shared<dcp::SoundAsset>(dcp::Fraction(24, 1), 48000, 16, dcp::LanguageTag("en-US"), dcp::Standard::SMPTE);
+	auto writer = sound_asset->start_write(dir / "mxf.mxf", extra_active_channels, dcp::SoundAsset::AtmosSync::DISABLED, dcp::SoundAsset::MCASubDescriptors::ENABLED);
+
+	vector<vector<float>> samples(6);
+	float* pointers[6];
+	for (int i = 0; i < 6; ++i) {
+		samples[i].resize(2000);
+		pointers[i] = samples[i].data();
+	}
+	for (int i = 0; i < 24; ++i) {
+		writer->write(pointers, 6, 2000);
+	}
+	writer->finalize();
+
+	/* Check MXF */
+
+	auto reader = new ASDCP::PCM::MXFReader();
+	reader->OpenRead(boost::filesystem::path(dir / "mxf.mxf").string());
+
+	list<ASDCP::MXF::InterchangeObject*> channels;
+	auto const r = reader->OP1aHeader().GetMDObjectsByType(
+		dcp::asdcp_smpte_dict->ul(ASDCP::MDD_AudioChannelLabelSubDescriptor),
+		channels
+		);
+	BOOST_REQUIRE(KM_SUCCESS(r));
+
+	vector<string> mxf_mca_tag_symbols;
+	for (auto channel: channels) {
+		auto audio_channel = reinterpret_cast<ASDCP::MXF::AudioChannelLabelSubDescriptor*>(channel);
+		char buffer[64];
+		audio_channel->MCATagSymbol.EncodeString(buffer, sizeof(buffer));
+		mxf_mca_tag_symbols.push_back(buffer);
+	}
+
+	BOOST_CHECK(expected_mca_tag_symbols == mxf_mca_tag_symbols);
+
+	/* Check CPL */
+
+	auto reel_sound_asset = make_shared<dcp::ReelSoundAsset>(sound_asset, 0);
+	auto reel = make_shared<dcp::Reel>();
+	reel->add(black_picture_asset(dir / "dcp", 24));
+	reel->add(reel_sound_asset);
+
+	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_sample_rate(48000);
+	cpl.set_main_picture_stored_area(dcp::Size(1998, 1080));
+	cpl.set_main_picture_active_area(dcp::Size(1998, 1080));
+	cpl.write_xml(dir / "dcp" / "cpl.xml", shared_ptr<dcp::CertificateChain>());
+
+	cxml::Document check("CompositionPlaylist", dir / "dcp" / "cpl.xml");
+	vector<string> cpl_mca_tag_symbols;
+
+	for (auto node: check.node_child("ReelList")->node_child("Reel")->node_child("AssetList")->node_child("CompositionMetadataAsset")->node_child("MCASubDescriptors")->node_children("AudioChannelLabelSubDescriptor")) {
+		cpl_mca_tag_symbols.push_back(node->string_child("MCATagSymbol"));
+	}
+
+	BOOST_CHECK(expected_mca_tag_symbols == cpl_mca_tag_symbols);
+}
+
+
+BOOST_AUTO_TEST_CASE(write_correct_mca_descriptors)
+{
+	int suffix = 0;
+
+	check_mca_descriptors(
+		suffix++,
+		{}, { "chL", "chR", "chC", "chLFE", "chLs", "chRs" }
+		);
+
+	check_mca_descriptors(
+		suffix++,
+		{ dcp::Channel::HI }, { "chL", "chR", "chC", "chLFE", "chLs", "chRs", "chHI" }
+		);
+
+	check_mca_descriptors(
+		suffix++,
+		{ dcp::Channel::VI }, { "chL", "chR", "chC", "chLFE", "chLs", "chRs", "chVIN" }
+		);
+
+	check_mca_descriptors(
+		suffix++,
+		{ dcp::Channel::BSL }, { "chL", "chR", "chC", "chLFE", "chLss", "chRss", "chLrs" }
+		);
+
+	check_mca_descriptors(
+		suffix++,
+		{ dcp::Channel::BSR }, { "chL", "chR", "chC", "chLFE", "chLss", "chRss", "chRrs" }
+		);
+
+	check_mca_descriptors(
+		suffix++,
+		{ dcp::Channel::HI, dcp::Channel::VI }, { "chL", "chR", "chC", "chLFE", "chLs", "chRs", "chHI", "chVIN" }
+		);
+
+	check_mca_descriptors(
+		suffix++,
+		{ dcp::Channel::HI, dcp::Channel::VI, dcp::Channel::BSL, dcp::Channel::BSR }, { "chL", "chR", "chC", "chLFE", "chLss", "chRss", "chHI", "chVIN", "chLrs", "chRrs" }
+		);
+
+	check_mca_descriptors(
+		suffix++,
+		{ dcp::Channel::BSL, dcp::Channel::BSR }, { "chL", "chR", "chC", "chLFE", "chLss", "chRss", "chLrs", "chRrs" }
+		);
+
+	/* Duplicates should be ignored */
+	check_mca_descriptors(
+		suffix++,
+		{ dcp::Channel::HI, dcp::Channel::HI }, { "chL", "chR", "chC", "chLFE", "chLs", "chRs", "chHI" }
+		);
+}
+