Add channel details to high-audio-level hints (#822).

9 files changed, 169 insertions, 63 deletions

diff --git a/src/lib/analyse_audio_job.cc b/src/lib/analyse_audio_job.cc
index bfbf33a9d..9fce354df 100644
--- a/src/lib/analyse_audio_job.cc
+++ b/src/lib/analyse_audio_job.cc
@@ -41,6 +41,7 @@ extern "C" {
 #include "i18n.h"
 
 using std::string;
+using std::vector;
 using std::max;
 using std::min;
 using std::cout;
@@ -55,8 +56,8 @@ AnalyseAudioJob::AnalyseAudioJob (shared_ptr<const Film> film, shared_ptr<const
 	, _done (0)
 	, _samples_per_point (1)
 	, _current (0)
-	, _sample_peak (0)
-	, _sample_peak_frame (0)
+	, _sample_peak (new float[film->audio_channels()])
+	, _sample_peak_frame (new Frame[film->audio_channels()])
 #ifdef DCPOMATIC_HAVE_EBUR128_PATCHED_FFMPEG
 	, _ebur128 (new AudioFilterGraph (film->audio_frame_rate(), film->audio_channels()))
 #endif
@@ -73,6 +74,8 @@ AnalyseAudioJob::~AnalyseAudioJob ()
 		delete const_cast<Filter*> (i);
 	}
 	delete[] _current;
+	delete[] _sample_peak;
+	delete[] _sample_peak_frame;
 }
 
 string
@@ -127,14 +130,20 @@ AnalyseAudioJob::run ()
 		}
 	}
 
