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.

diff --git a/ChangeLog b/ChangeLog
index 4d066e6572c27d7358730db21bfe21eebe8a7aaa..a0e776b0e0bcf66a52516f815f9ec976c8fda873 100644 (file)
--- 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 dbca6652ca1095abb78fb8d2ca2494d4baf9b2d2..79912f1aee7ec4eb1f86a2b208fd33eb0bba6c75 100644 (file)
--- 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 d30db02d775709d270eb75eed7a421aa1c877907..2a1216d86f81bd63596ef63c21b862a0a6747904 100644 (file)
--- 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 596a0a905c95217d4daaf8ba116b8a6f518c6555..3cea93cfcf3ef11be45d3bffc890beeb656d0840 100644 (file)
--- 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 41a3724a29cfee6b19523a776fd0edcb773542ea..4ea6dbc6ac4c23f9c3f2cefd747848968b8a3749 100644 (file)
--- 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 6ab95d2fe9d2bf53a08acd050845721ed40fc1a1..c15e5c10ec0edc66d61db8238739d2d259f67ec1 100644 (file)
--- 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 b112760ef412a024fa581c9779ca5be4b1c65138..68df8ea709afd905955be97dd410366092f5a612 100644 (file)
--- 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 bf6260c0a912260d71a7203a0a6f845e20182db2..6e70ad707cf79052cd24a64cbc0c1344020a2e1c 100644 (file)
--- 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 fcdf88778ac81173080aebfbda9cbf832d8a169c..6f18ebbb4ce2d14754196d04c1317d15c61c876f 100644 (file)
--- 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 a79fb99b6bd01f6497d6e3dd2d2184c2c57d5ec1..dcfdfd8d7b199eba9730f154951187d36554eb4d 100644 (file)
--- 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 (file)
index 0000000..9860332
--- /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 676f471049cfcf835e5da2bde766ef3ee053f687..6f1490a152c4980d18a0787fd4e743d55e5b6e25 100644 (file)
--- 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