All audio content should resample if the output frame rate and content

frame rates differ; make Sndfile sources use the video-frame-rate-based
calculation for output frame rate (like FFmpeg sources do).  Also, fix
small problems when flushing Resamplers used to get fed back to
Resamplers again.

12 files changed, 121 insertions, 56 deletions

diff --git a/ChangeLog b/ChangeLog
index 4d066e657..a0e776b0e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,8 @@
 2014-06-03  Carl Hetherington  <cth@carlh.net>
 
+	* Fix bad resampling of separate sound file sources that
+	have specified video frame rates.
+
 	* Version 1.69.20 released.
 
 2014-06-03  Carl Hetherington  <cth@carlh.net>
diff --git a/src/lib/audio_content.cc b/src/lib/audio_content.cc
index dbca6652c..79912f1ae 100644
--- a/src/lib/audio_content.cc
+++ b/src/lib/audio_content.cc
@@ -29,6 +29,7 @@
 #include "i18n.h"
 
 using std::string;
+using std::cout;
 using std::vector;
 using boost::shared_ptr;
 using boost::dynamic_pointer_cast;
@@ -148,3 +149,28 @@ AudioContent::technical_summary () const
 {
 	return String::compose ("audio: channels %1, length %2, raw rate %3, out rate %4", audio_channels(), audio_length(), content_audio_frame_rate(), output_audio_frame_rate());
 }
+
+int
+AudioContent::output_audio_frame_rate () const
+{
+	shared_ptr<const Film> film = _film.lock ();
+	assert (film);
+	
+	/* Resample to a DCI-approved sample rate */
+	double t = dcp_audio_frame_rate (content_audio_frame_rate ());
+
+	FrameRateConversion frc (video_frame_rate(), film->video_frame_rate());
+
+	/* Compensate if the DCP is being run at a different frame rate
+	   to the source; that is, if the video is run such that it will
+	   look different in the DCP compared to the source (slower or faster).
+	   skip/repeat doesn't come into effect here.
+	*/
+
+	if (frc.change_speed) {
+		t *= video_frame_rate() * frc.factor() / film->video_frame_rate();
+	}
+
+	return rint (t);
+}
+
diff --git a/src/lib/audio_content.h b/src/lib/audio_content.h
index d30db02d7..2a1216d86 100644
--- a/src/lib/audio_content.h
+++ b/src/lib/audio_content.h
@@ -54,11 +54,12 @@ public:
 	virtual int audio_channels () const = 0;
 	virtual AudioContent::Frame audio_length () const = 0;
 	virtual int content_audio_frame_rate () const = 0;
-	virtual int output_audio_frame_rate () const = 0;
 	virtual AudioMapping audio_mapping () const = 0;
 	virtual void set_audio_mapping (AudioMapping) = 0;
 	virtual boost::filesystem::path audio_analysis_path () const;
 
+	int output_audio_frame_rate () const;
+	
 	boost::signals2::connection analyse_audio (boost::function<void()>);
 
 	void set_audio_gain (float);
diff --git a/src/lib/content.h b/src/lib/content.h
index 596a0a905..3cea93cfc 100644
--- a/src/lib/content.h
+++ b/src/lib/content.h
@@ -65,6 +65,8 @@ public:
 	virtual void as_xml (xmlpp::Node *) const;
 	virtual Time full_length () const = 0;
 	virtual std::string identifier () const;
+	/** @return the video frame rate that this content has or was prepared to be used with */
+	virtual float video_frame_rate () const = 0;
 
 	boost::shared_ptr<Content> clone () const;
 
diff --git a/src/lib/ffmpeg_content.cc b/src/lib/ffmpeg_content.cc
index 41a3724a2..4ea6dbc6a 100644
--- a/src/lib/ffmpeg_content.cc
+++ b/src/lib/ffmpeg_content.cc
@@ -309,30 +309,6 @@ FFmpegContent::content_audio_frame_rate () const
 	return _audio_stream->frame_rate;
 }
 
-int
-FFmpegContent::output_audio_frame_rate () const
-{
-	shared_ptr<const Film> film = _film.lock ();
-	assert (film);
-	
-	/* Resample to a DCI-approved sample rate */
-	double t = dcp_audio_frame_rate (content_audio_frame_rate ());
-
-	FrameRateConversion frc (video_frame_rate(), film->video_frame_rate());
-
-	/* Compensate if the DCP is being run at a different frame rate
-	   to the source; that is, if the video is run such that it will
-	   look different in the DCP compared to the source (slower or faster).
-	   skip/repeat doesn't come into effect here.
-	*/
-
-	if (frc.change_speed) {
-		t *= video_frame_rate() * frc.factor() / film->video_frame_rate();
-	}
-
-	return rint (t);
-}
-
 bool
 operator== (FFmpegStream const & a, FFmpegStream const & b)
 {
diff --git a/src/lib/ffmpeg_content.h b/src/lib/ffmpeg_content.h
index 6ab95d2fe..c15e5c10e 100644
--- a/src/lib/ffmpeg_content.h
+++ b/src/lib/ffmpeg_content.h
@@ -147,7 +147,6 @@ public:
 	int audio_channels () const;
 	AudioContent::Frame audio_length () const;
 	int content_audio_frame_rate () const;
-	int output_audio_frame_rate () const;
 	AudioMapping audio_mapping () const;
 	void set_audio_mapping (AudioMapping);
 	boost::filesystem::path audio_analysis_path () const;
diff --git a/src/lib/player.cc b/src/lib/player.cc
index b112760ef..68df8ea70 100644
--- a/src/lib/player.cc
+++ b/src/lib/player.cc
@@ -149,7 +149,7 @@ Player::pass ()
 				if (re) {
 					shared_ptr<const AudioBuffers> b = re->flush ();
 					if (b->frames ()) {
-						process_audio (earliest, b, ac->audio_length ());
+						process_audio (earliest, b, ac->audio_length (), true);
 					}
 				}
 			}