-	_analysis->set_sample_peak (_sample_peak, DCPTime::from_frames (_sample_peak_frame, _film->audio_frame_rate ()));
+	vector<AudioAnalysis::PeakTime> sample_peak;
+	for (int i = 0; i < _film->audio_channels(); ++i) {
+		sample_peak.push_back (
+			AudioAnalysis::PeakTime (_sample_peak[i], DCPTime::from_frames (_sample_peak_frame[i], _film->audio_frame_rate ()))
+			);
+	}
+	_analysis->set_sample_peak (sample_peak);
 
 #ifdef DCPOMATIC_HAVE_EBUR128_PATCHED_FFMPEG
 	if (Config::instance()->analyse_ebur128 ()) {
 		void* eb = _ebur128->get("Parsed_ebur128_0")->priv;
-		double true_peak = 0;
+		vector<float> true_peak;
 		for (int i = 0; i < _film->audio_channels(); ++i) {
-			true_peak = max (true_peak, av_ebur128_get_true_peaks(eb)[i]);
+			true_peak.push_back (av_ebur128_get_true_peaks(eb)[i]);
 		}
 		_analysis->set_true_peak (true_peak);
 		_analysis->set_integrated_loudness (av_ebur128_get_integrated_loudness(eb));
@@ -176,9 +185,9 @@ AnalyseAudioJob::analyse (shared_ptr<const AudioBuffers> b)
 			_current[j][AudioPoint::RMS] += pow (s, 2);
 			_current[j][AudioPoint::PEAK] = max (_current[j][AudioPoint::PEAK], as);
 
-			if (as > _sample_peak) {
-				_sample_peak = as;
-				_sample_peak_frame = _done + i;
+			if (as > _sample_peak[j]) {
+				_sample_peak[j] = as;
+				_sample_peak_frame[j] = _done + i;
 			}
 
 			if (((_done + i) % _samples_per_point) == 0) {
diff --git a/src/lib/analyse_audio_job.h b/src/lib/analyse_audio_job.h
index ce86e62cf..ee20bedc4 100644
--- a/src/lib/analyse_audio_job.h
+++ b/src/lib/analyse_audio_job.h
@@ -63,8 +63,8 @@ private:
 	int64_t _samples_per_point;
 	AudioPoint* _current;
 
-	float _sample_peak;
-	Frame _sample_peak_frame;
+	float* _sample_peak;
+	Frame* _sample_peak_frame;
 
 	boost::shared_ptr<AudioAnalysis> _analysis;
 
diff --git a/src/lib/audio_analysis.cc b/src/lib/audio_analysis.cc
index 1e5d08925..022c5935d 100644
--- a/src/lib/audio_analysis.cc
+++ b/src/lib/audio_analysis.cc
@@ -1,5 +1,5 @@
 /*
-    Copyright (C) 2012-2015 Carl Hetherington <cth@carlh.net>
+    Copyright (C) 2012-2016 Carl Hetherington <cth@carlh.net>
 
     This file is part of DCP-o-matic.
 
@@ -39,11 +39,16 @@ using std::string;
 using std::vector;
 using std::cout;
 using std::max;
+using std::pair;
+using std::make_pair;
 using std::list;
 using boost::shared_ptr;
+using boost::optional;
 using boost::dynamic_pointer_cast;
 using dcp::raw_convert;
 
+int const AudioAnalysis::_current_state_version = 2;
+
 AudioAnalysis::AudioAnalysis (int channels)
 {
 	_data.resize (channels);
@@ -54,6 +59,11 @@ AudioAnalysis::AudioAnalysis (boost::filesystem::path filename)
 	cxml::Document f ("AudioAnalysis");
 	f.read_file (filename);
 
+	if (f.optional_number_child<int>("Version").get_value_or(1) < _current_state_version) {
+		/* Too old.  Throw an exception so that this analysis is re-run. */
+		throw OldFormatError ("Audio analysis file is too old");
+	}
+
 	BOOST_FOREACH (cxml::NodePtr i, f.node_children ("Channel")) {
 		vector<AudioPoint> channel;
 
@@ -64,19 +74,18 @@ AudioAnalysis::AudioAnalysis (boost::filesystem::path filename)
 		_data.push_back (channel);
 	}
 
-	_sample_peak = f.optional_number_child<float> ("Peak");
-	if (!_sample_peak) {
-		/* New key */
-		_sample_peak = f.optional_number_child<float> ("SamplePeak");
+	BOOST_FOREACH (cxml::ConstNodePtr i, f.node_children ("SamplePeak")) {
+		_sample_peak.push_back (
+			PeakTime (
+				dcp::raw_convert<float>(i->content()), DCPTime(i->number_attribute<Frame>("Time"))
+				)
+			);
 	}
 
-	if (f.optional_number_child<DCPTime::Type> ("PeakTime")) {
-		_sample_peak_time = DCPTime (f.number_child<DCPTime::Type> ("PeakTime"));
-	} else if (f.optional_number_child<DCPTime::Type> ("SamplePeakTime")) {
-		_sample_peak_time = DCPTime (f.number_child<DCPTime::Type> ("SamplePeakTime"));
+	BOOST_FOREACH (cxml::ConstNodePtr i, f.node_children ("TruePeak")) {
+		_true_peak.push_back (dcp::raw_convert<float> (i->content ()));
 	}
 
-	_true_peak = f.optional_number_child<float> ("TruePeak");
 	_integrated_loudness = f.optional_number_child<float> ("IntegratedLoudness");
 	_loudness_range = f.optional_number_child<float> ("LoudnessRange");
 
@@ -116,6 +125,8 @@ AudioAnalysis::write (boost::filesystem::path filename)
 	shared_ptr<xmlpp::Document> doc (new xmlpp::Document);
 	xmlpp::Element* root = doc->create_root_node ("AudioAnalysis");
 
+	root->add_child("Version")->add_child_text (raw_convert<string> (_current_state_version));
+
 	BOOST_FOREACH (vector<AudioPoint>& i, _data) {
 		xmlpp::Element* channel = root->add_child ("Channel");
 		BOOST_FOREACH (AudioPoint& j, i) {
@@ -123,13 +134,14 @@ AudioAnalysis::write (boost::filesystem::path filename)
 		}
 	}
 
-	if (_sample_peak) {
-		root->add_child("SamplePeak")->add_child_text (raw_convert<string> (_sample_peak.get ()));
-		root->add_child("SamplePeakTime")->add_child_text (raw_convert<string> (_sample_peak_time.get().get ()));
+	for (size_t i = 0; i < _sample_peak.size(); ++i) {
+		xmlpp::Element* n = root->add_child("SamplePeak");
+		n->add_child_text (raw_convert<string> (_sample_peak[i].peak));
+		n->set_attribute ("Time", raw_convert<string> (_sample_peak[i].time.get()));
 	}
 
-	if (_true_peak) {
-		root->add_child("TruePeak")->add_child_text (raw_convert<string> (_true_peak.get ()));
+	BOOST_FOREACH (float i, _true_peak) {
+		root->add_child("TruePeak")->add_child_text (raw_convert<string> (i));
 	}
 
 	if (_integrated_loudness) {
@@ -161,3 +173,34 @@ AudioAnalysis::gain_correction (shared_ptr<const Playlist> playlist)
 
 	return 0.0f;
 }
+
+/** @return Peak across all channels, and the channel number it is on */
+pair<AudioAnalysis::PeakTime, int>
+AudioAnalysis::overall_sample_peak () const
+{
+	optional<PeakTime> pt;
+	int c;
+
+	for (size_t i = 0; i < _sample_peak.size(); ++i) {
+		if (!pt || _sample_peak[i].peak > pt->peak) {
+			pt = _sample_peak[i];
+			c = i;
+		}
+	}
+
+	return make_pair (pt.get(), c);
+}
+
+optional<float>
+AudioAnalysis::overall_true_peak () const
+{
+	optional<float> p;
+
+	BOOST_FOREACH (float i, _true_peak) {
+		if (!p || i > *p) {
+			p = i;
+		}
+	}
+
+	return p;
+}
diff --git a/src/lib/audio_analysis.h b/src/lib/audio_analysis.h
index 9acd491ce..a8ef4fb2d 100644
--- a/src/lib/audio_analysis.h
+++ b/src/lib/audio_analysis.h
@@ -42,12 +42,21 @@ public:
 
 	void add_point (int c, AudioPoint const & p);
 
-	void set_sample_peak (float peak, DCPTime time) {
+	struct PeakTime {
+		PeakTime (float p, DCPTime t)
+			: peak (p)
+			, time (t)
+		{}
+
+		float peak;
+		DCPTime time;
+	};
+
+	void set_sample_peak (std::vector<PeakTime> peak) {
 		_sample_peak = peak;
-		_sample_peak_time = time;
 	}
 
-	void set_true_peak (float peak) {
+	void set_true_peak (std::vector<float> peak) {
 		_true_peak = peak;
 	}
 
@@ -63,18 +72,18 @@ public:
 	int points (int c) const;
 	int channels () const;
 
-	boost::optional<float> sample_peak () const {
+	std::vector<PeakTime> sample_peak () const {
 		return _sample_peak;
 	}
 
-	boost::optional<DCPTime> sample_peak_time () const {
-		return _sample_peak_time;
-	}
+	std::pair<PeakTime, int> overall_sample_peak () const;
 
-	boost::optional<float> true_peak () const {
+	std::vector<float> true_peak () const {
 		return _true_peak;
 	}
 
+	boost::optional<float> overall_true_peak () const;
+
 	boost::optional<float> integrated_loudness () const {
 		return _integrated_loudness;
 	}
@@ -97,9 +106,8 @@ public:
 
 private:
 	std::vector<std::vector<AudioPoint> > _data;
-	boost::optional<float> _sample_peak;
-	boost::optional<DCPTime> _sample_peak_time;
-	boost::optional<float> _true_peak;
+	std::vector<PeakTime> _sample_peak;
+	std::vector<float> _true_peak;
 	boost::optional<float> _integrated_loudness;
 	boost::optional<float> _loudness_range;
 	/** If this analysis was run on a single piece of
@@ -107,6 +115,8 @@ private:
 	 *  happened.
 	 */
 	boost::optional<double> _analysis_gain;
+
+	static int const _current_state_version;
 };
 
 #endif
diff --git a/src/lib/exceptions.h b/src/lib/exceptions.h
index 75f4a8cd1..98727e0cc 100644
--- a/src/lib/exceptions.h
+++ b/src/lib/exceptions.h
@@ -241,4 +241,12 @@ public:
 	{}
 };
 
+class OldFormatError : public std::runtime_error
+{
+public:
+	OldFormatError (std::string s)
+		: std::runtime_error (s)
+	{}
+};
+
 #endif
diff --git a/src/lib/film.cc b/src/lib/film.cc
index e9788d09e..13a03d929 100644
--- a/src/lib/film.cc
+++ b/src/lib/film.cc
@@ -1390,24 +1390,12 @@ Film::audio_output_names () const
 	DCPOMATIC_ASSERT (MAX_DCP_AUDIO_CHANNELS == 16);
 
 	vector<string> n;
-	n.push_back (_("L"));
-	n.push_back (_("R"));
-	n.push_back (_("C"));
-	n.push_back (_("Lfe"));
-	n.push_back (_("Ls"));
-	n.push_back (_("Rs"));
-	n.push_back (_("HI"));
-	n.push_back (_("VI"));
-	n.push_back (_("Lc"));
-	n.push_back (_("Rc"));
-	n.push_back (_("BsL"));
-	n.push_back (_("BsR"));
-	n.push_back (_("DBP"));
-	n.push_back (_("DBS"));
-	n.push_back ("");
-	n.push_back ("");
-
-	return vector<string> (n.begin(), n.begin() + audio_channels ());
+
+	for (int i = 0; i < audio_channels(); ++i) {
+		n.push_back (short_audio_channel_name (i));
+	}
+
+	return n;
 }
 
 void
diff --git a/src/lib/hints.cc b/src/lib/hints.cc
index bf0f44096..d196b7ddb 100644
--- a/src/lib/hints.cc
+++ b/src/lib/hints.cc
@@ -28,6 +28,8 @@
 #include "ratio.h"
 #include "audio_analysis.h"
 #include "compose.hpp"
+#include "util.h"
+#include <dcp/raw_convert.h>
 #include <boost/foreach.hpp>
 #include <boost/algorithm/string.hpp>
 
@@ -128,15 +130,30 @@ get_hints (shared_ptr<const Film> film)
 	boost::filesystem::path path = film->audio_analysis_path (film->playlist ());
 	if (boost::filesystem::exists (path)) {
 		shared_ptr<AudioAnalysis> an (new AudioAnalysis (path));
-		if (an->sample_peak() || an->true_peak()) {
-			float const peak = max (an->sample_peak().get_value_or(0), an->true_peak().get_value_or(0));
+
+		string ch;
+
+		vector<AudioAnalysis::PeakTime> sample_peak = an->sample_peak ();
+		vector<float> true_peak = an->true_peak ();
+
+		for (size_t i = 0; i < sample_peak.size(); ++i) {
+			float const peak = max (sample_peak[i].peak, true_peak.empty() ? 0 : true_peak[i]);
 			float const peak_dB = 20 * log10 (peak) + an->gain_correction (film->playlist ());
-			if (peak_dB > -3 && peak_dB < -0.5) {
-				hints.push_back (_("Your audio level is very high.  You should reduce the gain of your audio content."));
-			} else if (peak_dB > -0.5) {
-				hints.push_back (_("Your audio level is very close to clipping.  You should reduce the gain of your audio content."));
+			if (peak_dB > -3) {
+				ch += dcp::raw_convert<string> (short_audio_channel_name (i)) + ", ";
 			}
 		}
+
+		ch = ch.substr (0, ch.length() - 2);
+
+		if (!ch.empty ()) {
+			hints.push_back (
+				String::compose (
+					_("Your audio level is very high (on %1).  You should reduce the gain of your audio content."),
+					ch
+					)
+				);
+		}
 	}
 
 	return hints;
diff --git a/src/lib/util.cc b/src/lib/util.cc
index e497ecf3c..71b48a520 100644
--- a/src/lib/util.cc
+++ b/src/lib/util.cc
@@ -485,8 +485,7 @@ audio_channel_name (int c)
 	DCPOMATIC_ASSERT (MAX_DCP_AUDIO_CHANNELS == 16);
 
 	/// TRANSLATORS: these are the names of audio channels; Lfe (sub) is the low-frequency
-	/// enhancement channel (sub-woofer).  HI is the hearing-impaired audio track and
-	/// VI is the visually-impaired audio track (audio describe).
+	/// enhancement channel (sub-woofer).
 	string const channels[] = {
 		_("Left"),
 		_("Right"),
@@ -509,6 +508,37 @@ audio_channel_name (int c)
 	return channels[c];
 }
 
+string
+short_audio_channel_name (int c)
+{
+	DCPOMATIC_ASSERT (MAX_DCP_AUDIO_CHANNELS == 16);
+
+	/// TRANSLATORS: these are short names of audio channels; Lfe is the low-frequency
+	/// enhancement channel (sub-woofer).  HI is the hearing-impaired audio track and
+	/// VI is the visually-impaired audio track (audio describe).
+	string const channels[] = {
+		_("L"),
+		_("R"),
+		_("C"),
+		_("Lfe"),
+		_("Ls"),
+		_("Rs"),
+		_("HI"),
+		_("VI"),
+		_("Lc"),
+		_("Rc"),
+		_("BsL"),
+		_("BsR"),
+		_("DBP"),
+		_("DBPS"),
+		_(""),
+		_("")
+	};
+
+	return channels[c];
+}
+
+
 bool
 valid_image_file (boost::filesystem::path f)
 {
diff --git a/src/lib/util.h b/src/lib/util.h
index e786bb9c1..f9b4d0e05 100644
--- a/src/lib/util.h
+++ b/src/lib/util.h
@@ -1,5 +1,5 @@
 /*
-    Copyright (C) 2012-2015 Carl Hetherington <cth@carlh.net>
+    Copyright (C) 2012-2016 Carl Hetherington <cth@carlh.net>
 
     This file is part of DCP-o-matic.
 
@@ -64,6 +64,7 @@ extern void dcpomatic_setup_gettext_i18n (std::string);
 extern std::string digest_head_tail (std::vector<boost::filesystem::path>, boost::uintmax_t size);
 extern void ensure_ui_thread ();
 extern std::string audio_channel_name (int);
+extern std::string short_audio_channel_name (int);
 extern bool valid_image_file (boost::filesystem::path);
 extern bool valid_j2k_file (boost::filesystem::path);
 #ifdef DCPOMATIC_WINDOWS