From e97d48b043fe39ec22687555225d6b4b526a2172 Mon Sep 17 00:00:00 2001
From: Carl Hetherington <cth@carlh.net>
Date: Tue, 5 Nov 2019 21:09:37 +0100
Subject: [PATCH] Remove dubious _buffers_mutex and maintain a lock on _mutex
 for the whole of ::audio.  Otherwise changes to pending seeks can be mixed up
 with audio being put into the ringbuffer.

---
 src/lib/butler.cc | 22 +++++++---------------
 src/lib/butler.h  |  6 ------
 2 files changed, 7 insertions(+), 21 deletions(-)

diff --git a/src/lib/butler.cc b/src/lib/butler.cc
index 2d6c46c7e..6062b0f21 100644
--- a/src/lib/butler.cc
+++ b/src/lib/butler.cc
@@ -275,12 +275,9 @@ Butler::seek_unlocked (DCPTime position, bool accurate)
 	_pending_seek_position = position;
 	_pending_seek_accurate = accurate;
 
-	{
-		boost::mutex::scoped_lock lm (_buffers_mutex);
-		_video.clear ();
-		_audio.clear ();
-		_closed_caption.clear ();
-	}
+	_video.clear ();
+	_audio.clear ();
+	_closed_caption.clear ();
 
 	_summon.notify_all ();
 }
@@ -316,22 +313,18 @@ Butler::video (shared_ptr<PlayerVideo> video, DCPTime time)
 
 	_prepare_service.post (bind (&Butler::prepare, this, weak_ptr<PlayerVideo>(video)));
 
-	boost::mutex::scoped_lock lm2 (_buffers_mutex);
 	_video.put (video, time);
 }
 
 void
 Butler::audio (shared_ptr<AudioBuffers> audio, DCPTime time, int frame_rate)
 {
-	{
-		boost::mutex::scoped_lock lm (_mutex);
-		if (_pending_seek_position || _disable_audio) {
-			/* Don't store any audio in these cases */
-			return;
-		}
+	boost::mutex::scoped_lock lm (_mutex);
+	if (_pending_seek_position || _disable_audio) {
+		/* Don't store any audio in these cases */
+		return;
 	}
 
-	boost::mutex::scoped_lock lm2 (_buffers_mutex);
 	_audio.put (remap (audio, _audio_channels, _audio_mapping), time, frame_rate);
 }
 
@@ -406,6 +399,5 @@ Butler::text (PlayerText pt, TextType type, optional<DCPTextTrack> track, DCPTim
 
 	DCPOMATIC_ASSERT (track);
 
-	boost::mutex::scoped_lock lm2 (_buffers_mutex);
 	_closed_caption.put (pt, *track, period);
 }
diff --git a/src/lib/butler.h b/src/lib/butler.h
index e5581ccb4..ea4337443 100644
--- a/src/lib/butler.h
+++ b/src/lib/butler.h
@@ -75,12 +75,6 @@ private:
 	boost::shared_ptr<Player> _player;
 	boost::thread* _thread;
 
-	/** mutex to protect _video, _audio and _closed_caption for when we are clearing them and they all need to be
-	    cleared together without any data being inserted in the interim;
-	    XXX: is this necessary now that all butler output data is timestamped? Perhaps the locked clear-out
-	    is only required if we guarantee that get_video() and get_audio() calls are in sync.
-	*/
-	boost::mutex _buffers_mutex;
 	VideoRingBuffers _video;
 	AudioRingBuffers _audio;
 	TextRingBuffers _closed_caption;
-- 
2.30.2