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.

diff --git a/src/lib/butler.cc b/src/lib/butler.cc
index 27b39ba4fb13b3a3bae76963f9f42b56472309fe..ce35b1f39aff61da043aa71e0a3d5fad0e963f11 100644 (file)
--- 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 c7e71658da25889107d033147e62b16897f92f4d..79701d3701e16273e6decb93ad6d268b2b489de5 100644 (file)
--- 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 d4f0b4b472612bb7ee449bd7b51bad00d81fd9f6..bcf2b82de898b5dd754f431b6533c4661ac3af71 100644 (file)
--- 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 5102697935b6565b1d1598079acafab871de05cd..b5b2ca972e57aa345856cea8ce28037db1788283 100644 (file)
--- 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 0422b89ad45af1a834704332497292a23ea1e620..7fd5ea57113bdb43135b281000a86204b3140284 100644 (file)
--- 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 4cdb9f8976148367c8d085954841b736ad690c0e..62c72cc84e85fd25e86ee605643e27fe4811a78c 100644 (file)
--- 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 03f2eb6e655d977f0910c3338e15f94cbd13675c..e78e0ee355329035c5124beec2ff5df3cbb2cbac 100644 (file)
--- 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 b4599cf809fe1914625a0ac3de959920596ed4ba..4413ff01bcc737c7962da57abebb89c79e78dff7 100644 (file)
--- 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);