@@ -269,8 +269,9 @@ Player::process_video (weak_ptr<Piece> weak_piece, shared_ptr<const ImageProxy>
 	}
 }
 
+/** @param already_resampled true if this data has already been through the chain up to the resampler */
 void
-Player::process_audio (weak_ptr<Piece> weak_piece, shared_ptr<const AudioBuffers> audio, AudioContent::Frame frame)
+Player::process_audio (weak_ptr<Piece> weak_piece, shared_ptr<const AudioBuffers> audio, AudioContent::Frame frame, bool already_resampled)
 {
 	shared_ptr<Piece> piece = weak_piece.lock ();
 	if (!piece) {
@@ -280,19 +281,21 @@ Player::process_audio (weak_ptr<Piece> weak_piece, shared_ptr<const AudioBuffers
 	shared_ptr<AudioContent> content = dynamic_pointer_cast<AudioContent> (piece->content);
 	assert (content);
 
-	/* Gain */
-	if (content->audio_gain() != 0) {
-		shared_ptr<AudioBuffers> gain (new AudioBuffers (audio));
-		gain->apply_gain (content->audio_gain ());
-		audio = gain;
-	}
-
-	/* Resample */
-	if (content->content_audio_frame_rate() != content->output_audio_frame_rate()) {
-		shared_ptr<Resampler> r = resampler (content, true);
-		pair<shared_ptr<const AudioBuffers>, AudioContent::Frame> ro = r->run (audio, frame);
-		audio = ro.first;
-		frame = ro.second;
+	if (!already_resampled) {
+		/* Gain */
+		if (content->audio_gain() != 0) {
+			shared_ptr<AudioBuffers> gain (new AudioBuffers (audio));
+			gain->apply_gain (content->audio_gain ());
+			audio = gain;
+		}
+		
+		/* Resample */
+		if (content->content_audio_frame_rate() != content->output_audio_frame_rate()) {
+			shared_ptr<Resampler> r = resampler (content, true);
+			pair<shared_ptr<const AudioBuffers>, AudioContent::Frame> ro = r->run (audio, frame);
+			audio = ro.first;
+			frame = ro.second;
+		}
 	}
 	
 	Time const relative_time = _film->audio_frames_to_time (frame);
@@ -427,7 +430,7 @@ Player::setup_pieces ()
 			shared_ptr<FFmpegDecoder> fd (new FFmpegDecoder (_film, fc, _video, _audio));
 			
 			fd->Video.connect (bind (&Player::process_video, this, weak_ptr<Piece> (piece), _1, _2, _3, _4, _5, 0));
-			fd->Audio.connect (bind (&Player::process_audio, this, weak_ptr<Piece> (piece), _1, _2));
+			fd->Audio.connect (bind (&Player::process_audio, this, weak_ptr<Piece> (piece), _1, _2, false));
 			fd->Subtitle.connect (bind (&Player::process_subtitle, this, weak_ptr<Piece> (piece), _1, _2, _3, _4));
 
 			fd->seek (fc->time_to_content_video_frames (fc->trim_start ()), true);
@@ -457,7 +460,7 @@ Player::setup_pieces ()
 		shared_ptr<const SndfileContent> sc = dynamic_pointer_cast<const SndfileContent> (*i);
 		if (sc) {
 			shared_ptr<AudioDecoder> sd (new SndfileDecoder (_film, sc));
-			sd->Audio.connect (bind (&Player::process_audio, this, weak_ptr<Piece> (piece), _1, _2));
+			sd->Audio.connect (bind (&Player::process_audio, this, weak_ptr<Piece> (piece), _1, _2, false));
 
 			piece->decoder = sd;
 		}
@@ -555,7 +558,7 @@ Player::resampler (shared_ptr<AudioContent> c, bool create)
 	LOG_GENERAL (
 		"Creating new resampler for %1 to %2 with %3 channels", c->content_audio_frame_rate(), c->output_audio_frame_rate(), c->audio_channels()
 		);
-	
+
 	shared_ptr<Resampler> r (new Resampler (c->content_audio_frame_rate(), c->output_audio_frame_rate(), c->audio_channels()));
 	_resamplers[c] = r;
 	return r;
diff --git a/src/lib/player.h b/src/lib/player.h
index bf6260c0a..6e70ad707 100644
--- a/src/lib/player.h
+++ b/src/lib/player.h
@@ -87,7 +87,7 @@ private:
 	friend class Piece;
 
 	void process_video (boost::weak_ptr<Piece>, boost::shared_ptr<const ImageProxy>, Eyes, Part, bool, VideoContent::Frame, Time);
-	void process_audio (boost::weak_ptr<Piece>, boost::shared_ptr<const AudioBuffers>, AudioContent::Frame);
+	void process_audio (boost::weak_ptr<Piece>, boost::shared_ptr<const AudioBuffers>, AudioContent::Frame, bool);
 	void process_subtitle (boost::weak_ptr<Piece>, boost::shared_ptr<Image>, dcpomatic::Rect<double>, Time, Time);
 	void setup_pieces ();
 	void playlist_changed ();
diff --git a/src/lib/sndfile_content.cc b/src/lib/sndfile_content.cc
index fcdf88778..6f18ebbb4 100644
--- a/src/lib/sndfile_content.cc
+++ b/src/lib/sndfile_content.cc
@@ -159,15 +159,6 @@ SndfileContent::full_length () const
 	return film->audio_frames_to_time (len);
 }
 
-int
-SndfileContent::output_audio_frame_rate () const
-{
-	shared_ptr<const Film> film = _film.lock ();
-	assert (film);
-	
-	return film->audio_frame_rate ();
-}
-
 void
 SndfileContent::set_audio_mapping (AudioMapping m)
 {
diff --git a/src/lib/sndfile_content.h b/src/lib/sndfile_content.h
index a79fb99b6..dcfdfd8d7 100644
--- a/src/lib/sndfile_content.h
+++ b/src/lib/sndfile_content.h
@@ -68,8 +68,6 @@ public:
 		return _audio_frame_rate;
 	}
 
-	int output_audio_frame_rate () const;
-
 	AudioMapping audio_mapping () const {
 		boost::mutex::scoped_lock lm (_mutex);
 		return _audio_mapping;
diff --git a/test/audio_with_specified_video_frame_rate_test.cc b/test/audio_with_specified_video_frame_rate_test.cc
new file mode 100644
index 000000000..986033205
--- /dev/null
+++ b/test/audio_with_specified_video_frame_rate_test.cc
@@ -0,0 +1,65 @@
+/*
+    Copyright (C) 2014 Carl Hetherington <cth@carlh.net>
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 2 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program; if not, write to the Free Software
+    Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+
+*/
+
+#include <boost/test/unit_test.hpp>
+#include "lib/film.h"
+#include "lib/dcp_content_type.h"
+#include "lib/ratio.h"
+#include "lib/sndfile_content.h"
+#include "lib/audio_buffers.h"
+#include "lib/player.h"
+#include "test.h"
+
+using std::cout;
+using boost::shared_ptr;
+
+static
+void
+process_audio (shared_ptr<const AudioBuffers> buffers, int* samples)
+{
+	cout << "weeeeeeeeee " << buffers->frames() << "\n";
+	*samples += buffers->frames ();
+}
+
+/** Test the situation where a piece of SndfileContent has its video
+ *  frame rate specified (i.e. the rate that it was prepared for),
+ *  and hence might need resampling.
+ */
+BOOST_AUTO_TEST_CASE (audio_with_specified_video_frame_rate_test)
+{
+	/* Make a film using staircase.wav with the DCP at 30fps and the audio specified
+	   as being prepared for 29.97.
+	*/
+	
+	shared_ptr<Film> film = new_test_film ("audio_with_specified_video_frame_rate_test");
+	film->set_dcp_content_type (DCPContentType::from_dci_name ("FTR"));
+	film->set_container (Ratio::from_id ("185"));
+	film->set_name ("audio_with_specified_video_frame_rate_test");
+
+	shared_ptr<SndfileContent> content (new SndfileContent (film, "test/data/sine_440.wav"));
+	content->set_video_frame_rate (29.97);
+	film->examine_and_add_content (content);
+	
+	wait_for_jobs ();
+
+	film->set_video_frame_rate (30);
+
+	BOOST_CHECK_EQUAL (content->content_audio_frame_rate(), 48000);
+	BOOST_CHECK_EQUAL (content->output_audio_frame_rate(), 47952);
+}
diff --git a/test/wscript b/test/wscript
index 676f47104..6f1490a15 100644
--- a/test/wscript
+++ b/test/wscript
@@ -20,6 +20,7 @@ def build(bld):
                  audio_delay_test.cc
                  audio_mapping_test.cc
                  audio_merger_test.cc
+                 audio_with_specified_video_frame_rate_test.cc
                  black_fill_test.cc
                  client_server_test.cc
                  colour_conversion_test.cc