Fix race between the Butler thread starting and audio (perhaps) being disabled.

This could cause Butler::audio to be called with _audio_channels = 0
and _disable_audio = false, causing an exception in AudioBuffers when
remap() tried to make an AudioBuffers object with a channel count of 0.

8 files changed, 60 insertions, 29 deletions

diff --git a/src/lib/butler.cc b/src/lib/butler.cc
index 27b39ba4f..ce35b1f39 100644
--- a/src/lib/butler.cc
+++ b/src/lib/butler.cc
@@ -69,7 +69,8 @@ Butler::Butler (
 	VideoRange video_range,
 	Image::Alignment alignment,
 	bool fast,
-	bool prepare_only_proxy
+	bool prepare_only_proxy,
+	Audio audio
 	)
 	: _film (film)
 	, _player (player)
@@ -81,7 +82,7 @@ Butler::Butler (
 	, _stop_thread (false)
 	, _audio_mapping (audio_mapping)
 	, _audio_channels (audio_channels)
-	, _disable_audio (false)
+	, _disable_audio (audio == Audio::DISABLED)
 	, _pixel_format (pixel_format)
 	, _video_range (video_range)
 	, _alignment (alignment)
@@ -394,14 +395,6 @@ Butler::get_audio (Behaviour behaviour, float* out, Frame frames)
 }
 
 
-void
-Butler::disable_audio ()
-{
-	boost::mutex::scoped_lock lm (_mutex);
-	_disable_audio = true;
-}
-
-
 pair<size_t, string>
 Butler::memory_used () const
 {
diff --git a/src/lib/butler.h b/src/lib/butler.h
index c7e71658d..79701d370 100644
--- a/src/lib/butler.h
+++ b/src/lib/butler.h
@@ -38,6 +38,12 @@ class PlayerVideo;
 class Butler : public ExceptionStore
 {
 public:
+	enum class Audio
+	{
+		ENABLED,
+		DISABLED
+	};
+
 	Butler (
 		std::weak_ptr<const Film> film,
 		std::shared_ptr<Player> player,
@@ -47,7 +53,8 @@ public:
 		VideoRange video_range,
 		Image::Alignment alignment,
 		bool fast,
-		bool prepare_only_proxy
+		bool prepare_only_proxy,
+		Audio audio
 		);
 
 	~Butler ();
@@ -81,8 +88,6 @@ public:
 	boost::optional<dcpomatic::DCPTime> get_audio (Behaviour behaviour, float* out, Frame frames);
 	boost::optional<TextRingBuffers::Data> get_closed_caption ();
 
-	void disable_audio ();
-
 	std::pair<size_t, std::string> memory_used () const;
 
 private:
diff --git a/src/lib/ffmpeg_encoder.cc b/src/lib/ffmpeg_encoder.cc
index d4f0b4b47..bcf2b82de 100644
--- a/src/lib/ffmpeg_encoder.cc
+++ b/src/lib/ffmpeg_encoder.cc
@@ -108,7 +108,16 @@ FFmpegEncoder::FFmpegEncoder (
 	}
 
 	_butler = std::make_shared<Butler>(
-		_film, _player, map, _output_audio_channels, bind(&PlayerVideo::force, FFmpegFileEncoder::pixel_format(format)), VideoRange::VIDEO, Image::Alignment::PADDED, false, false
+		_film,
+		_player,
+		map,
+		_output_audio_channels,
+		bind(&PlayerVideo::force, FFmpegFileEncoder::pixel_format(format)),
+		VideoRange::VIDEO,
+		Image::Alignment::PADDED,
+		false,
+		false,
+		Butler::Audio::ENABLED
 		);
 }
 
diff --git a/src/wx/film_viewer.cc b/src/wx/film_viewer.cc
index 510269793..b5b2ca972 100644
--- a/src/wx/film_viewer.cc
+++ b/src/wx/film_viewer.cc
@@ -228,13 +228,10 @@ FilmViewer::recreate_butler ()
 		VideoRange::FULL,
 		j2k_gl_optimised ? Image::Alignment::COMPACT : Image::Alignment::PADDED,
 		true,
-		j2k_gl_optimised
+		j2k_gl_optimised,
+		(Config::instance()->sound() && _audio.isStreamOpen()) ? Butler::Audio::ENABLED : Butler::Audio::DISABLED
 		);
 
