From: Carl Hetherington Date: Mon, 14 Mar 2022 19:29:44 +0000 (+0100) Subject: Fix incorrectly-timed emission of silence padding causing buffer fill (#2217). X-Git-Tag: v2.16.8~32 X-Git-Url: https://git.carlh.net/gitweb/?p=dcpomatic.git;a=commitdiff_plain;h=0c846af54055b9915c6c68617cd28176d5f84351 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 2d02043b5..664a56c2a 100644 --- a/src/lib/audio_decoder.cc +++ b/src/lib/audio_decoder.cc @@ -69,22 +69,20 @@ AudioDecoder::emit (shared_ptr 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 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; 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(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(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 81b48b73c..a2e06f11c 100644 --- a/src/lib/audio_decoder.h +++ b/src/lib/audio_decoder.h @@ -61,7 +61,7 @@ public: boost::signals2::signal Data; private: - void silence (int milliseconds); + void silence (AudioStreamPtr stream, int milliseconds); std::shared_ptr _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 3d524a3b2..bee66217f 100644 --- a/test/butler_test.cc +++ b/test/butler_test.cc @@ -1,5 +1,5 @@ /* - Copyright (C) 2017-2021 Carl Hetherington + Copyright (C) 2017-2022 Carl Hetherington 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 @@ -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(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(); +} +