Timestamp audio emissions from butler and hence discard very late

audio in FilmViewer.  This should help with the case where lots
of video frames are rapidly discarded when they are late but the
corresponding audio is not, hence audio buffers get overfilled.

diff --git a/src/lib/audio_ring_buffers.cc b/src/lib/audio_ring_buffers.cc
index 42115efa3e786835cfb7e62b20e31b80d8095f1f..f51ff8a38860e03b7d3e867b9fea9e861d46ead4 100644 (file)
--- a/src/lib/audio_ring_buffers.cc
+++ b/src/lib/audio_ring_buffers.cc
@@ -1,5 +1,5 @@
 /*
-    Copyright (C) 2016-2017 Carl Hetherington <cth@carlh.net>
+    Copyright (C) 2016-2018 Carl Hetherington <cth@carlh.net>
 
     This file is part of DCP-o-matic.
 
@@ -26,7 +26,11 @@
 
 using std::min;
 using std::cout;
+using std::make_pair;
+using std::pair;
+using std::list;
 using boost::shared_ptr;
+using boost::optional;
 
 AudioRingBuffers::AudioRingBuffers ()
        : _used_in_head (0)
@@ -35,23 +39,26 @@ AudioRingBuffers::AudioRingBuffers ()
 }
 
 void
-AudioRingBuffers::put (shared_ptr<const AudioBuffers> data)
+AudioRingBuffers::put (shared_ptr<const AudioBuffers> data, DCPTime time)
 {
        boost::mutex::scoped_lock lm (_mutex);
 
-       if (!_buffers.empty ()) {
-               DCPOMATIC_ASSERT (_buffers.front()->channels() == data->channels());
+       if (!_buffers.empty()) {
+               DCPOMATIC_ASSERT (_buffers.front().first->channels() == data->channels());
+               DCPOMATIC_ASSERT ((_buffers.back().second + DCPTime::from_frames(_buffers.back().first->frames(), 48000)) == time);
        }
 
-       _buffers.push_back (data);
+       _buffers.push_back(make_pair(data, time));
 }
 
-/** @return true if there was an underrun, otherwise false */
-bool
+/** @return time of the returned data; if it's not set this indicates an underrun */
+optional<DCPTime>
 AudioRingBuffers::get (float* out, int channels, int frames)
 {
        boost::mutex::scoped_lock lm (_mutex);
 
+       optional<DCPTime> time;
+
        while (frames > 0) {
                if (_buffers.empty ()) {
                        for (int i = 0; i < frames; ++i) {
@@ -60,14 +67,17 @@ AudioRingBuffers::get (float* out, int channels, int frames)
                                }
                        }
                        cout << "audio underrun; missing " << frames << "!\n";
-                       return true;
+                       return time;
                }
 
-               shared_ptr<const AudioBuffers> front = _buffers.front ();
+               pair<shared_ptr<const AudioBuffers>, DCPTime> front = _buffers.front ();
+               if (!time) {
+                       time = front.second + DCPTime::from_frames(_used_in_head, 48000);
+               }
 
-               int const to_do = min (frames, front->frames() - _used_in_head);
-               float** p = front->data();
-               int const c = min (front->channels(), channels);
+               int const to_do = min (frames, front.first->frames() - _used_in_head);
+               float** p = front.first->data();
+               int const c = min (front.first->channels(), channels);
                for (int i = 0; i < to_do; ++i) {
                        for (int j = 0; j < c; ++j) {
                                *out++ = p[j][i + _used_in_head];
@@ -79,13 +89,13 @@ AudioRingBuffers::get (float* out, int channels, int frames)
                _used_in_head += to_do;
                frames -= to_do;
 
-               if (_used_in_head == front->frames()) {
+               if (_used_in_head == front.first->frames()) {
                        _buffers.pop_front ();
                        _used_in_head = 0;
                }
        }
 
-       return false;
+       return time;
 }
 
 void
@@ -101,8 +111,8 @@ AudioRingBuffers::size () const
 {
        boost::mutex::scoped_lock lm (_mutex);
        Frame s = 0;
-       BOOST_FOREACH (shared_ptr<const AudioBuffers> i, _buffers) {
-               s += i->frames ();
+       for (list<pair<shared_ptr<const AudioBuffers>, DCPTime> >::const_iterator i = _buffers.begin(); i != _buffers.end(); ++i) {
+               s += i->first->frames();
        }
        return s - _used_in_head;
 }

diff --git a/src/lib/audio_ring_buffers.h b/src/lib/audio_ring_buffers.h
index 3c726425c9b749b6807a19da2eaf38f218d379c1..53236cb3226f508a46d04b21272ff882dcb0c4e7 100644 (file)
--- a/src/lib/audio_ring_buffers.h
+++ b/src/lib/audio_ring_buffers.h
@@ -33,15 +33,15 @@ class AudioRingBuffers : public boost::noncopyable
 public:
        AudioRingBuffers ();
 
-       void put (boost::shared_ptr<const AudioBuffers> data);
-       bool get (float* out, int channels, int frames);
+       void put (boost::shared_ptr<const AudioBuffers> data, DCPTime time);
+       boost::optional<DCPTime> get (float* out, int channels, int frames);
 
        void clear ();
        Frame size () const;
 
 private:
        mutable boost::mutex _mutex;
-       std::list<boost::shared_ptr<const AudioBuffers> > _buffers;
+       std::list<std::pair<boost::shared_ptr<const AudioBuffers>, DCPTime> > _buffers;
        int _used_in_head;
 };
 

diff --git a/src/lib/butler.cc b/src/lib/butler.cc
index 63fe729aecea5f2c43557022e3a90769576e9c11..aee5846542d977586bae2d65f04466deedafc7a8 100644 (file)
--- a/src/lib/butler.cc
+++ b/src/lib/butler.cc
@@ -62,7 +62,7 @@ Butler::Butler (shared_ptr<Player> player, shared_ptr<Log> log, AudioMapping aud
        , _disable_audio (false)
 {
        _player_video_connection = _player->Video.connect (bind (&Butler::video, this, _1, _2));
-       _player_audio_connection = _player->Audio.connect (bind (&Butler::audio, this, _1));
+       _player_audio_connection = _player->Audio.connect (bind (&Butler::audio, this, _1, _2));
        _player_changed_connection = _player->Changed.connect (bind (&Butler::player_changed, this, _1));
        _thread = new boost::thread (bind (&Butler::thread, this));
 #ifdef DCPOMATIC_LINUX
@@ -258,7 +258,7 @@ Butler::video (shared_ptr<PlayerVideo> video, DCPTime time)
 }
 
 void
-Butler::audio (shared_ptr<AudioBuffers> audio)
+Butler::audio (shared_ptr<AudioBuffers> audio, DCPTime time)
 {
        {
                boost::mutex::scoped_lock lm (_mutex);
@@ -269,19 +269,19 @@ Butler::audio (shared_ptr<AudioBuffers> audio)
        }
 
        boost::mutex::scoped_lock lm2 (_video_audio_mutex);
-       _audio.put (remap (audio, _audio_channels, _audio_mapping));
+       _audio.put (remap (audio, _audio_channels, _audio_mapping), time);
 }
 
 /** Try to get `frames' frames of audio and copy it into `out'.  Silence
  *  will be filled if no audio is available.
- *  @return true if there was a buffer underrun, otherwise false.
+ *  @return time of this audio, or unset if there was a buffer underrun.
  */
-bool
+optional<DCPTime>
 Butler::get_audio (float* out, Frame frames)
 {
-       bool const underrun = _audio.get (out, _audio_channels, frames);
+       optional<DCPTime> t = _audio.get (out, _audio_channels, frames);
        _summon.notify_all ();
-       return underrun;
+       return t;
 }
 
 void

diff --git a/src/lib/butler.h b/src/lib/butler.h
index a8b38ef2e83075e71464a426ff4e574b91467357..e6553cf8c65d05958bf53fdec6a316938c9e5cc2 100644 (file)
--- a/src/lib/butler.h
+++ b/src/lib/butler.h
@@ -41,7 +41,7 @@ public:
 
        void seek (DCPTime position, bool accurate);
        std::pair<boost::shared_ptr<PlayerVideo>, DCPTime> get_video ();
-       bool get_audio (float* out, Frame frames);
+       boost::optional<DCPTime> get_audio (float* out, Frame frames);
 
        void disable_audio ();
 
@@ -50,7 +50,7 @@ public:
 private:
        void thread ();
        void video (boost::shared_ptr<PlayerVideo> video, DCPTime time);
-       void audio (boost::shared_ptr<AudioBuffers> audio);
+       void audio (boost::shared_ptr<AudioBuffers> audio, DCPTime time);
        bool should_run () const;
        void prepare (boost::weak_ptr<PlayerVideo> video) const;
        void player_changed (int);

diff --git a/src/wx/film_viewer.cc b/src/wx/film_viewer.cc
index ef28018e96b0a960623ba665d4cc6344851f07b0..ce34b06b7122f56f2a12243c7cb448a3f333a6b9 100644 (file)
--- a/src/wx/film_viewer.cc
+++ b/src/wx/film_viewer.cc
@@ -881,6 +881,16 @@ FilmViewer::config_changed (Config::Property p)
        }
 }
 
+DCPTime
+FilmViewer::uncorrected_time () const
+{
+       if (_audio.isStreamRunning ()) {
+               return DCPTime::from_seconds (const_cast<RtAudio*>(&_audio)->getStreamTime());
+       }
+
+       return _video_position;
+}
+
 DCPTime
 FilmViewer::time () const
 {
@@ -895,7 +905,14 @@ FilmViewer::time () const
 int
 FilmViewer::audio_callback (void* out_p, unsigned int frames)
 {
-       _butler->get_audio (reinterpret_cast<float*> (out_p), frames);
+       while (true) {
+               optional<DCPTime> t = _butler->get_audio (reinterpret_cast<float*> (out_p), frames);
+               if (!t || DCPTime(uncorrected_time() - *t) < one_video_frame()) {
+                       /* There was an underrun or this audio is on time; carry on */
+                       break;
+               }
+               /* The audio we just got was (very) late; drop it and get some more. */
+       }
 
         boost::mutex::scoped_lock lm (_latency_history_mutex, boost::try_to_lock);
         if (lm) {

diff --git a/src/wx/film_viewer.h b/src/wx/film_viewer.h
index e4449fa2f1f35d755edcf10b9378fec8792f091f..a41500acee992ba0020608ca6f8a41b1e1ce47e1 100644 (file)
--- a/src/wx/film_viewer.h
+++ b/src/wx/film_viewer.h
@@ -114,6 +114,7 @@ private:
        void recreate_butler ();
        void config_changed (Config::Property);
        DCPTime time () const;
+       DCPTime uncorrected_time () const;
        Frame average_latency () const;
        DCPTime one_video_frame () const;
 

diff --git a/test/audio_ring_buffers_test.cc b/test/audio_ring_buffers_test.cc
index 23491f6b380b694a520cd8b073ddf54e43f3d4e2..506a6350ecf5a1c2d456766d4155d7fab7551e71 100644 (file)
--- a/test/audio_ring_buffers_test.cc
+++ b/test/audio_ring_buffers_test.cc
@@ -1,5 +1,5 @@
 /*
-    Copyright (C) 2016-2017 Carl Hetherington <cth@carlh.net>
+    Copyright (C) 2016-2018 Carl Hetherington <cth@carlh.net>
 
     This file is part of DCP-o-matic.
 
@@ -27,6 +27,8 @@ using boost::shared_ptr;
 
 #define CANARY 9999
 
+/* XXX: these tests don't check the timestamping in AudioRingBuffers */
+
 /** Basic tests fetching the same number of channels as went in */
 BOOST_AUTO_TEST_CASE (audio_ring_buffers_test1)
 {
@@ -38,7 +40,7 @@ BOOST_AUTO_TEST_CASE (audio_ring_buffers_test1)
        /* Getting some data should give an underrun and write zeros */
        float buffer[256 * 6];
        buffer[240 * 6] = CANARY;
-       BOOST_CHECK_EQUAL (rb.get (buffer, 6, 240), true);
+       BOOST_CHECK (!rb.get(buffer, 6, 240));
        for (int i = 0; i < 240 * 6; ++i) {
                BOOST_REQUIRE_EQUAL (buffer[i], 0);
        }
@@ -48,7 +50,7 @@ BOOST_AUTO_TEST_CASE (audio_ring_buffers_test1)
        rb.clear ();
        BOOST_CHECK_EQUAL (rb.size(), 0);
        buffer[240 * 6] = CANARY;
-       BOOST_CHECK_EQUAL (rb.get (buffer, 6, 240), true);
+       BOOST_CHECK (rb.get(buffer, 6, 240) == boost::optional<DCPTime>());
        for (int i = 0; i < 240 * 6; ++i) {
                BOOST_REQUIRE_EQUAL (buffer[i], 0);
        }
@@ -62,12 +64,12 @@ BOOST_AUTO_TEST_CASE (audio_ring_buffers_test1)
                        data->data(j)[i] = value++;
                }
        }
-       rb.put (data);
+       rb.put (data, DCPTime());
        BOOST_CHECK_EQUAL (rb.size(), 91);
 
        /* Get part of it out */
        buffer[40 * 6] = CANARY;
-       BOOST_CHECK_EQUAL (rb.get (buffer, 6, 40), false);
+       BOOST_CHECK (*rb.get(buffer, 6, 40) == DCPTime());
        int check = 0;
        for (int i = 0; i < 40 * 6; ++i) {
                BOOST_REQUIRE_EQUAL (buffer[i], check++);
@@ -77,7 +79,7 @@ BOOST_AUTO_TEST_CASE (audio_ring_buffers_test1)
 
        /* Get the rest */
        buffer[51 * 6] = CANARY;
-       BOOST_CHECK_EQUAL (rb.get (buffer, 6, 51), false);
+       BOOST_CHECK (*rb.get(buffer, 6, 51) == DCPTime());
        for (int i = 0; i < 51 * 6; ++i) {
                BOOST_REQUIRE_EQUAL (buffer[i], check++);
        }
@@ -86,7 +88,7 @@ BOOST_AUTO_TEST_CASE (audio_ring_buffers_test1)
 
        /* Now there should be an underrun */
        buffer[240 * 6] = CANARY;
-       BOOST_CHECK_EQUAL (rb.get (buffer, 6, 240), true);
+       BOOST_CHECK (!rb.get(buffer, 6, 240));
        BOOST_CHECK_EQUAL (buffer[240 * 6], CANARY);
 }
 
@@ -103,13 +105,13 @@ BOOST_AUTO_TEST_CASE (audio_ring_buffers_test2)
                        data->data(j)[i] = value++;
                }
        }
-       rb.put (data);
+       rb.put (data, DCPTime());
        BOOST_CHECK_EQUAL (rb.size(), 91);
 
        /* Get part of it out */
        float buffer[256 * 6];
        buffer[40 * 6] = CANARY;
-       BOOST_CHECK_EQUAL (rb.get (buffer, 6, 40), false);
+       BOOST_CHECK (*rb.get(buffer, 6, 40) == DCPTime());
        int check = 0;
        for (int i = 0; i < 40; ++i) {
                for (int j = 0; j < 2; ++j) {
@@ -124,7 +126,7 @@ BOOST_AUTO_TEST_CASE (audio_ring_buffers_test2)
 
        /* Get the rest */
        buffer[51 * 6] = CANARY;
-       BOOST_CHECK_EQUAL (rb.get (buffer, 6, 51), false);
+       BOOST_CHECK (*rb.get(buffer, 6, 51) == DCPTime());
        for (int i = 0; i < 51; ++i) {
                for (int j = 0; j < 2; ++j) {
                        BOOST_REQUIRE_EQUAL (buffer[i * 6 + j], check++);
@@ -138,7 +140,7 @@ BOOST_AUTO_TEST_CASE (audio_ring_buffers_test2)
 
        /* Now there should be an underrun */
        buffer[240 * 6] = CANARY;
-       BOOST_CHECK_EQUAL (rb.get (buffer, 6, 240), true);
+       BOOST_CHECK (!rb.get(buffer, 6, 240));
        BOOST_CHECK_EQUAL (buffer[240 * 6], CANARY);
 }
 
@@ -155,13 +157,13 @@ BOOST_AUTO_TEST_CASE (audio_ring_buffers_test3)
                        data->data(j)[i] = value++;
                }
        }
-       rb.put (data);
+       rb.put (data, DCPTime ());
        BOOST_CHECK_EQUAL (rb.size(), 91);
 
        /* Get part of it out */
        float buffer[256 * 6];
        buffer[40 * 2] = CANARY;
-       BOOST_CHECK_EQUAL (rb.get (buffer, 2, 40), false);
+       BOOST_CHECK (*rb.get(buffer, 2, 40) == DCPTime());
        int check = 0;
        for (int i = 0; i < 40; ++i) {
                for (int j = 0; j < 2; ++j) {
@@ -174,7 +176,7 @@ BOOST_AUTO_TEST_CASE (audio_ring_buffers_test3)
 
        /* Get the rest */
        buffer[51 * 2] = CANARY;
-       BOOST_CHECK_EQUAL (rb.get (buffer, 2, 51), false);
+       BOOST_CHECK (*rb.get(buffer, 2, 51) == DCPTime());
        for (int i = 0; i < 51; ++i) {
                for (int j = 0; j < 2; ++j)  {
                        BOOST_REQUIRE_EQUAL (buffer[i * 2 + j], check++);
@@ -186,6 +188,6 @@ BOOST_AUTO_TEST_CASE (audio_ring_buffers_test3)
 
        /* Now there should be an underrun */
        buffer[240 * 2] = CANARY;
-       BOOST_CHECK_EQUAL (rb.get (buffer, 2, 240), true);
+       BOOST_CHECK (!rb.get(buffer, 2, 240));
        BOOST_CHECK_EQUAL (buffer[240 * 2], CANARY);
 }