Fix incorrectly-timed emission of silence padding causing buffer fill (#2217).

On initialisation or after seek we insert silence corresponding to
a positive delay in an audio stream.  Previously this inserted silence
was done at time 0, so that after a seek to time T the silent frames
would come out of the audio merger at time 0 and then the player would
fill the space up to time T with silence.  If T was far enough along
this would fill the audio buffers without there being any video.

diff --git a/src/lib/audio_decoder.cc b/src/lib/audio_decoder.cc
index 2d02043b5948d1071d0de18982e14abfcf03c12e..664a56c2a08a08dcaaba00632268eca9c08dc39c 100644 (file)
--- a/src/lib/audio_decoder.cc
+++ b/src/lib/audio_decoder.cc
@@ -69,22 +69,20 @@ AudioDecoder::emit (shared_ptr<const Film> film, AudioStreamPtr stream, shared_p
                time += ContentTime::from_seconds (_content->delay() / 1000.0);
        }
 
-       auto reset = false;
-       if (_positions[stream] == 0) {
-               /* This is the first data we have received since initialisation or seek.  Set
-                  the position based on the ContentTime that was given.  After this first time
-                  we just count samples unless the timestamp is more than slack_frames away
-                  from where we think it should be.  This is because ContentTimes seem to be
-                  slightly unreliable from FFmpegDecoder (i.e. not sample accurate), but we still
-                  need to obey them sometimes otherwise we get sync problems such as #1833.
-               */
-               if (_content->delay() > 0) {
-                       /* Insert silence to give the delay */
-                       silence (_content->delay ());
-               }
-               reset = true;
-       } else if (std::abs(_positions[stream] - time.frames_round(resampled_rate)) > slack_frames) {
-               reset = true;
+       /* first_since_seek is set to true if this is the first data we have
+          received since initialisation or seek.  We'll set the position based
+          on the ContentTime that was given.  After this first time we just
+          count samples unless the timestamp is more than slack_frames away
+          from where we think it should be.  This is because ContentTimes seem
+          to be slightly unreliable from FFmpegDecoder (i.e.  not sample
+          accurate), but we still need to obey them sometimes otherwise we get
+          sync problems such as #1833.
+       */
+
+       auto const first_since_seek = _positions[stream] == 0;
+       auto const need_reset = !first_since_seek && (std::abs(_positions[stream] - time.frames_round(resampled_rate)) > slack_frames);
+
+       if (need_reset) {
                LOG_GENERAL (
                        "Reset audio position: was %1, new data at %2, slack: %3 frames",
                        _positions[stream],
@@ -93,10 +91,14 @@ AudioDecoder::emit (shared_ptr<const Film> film, AudioStreamPtr stream, shared_p
                        );
        }
 
-       if (reset) {
+       if (first_since_seek || need_reset) {
                _positions[stream] = time.frames_round (resampled_rate);
        }
 
+       if (first_since_seek && _content->delay() > 0) {
+               silence (stream, _content->delay());
+       }
+
        shared_ptr<Resampler> resampler;
        auto i = _resamplers.find(stream);
        if (i != _resamplers.end()) {
@@ -183,18 +185,18 @@ AudioDecoder::flush ()
 
        if (_content->delay() < 0) {
                /* Finish off with the gap caused by the delay */
-               silence (-_content->delay ());
+               for (auto stream: _content->streams()) {
+                       silence (stream, -_content->delay());
+               }
        }
 }
 
 
 void
-AudioDecoder::silence (int milliseconds)
+AudioDecoder::silence (AudioStreamPtr stream, int milliseconds)
 {
-       for (auto i: _content->streams()) {
-               int const samples = ContentTime::from_seconds(milliseconds / 1000.0).frames_round(i->frame_rate());
-               auto silence = make_shared<AudioBuffers>(i->channels(), samples);
-               silence->make_silent ();
-               Data (i, ContentAudio (silence, _positions[i]));
-       }
+       int const samples = ContentTime::from_seconds(milliseconds / 1000.0).frames_round(stream->frame_rate());
+       auto silence = make_shared<AudioBuffers>(stream->channels(), samples);
+       silence->make_silent ();
+       Data (stream, ContentAudio(silence, _positions[stream]));
 }

diff --git a/src/lib/audio_decoder.h b/src/lib/audio_decoder.h
index 81b48b73cb7ff7aaafcf8a86ff18a4bea28fa744..a2e06f11ceed15773f9987eb96e3017eddca1ed0 100644 (file)
--- a/src/lib/audio_decoder.h
+++ b/src/lib/audio_decoder.h
@@ -61,7 +61,7 @@ public:
        boost::signals2::signal<void (AudioStreamPtr, ContentAudio)> Data;
 
 private:
-       void silence (int milliseconds);
+       void silence (AudioStreamPtr stream, int milliseconds);
 
        std::shared_ptr<const AudioContent> _content;
        /** Frame after the last one that was emitted from Data (i.e. at the resampled rate, if applicable)

diff --git a/test/butler_test.cc b/test/butler_test.cc
index 3d524a3b258382d98f82786224fae38a69793eb2..bee66217f31c432de7a6fb419cfa587db1dbeab5 100644 (file)
--- a/test/butler_test.cc
+++ b/test/butler_test.cc
@@ -1,5 +1,5 @@
 /*
-    Copyright (C) 2017-2021 Carl Hetherington <cth@carlh.net>
+    Copyright (C) 2017-2022 Carl Hetherington <cth@carlh.net>
 
     This file is part of DCP-o-matic.
 
@@ -19,13 +19,14 @@
 */
 
 
+#include "lib/audio_content.h"
+#include "lib/audio_mapping.h"
 #include "lib/butler.h"
-#include "lib/film.h"
-#include "lib/dcp_content_type.h"
-#include "lib/ratio.h"
 #include "lib/content_factory.h"
-#include "lib/audio_mapping.h"
+#include "lib/dcp_content_type.h"
+#include "lib/film.h"
 #include "lib/player.h"
+#include "lib/ratio.h"
 #include "test.h"
 #include <boost/test/unit_test.hpp>
 
@@ -77,3 +78,38 @@ BOOST_AUTO_TEST_CASE (butler_test1)
                BOOST_REQUIRE_EQUAL (buffer[i * 6 + 5], 0);
        }
 }
+
+
+BOOST_AUTO_TEST_CASE (butler_test2)
+{
+       auto content = content_factory(TestPaths::private_data() / "arrietty_JP-EN.mkv");
+       BOOST_REQUIRE (!content.empty());
+       auto film = new_test_film2 ("butler_test2", { content.front() });
+       BOOST_REQUIRE (content.front()->audio);
+       content.front()->audio->set_delay(100);
+
+       /* This is the map of the player output (5.1) to the butler output (also 5.1) */
+       auto map = AudioMapping (6, 6);
+       for (int i = 0; i < 6; ++i) {
+               map.set (i, i, 1);
+       }
+
+       Butler butler (film, make_shared<Player>(film, Image::Alignment::COMPACT), map, 6, bind(&PlayerVideo::force, _1, AV_PIX_FMT_RGB24), VideoRange::FULL, Image::Alignment::COMPACT, false, false);
+
+       int const audio_frames_per_video_frame = 48000 / 25;
+       float audio_buffer[audio_frames_per_video_frame * 6];
+       for (int i = 0; i < 16; ++i) {
+               butler.get_video(Butler::Behaviour::BLOCKING, 0);
+               butler.get_audio(Butler::Behaviour::BLOCKING, audio_buffer, audio_frames_per_video_frame);
+       }
+
+       butler.seek (DCPTime::from_seconds(60), false);
+
+       for (int i = 0; i < 240; ++i) {
+               butler.get_video(Butler::Behaviour::BLOCKING, 0);
+               butler.get_audio(Butler::Behaviour::BLOCKING, audio_buffer, audio_frames_per_video_frame);
+       }
+
+       butler.rethrow();
+}
+