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.

diff --git a/src/lib/audio_decoder.cc b/src/lib/audio_decoder.cc
index a5e86f22b8e352d529fa39e69149ad10cca9fec4..051fb4dd8bebe7eff08b69f095edc18a681c8551 100644 (file)
--- a/src/lib/audio_decoder.cc
+++ b/src/lib/audio_decoder.cc
@@ -48,25 +48,51 @@ AudioDecoder::AudioDecoder (Decoder* parent, shared_ptr<const AudioContent> cont
        }
 }
 
+/** @param time_already_delayed true if the delay should not be added to time */
 void
-AudioDecoder::emit (shared_ptr<const Film> film, AudioStreamPtr stream, shared_ptr<const AudioBuffers> data, ContentTime time)
+AudioDecoder::emit (shared_ptr<const Film> film, AudioStreamPtr stream, shared_ptr<const AudioBuffers> 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> resampler;
@@ -74,15 +100,15 @@ AudioDecoder::emit (shared_ptr<const Film> 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 8bd65a67137a7ed4fffa4bbaf098b19975054788..897ddb7e40b8253ecb54c74e4b3fb58a98d3c4ef 100644 (file)
--- a/src/lib/audio_decoder.h
+++ b/src/lib/audio_decoder.h
@@ -48,7 +48,7 @@ public:
        AudioDecoder (Decoder* parent, boost::shared_ptr<const AudioContent> content, bool fast);
 
        boost::optional<dcpomatic::ContentTime> position (boost::shared_ptr<const Film> film) const;
-       void emit (boost::shared_ptr<const Film> film, AudioStreamPtr stream, boost::shared_ptr<const AudioBuffers>, dcpomatic::ContentTime);
+       void emit (boost::shared_ptr<const Film> film, AudioStreamPtr stream, boost::shared_ptr<const AudioBuffers>, 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 41b93dad758b1eb502406bcb1e64dab3aab24401..0d653906116a1ada85573e50e5eac357593d6be8 100644 (file)
--- 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<AudioBuffers> 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 23fb23865bfe71da1f8f0022bc53e1ba348b30ef..de9ca77747a0184ea02d120ca6ea42aea7423b24 100644 (file)
--- 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> 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> film = new_test_film2 ("content_test7");
+       shared_ptr<Content> 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());
+}