-	if (!Config::instance()->sound() && !_audio.isStreamOpen()) {
-		_butler->disable_audio ();
-	}
-
 	_closed_captions_dialog->set_butler (_butler);
 
 	resume ();
diff --git a/test/butler_test.cc b/test/butler_test.cc
index 0422b89ad..7fd5ea571 100644
--- a/test/butler_test.cc
+++ b/test/butler_test.cc
@@ -60,7 +60,18 @@ BOOST_AUTO_TEST_CASE (butler_test1)
 		map.set (i, i, 1);
 	}
 
-	Butler butler (film, make_shared<Player>(film, Image::Alignment::COMPACT), map, 6, boost::bind(&PlayerVideo::force, AV_PIX_FMT_RGB24), VideoRange::FULL, Image::Alignment::COMPACT, false, false);
+	Butler butler (
+		film,
+		make_shared<Player>(film, Image::Alignment::COMPACT),
+		map,
+		6,
+		boost::bind(&PlayerVideo::force, AV_PIX_FMT_RGB24),
+		VideoRange::FULL,
+		Image::Alignment::COMPACT,
+		false,
+		false,
+		Butler::Audio::ENABLED
+		);
 
 	BOOST_CHECK (butler.get_video(Butler::Behaviour::BLOCKING, 0).second == DCPTime());
 	BOOST_CHECK (butler.get_video(Butler::Behaviour::BLOCKING, 0).second == DCPTime::from_frames(1, 24));
@@ -95,7 +106,16 @@ BOOST_AUTO_TEST_CASE (butler_test2)
 	}
 
 	Butler butler (
-		film, make_shared<Player>(film, Image::Alignment::COMPACT), map, 6, boost::bind(&PlayerVideo::force, AV_PIX_FMT_RGB24), VideoRange::FULL, Image::Alignment::COMPACT, false, false
+		film,
+		make_shared<Player>(film, Image::Alignment::COMPACT),
+		map,
+		6,
+		boost::bind(&PlayerVideo::force, AV_PIX_FMT_RGB24),
+		VideoRange::FULL,
+		Image::Alignment::COMPACT,
+		false,
+		false,
+		Butler::Audio::ENABLED
 		);
 
 	int const audio_frames_per_video_frame = 48000 / 25;
diff --git a/test/dcp_playback_test.cc b/test/dcp_playback_test.cc
index 4cdb9f897..62c72cc84 100644
--- a/test/dcp_playback_test.cc
+++ b/test/dcp_playback_test.cc
@@ -52,7 +52,8 @@ BOOST_AUTO_TEST_CASE (dcp_playback_test)
 		VideoRange::FULL,
 		Image::Alignment::PADDED,
 		true,
-		false
+		false,
+		Butler::Audio::ENABLED
 		);
 
 	std::vector<float> audio_buffer(2000 * 6);
diff --git a/test/player_test.cc b/test/player_test.cc
index 03f2eb6e6..e78e0ee35 100644
--- a/test/player_test.cc
+++ b/test/player_test.cc
@@ -233,8 +233,9 @@ BOOST_AUTO_TEST_CASE (player_seek_test)
 	player->set_always_burn_open_subtitles ();
 	player->set_play_referenced ();
 
