From 8798bfab1539c3ac6031bb325e1b181c58b13fc5 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Sun, 29 Nov 2020 00:51:38 +0100 Subject: [PATCH] Obey audio timestamps if they don't deviate by more than some threshold. Previously we would ignore audio timestamps because they are not contiguous in a sample-accurate way. However with bugs like #1833 we do need to obey large discontinuities in audio timestamps, otherwise we get large sync errors. Here we change timestamp handling to ignore small discontinuities in timestamps but not larger ones. --- src/lib/audio_decoder.cc | 42 +++++++++++++++++++++++++++++++-------- src/lib/audio_decoder.h | 2 +- src/lib/ffmpeg_decoder.cc | 2 +- test/content_test.cc | 26 ++++++++++++++++++++++++ 4 files changed, 62 insertions(+), 10 deletions(-) diff --git a/src/lib/audio_decoder.cc b/src/lib/audio_decoder.cc index a5e86f22b..051fb4dd8 100644 --- a/src/lib/audio_decoder.cc +++ b/src/lib/audio_decoder.cc @@ -48,25 +48,51 @@ AudioDecoder::AudioDecoder (Decoder* parent, shared_ptr cont } } +/** @param time_already_delayed true if the delay should not be added to time */ void -AudioDecoder::emit (shared_ptr film, AudioStreamPtr stream, shared_ptr data, ContentTime time) +AudioDecoder::emit (shared_ptr film, AudioStreamPtr stream, shared_ptr data, ContentTime time, bool time_already_delayed) { if (ignore ()) { return; } + /* Amount of error we will tolerate on audio timestamps; see comment below. + * We'll use 1 24fps video frame at 48kHz as this seems to be roughly how + * ffplay does it. + */ + static Frame const slack_frames = 48000 / 24; + + int const resampled_rate = _content->resampled_frame_rate(film); + if (!time_already_delayed) { + time += ContentTime::from_seconds (_content->delay() / 1000.0); + } + + bool 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, as it seems that ContentTimes are unreliable from - FFmpegDecoder (not quite continuous; perhaps due to some rounding error). + 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 ()); } - time += ContentTime::from_seconds (_content->delay() / 1000.0); - _positions[stream] = time.frames_round (_content->resampled_frame_rate(film)); + reset = true; + } else if (std::abs(_positions[stream] - time.frames_round(resampled_rate)) > slack_frames) { + reset = true; + LOG_GENERAL ( + "Reset audio position: was %1, new data at %2, slack: %3 frames", + _positions[stream], + time.frames_round(resampled_rate), + std::abs(_positions[stream] - time.frames_round(resampled_rate)) + ); + } + + if (reset) { + _positions[stream] = time.frames_round (resampled_rate); } shared_ptr resampler; @@ -74,15 +100,15 @@ AudioDecoder::emit (shared_ptr film, AudioStreamPtr stream, shared_p if (i != _resamplers.end ()) { resampler = i->second; } else { - if (stream->frame_rate() != _content->resampled_frame_rate(film)) { + if (stream->frame_rate() != resampled_rate) { LOG_GENERAL ( "Creating new resampler from %1 to %2 with %3 channels", stream->frame_rate(), - _content->resampled_frame_rate(film), + resampled_rate, stream->channels() ); - resampler.reset (new Resampler (stream->frame_rate(), _content->resampled_frame_rate(film), stream->channels())); + resampler.reset (new Resampler(stream->frame_rate(), resampled_rate, stream->channels())); if (_fast) { resampler->set_fast (); } diff --git a/src/lib/audio_decoder.h b/src/lib/audio_decoder.h index 8bd65a671..897ddb7e4 100644 --- a/src/lib/audio_decoder.h +++ b/src/lib/audio_decoder.h @@ -48,7 +48,7 @@ public: AudioDecoder (Decoder* parent, boost::shared_ptr content, bool fast); boost::optional position (boost::shared_ptr film) const; - void emit (boost::shared_ptr film, AudioStreamPtr stream, boost::shared_ptr, dcpomatic::ContentTime); + void emit (boost::shared_ptr film, AudioStreamPtr stream, boost::shared_ptr, dcpomatic::ContentTime, bool time_already_delayed = false); void seek (); void flush (); diff --git a/src/lib/ffmpeg_decoder.cc b/src/lib/ffmpeg_decoder.cc index 41b93dad7..0d6539061 100644 --- a/src/lib/ffmpeg_decoder.cc +++ b/src/lib/ffmpeg_decoder.cc @@ -145,7 +145,7 @@ FFmpegDecoder::flush () ContentTime to_do = min (full_length - a, ContentTime::from_seconds (0.1)); shared_ptr silence (new AudioBuffers (i->channels(), to_do.frames_ceil (i->frame_rate()))); silence->make_silent (); - audio->emit (film(), i, silence, a); + audio->emit (film(), i, silence, a, true); a += to_do; } } diff --git a/test/content_test.cc b/test/content_test.cc index 23fb23865..de9ca7774 100644 --- a/test/content_test.cc +++ b/test/content_test.cc @@ -23,6 +23,7 @@ * @ingroup completedcp */ +#include "lib/audio_content.h" #include "lib/film.h" #include "lib/dcp_content_type.h" #include "lib/content_factory.h" @@ -150,3 +151,28 @@ BOOST_AUTO_TEST_CASE (content_test5) audio->set_trim_end (dcpomatic::ContentTime(3000)); BOOST_CHECK (audio->length_after_trim(film) == DCPTime(957000)); } + + +/** Sync error #1833 */ +BOOST_AUTO_TEST_CASE (content_test6) +{ + shared_ptr film = new_test_film2 ("content_test6"); + film->examine_and_add_content (content_factory(TestPaths::private_data() / "fha.mkv").front()); + BOOST_REQUIRE (!wait_for_jobs()); + film->make_dcp (); + BOOST_REQUIRE (!wait_for_jobs()); + check_dcp (TestPaths::private_data() / "fha", film); +} + + +/** Reel length error when making the test for #1833 */ +BOOST_AUTO_TEST_CASE (content_test7) +{ + shared_ptr film = new_test_film2 ("content_test7"); + shared_ptr content = content_factory(TestPaths::private_data() / "clapperboard.mp4").front(); + film->examine_and_add_content (content); + BOOST_REQUIRE (!wait_for_jobs()); + content->audio->set_delay (-1000); + film->make_dcp (); + BOOST_REQUIRE (!wait_for_jobs()); +} -- 2.30.2