-	auto butler = std::make_shared<Butler>(film, player, AudioMapping(), 2, bind(PlayerVideo::force, AV_PIX_FMT_RGB24), VideoRange::FULL, Image::Alignment::PADDED, true, false);
-	butler->disable_audio();
+	auto butler = std::make_shared<Butler>(
+		film, player, AudioMapping(), 2, bind(PlayerVideo::force, AV_PIX_FMT_RGB24), VideoRange::FULL, Image::Alignment::PADDED, true, false, Butler::Audio::DISABLED
+		);
 
 	for (int i = 0; i < 10; ++i) {
 		auto t = DCPTime::from_frames (i, 24);
@@ -265,8 +266,9 @@ BOOST_AUTO_TEST_CASE (player_seek_test2)
 	player->set_always_burn_open_subtitles ();
 	player->set_play_referenced ();
 
-	auto butler = std::make_shared<Butler>(film, player, AudioMapping(), 2, bind(PlayerVideo::force, AV_PIX_FMT_RGB24), VideoRange::FULL, Image::Alignment::PADDED, true, false);
-	butler->disable_audio();
+	auto butler = std::make_shared<Butler>
+		(film, player, AudioMapping(), 2, bind(PlayerVideo::force, AV_PIX_FMT_RGB24), VideoRange::FULL, Image::Alignment::PADDED, true, false, Butler::Audio::DISABLED
+		 );
 
 	butler->seek(DCPTime::from_seconds(5), true);
 
@@ -356,7 +358,9 @@ BOOST_AUTO_TEST_CASE (player_trim_crash)
 
 	auto player = std::make_shared<Player>(film, Image::Alignment::COMPACT);
 	player->set_fast ();
-	auto butler = std::make_shared<Butler>(film, player, AudioMapping(), 6, bind(&PlayerVideo::force, AV_PIX_FMT_RGB24), VideoRange::FULL, Image::Alignment::COMPACT, true, false);
+	auto butler = std::make_shared<Butler>(
+		film, player, AudioMapping(), 6, bind(&PlayerVideo::force, AV_PIX_FMT_RGB24), VideoRange::FULL, Image::Alignment::COMPACT, true, false, Butler::Audio::ENABLED
+		);
 
 	/* Wait for the butler to fill */
 	dcpomatic_sleep_seconds (5);
@@ -486,7 +490,7 @@ BOOST_AUTO_TEST_CASE (encrypted_dcp_with_no_kdm_gives_no_butler_error)
 	auto film2 = new_test_film2 ("encrypted_dcp_with_no_kdm_gives_no_butler_error2", { content2 });
 
 	auto player = std::make_shared<Player>(film2, Image::Alignment::COMPACT);
-	Butler butler(film2, player, AudioMapping(), 2, bind(PlayerVideo::force, AV_PIX_FMT_RGB24), VideoRange::FULL, Image::Alignment::PADDED, true, false);
+	Butler butler(film2, player, AudioMapping(), 2, bind(PlayerVideo::force, AV_PIX_FMT_RGB24), VideoRange::FULL, Image::Alignment::PADDED, true, false, Butler::Audio::ENABLED);
 
 	float buffer[2000 * 6];
 	for (int i = 0; i < length; ++i) {
diff --git a/test/threed_test.cc b/test/threed_test.cc
index b4599cf80..4413ff01b 100644
--- a/test/threed_test.cc
+++ b/test/threed_test.cc
@@ -272,7 +272,9 @@ BOOST_AUTO_TEST_CASE (threed_test_butler_overfill)
 
 	auto player = std::make_shared<Player>(film, Image::Alignment::COMPACT);
 	int const audio_channels = 2;
-	auto butler = std::make_shared<Butler>(film, player, AudioMapping(), audio_channels, boost::bind(PlayerVideo::force, AV_PIX_FMT_RGB24), VideoRange::FULL, Image::Alignment::PADDED, true, false);
+	auto butler = std::make_shared<Butler>(
+		film, player, AudioMapping(), audio_channels, boost::bind(PlayerVideo::force, AV_PIX_FMT_RGB24), VideoRange::FULL, Image::Alignment::PADDED, true, false, Butler::Audio::ENABLED
+		);
 
 	int const audio_frames = 1920;
 	std::vector<float> audio(audio_frames * audio_channels);