A little thread safety.
authorCarl Hetherington <cth@carlh.net>
Sun, 17 Nov 2019 17:13:36 +0000 (18:13 +0100)
committerCarl Hetherington <cth@carlh.net>
Wed, 8 Jan 2020 20:56:47 +0000 (21:56 +0100)
src/wx/film_viewer.cc
src/wx/film_viewer.h
src/wx/gl_video_view.cc
src/wx/gl_video_view.h
src/wx/simple_video_view.cc
src/wx/video_view.cc
src/wx/video_view.h

index f40ed22..f3250db 100644 (file)
@@ -156,9 +156,9 @@ FilmViewer::set_film (shared_ptr<Film> film)
        }
 
        _film = film;
-       _video_view->clear ();
 
-       _video_view->set_image (shared_ptr<Image>());
+       _video_view->set_film (_film);
+       _video_view->clear ();
        _closed_captions_dialog->clear ();
 
        if (!_film) {
@@ -326,8 +326,8 @@ FilmViewer::start ()
                _audio.startStream ();
        }
 
-       _playing = true;
        _dropped = 0;
+       _playing = true;
        _video_view->start ();
        Started (position());
 }
@@ -345,6 +345,7 @@ FilmViewer::stop ()
        }
 
        _playing = false;
+       _video_view->stop ();
        Stopped (position());
        return true;
 }
@@ -540,6 +541,17 @@ FilmViewer::uncorrected_time () const
        return _video_view->position();
 }
 
+optional<DCPTime>
+FilmViewer::audio_time () const
+{
+       if (!_audio.isStreamRunning()) {
+               return optional<DCPTime>();
+       }
+
+       return DCPTime::from_seconds (const_cast<RtAudio*>(&_audio)->getStreamTime ()) -
+               DCPTime::from_frames (average_latency(), _film->audio_frame_rate());
+}
+
 DCPTime
 FilmViewer::time () const
 {
@@ -630,14 +642,10 @@ FilmViewer::set_pad_black (bool p)
        _pad_black = p;
 }
 
-/* XXX_b: comment */
-int
-FilmViewer::time_until_next_frame () const
+/* May be called from a non-UI thread */
+void
+FilmViewer::emit_finished ()
 {
-       DCPTime const next = position() + one_video_frame();
-       std::cout << to_string(next) << " " << to_string(time()) << " " << ((next.seconds() - time().seconds()) * 1000) << "\n";
-       if (next < time()) {
-               return 0;
-       }
-       return (next.seconds() - time().seconds()) * 1000;
+       emit (boost::bind(boost::ref(Finished)));
 }
+
index beb1a5d..eaf46f1 100644 (file)
@@ -27,6 +27,7 @@
 #include "lib/config.h"
 #include "lib/player_text.h"
 #include "lib/timer.h"
+#include "lib/signaller.h"
 #include <RtAudio.h>
 #include <wx/wx.h>
 
@@ -42,7 +43,7 @@ class ClosedCaptionsDialog;
 /** @class FilmViewer
  *  @brief A wx widget to view a Film.
  */
-class FilmViewer
+class FilmViewer : public Signaller
 {
 public:
        FilmViewer (wxWindow *);
@@ -77,6 +78,7 @@ public:
        bool stop ();
        void suspend ();
        void resume ();
+
        bool playing () const {
                return _playing;
        }
@@ -138,13 +140,13 @@ public:
        boost::shared_ptr<Butler> butler () const {
                return _butler;
        }
-       int time_until_next_frame () const;
 
        boost::signals2::signal<void (boost::weak_ptr<PlayerVideo>)> ImageChanged;
        boost::signals2::signal<void (dcpomatic::DCPTime)> Started;
        boost::signals2::signal<void (dcpomatic::DCPTime)> Stopped;
        /** While playing back we reached the end of the film (emitted from GUI thread) */
        boost::signals2::signal<void ()> Finished;
+       void emit_finished ();
 
        boost::signals2::signal<bool ()> PlaybackPermitted;
 
@@ -165,6 +167,7 @@ private:
        void config_changed (Config::Property);
 
        dcpomatic::DCPTime time () const;
+       boost::optional<dcpomatic::DCPTime> audio_time () const;
        dcpomatic::DCPTime uncorrected_time () const;
        Frame average_latency () const;
 
index ebf8b8f..c3a6112 100644 (file)
@@ -267,6 +267,31 @@ GLVideoView::start ()
        _thread = new boost::thread (boost::bind(&GLVideoView::thread, this));
 }
 
+void
+GLVideoView::stop ()
+{
+       if (_thread) {
+               _thread->interrupt ();
+               _thread->join ();
+       }
+       delete _thread;
+       _thread = 0;
+}
+
+bool
+GLVideoView::one_shot () const
+{
+       boost::mutex::scoped_lock lm (_one_shot_mutex);
+       return _one_shot;
+}
+
+void
+GLVideoView::set_one_shot (bool s)
+{
+       boost::mutex::scoped_lock lm (_one_shot_mutex);
+       _one_shot = s;
+}
+
 void
 GLVideoView::thread ()
 try
@@ -279,30 +304,37 @@ try
        }
 
        while (true) {
-               if ((!_viewer->film() || !_viewer->playing()) && !_one_shot) {
+               if (!film() && !one_shot()) {
+                       /* XXX: this should be an indefinite wait until
+                          one of our conditions becomes true.
+                        */
                        dcpomatic_sleep_milliseconds (40);
                        continue;
                }
 
-               _one_shot = false;
+               set_one_shot (false);
 
-               dcpomatic::DCPTime const next = _viewer->position() + _viewer->one_video_frame();
+               dcpomatic::DCPTime const next = position() + one_video_frame();
 
-               if (next >= _viewer->film()->length()) {
+               if (next >= film()->length()) {
                        _viewer->stop ();
-                       _viewer->Finished ();
+                       _viewer->emit_finished ();
                        continue;
                }
 
                get_next_frame (false);
-               set_image (_player_video.first->image(bind(&PlayerVideo::force, _1, AV_PIX_FMT_RGB24), false, true));
+               {
+                       boost::mutex::scoped_lock lm (_mutex);
+                       set_image (_player_video.first->image(bind(&PlayerVideo::force, _1, AV_PIX_FMT_RGB24), false, true));
+               }
                draw ();
 
-               while (_viewer->time_until_next_frame() < 5) {
+               while (time_until_next_frame() < 5) {
                        get_next_frame (true);
                }
 
-               dcpomatic_sleep_milliseconds (_viewer->time_until_next_frame());
+               boost::this_thread::interruption_point ();
+               dcpomatic_sleep_milliseconds (time_until_next_frame());
        }
 
        {
@@ -328,6 +360,14 @@ bool
 GLVideoView::display_next_frame (bool non_blocking)
 {
        bool const g = get_next_frame (non_blocking);
-       _one_shot = true;
+       set_one_shot (true);
        return g;
 }
+
+dcpomatic::DCPTime
+GLVideoView::one_video_frame () const
+{
+       return dcpomatic::DCPTime::from_frames (1, film()->video_frame_rate());
+}
+
+
index 4ad4b12..cf42432 100644 (file)
@@ -19,6 +19,7 @@
 */
 
 #include "video_view.h"
+#include "lib/signaller.h"
 #include <wx/wx.h>
 #include <wx/glcanvas.h>
 #include <dcp/util.h>
@@ -39,6 +40,7 @@ public:
        }
        void update ();
        void start ();
+       void stop ();
 
        bool display_next_frame (bool);
 
@@ -51,6 +53,9 @@ private:
        void draw ();
        void thread ();
        wxGLContext* context () const;
+       bool one_shot () const;
+       void set_one_shot (bool s);
+       dcpomatic::DCPTime one_video_frame () const;
 
        wxGLCanvas* _canvas;
 
@@ -61,5 +66,6 @@ private:
        boost::optional<dcp::Size> _size;
        bool _vsync_enabled;
        boost::thread* _thread;
+       mutable boost::mutex _one_shot_mutex;
        bool _one_shot;
 };
index e66ed81..a00524f 100644 (file)
@@ -145,21 +145,21 @@ SimpleVideoView::update ()
 void
 SimpleVideoView::timer ()
 {
-       if (!_viewer->film() || !_viewer->playing()) {
+       if (!film() || !_viewer->playing()) {
                return;
        }
 
        display_next_frame (false);
        DCPTime const next = _viewer->position() + _viewer->one_video_frame();
 
-       if (next >= _viewer->film()->length()) {
+       if (next >= film()->length()) {
                _viewer->stop ();
                _viewer->Finished ();
                return;
        }
 
        LOG_DEBUG_PLAYER("%1 -> %2; delay %3", next.seconds(), _viewer->time().seconds(), max((next.seconds() - _viewer->time().seconds()) * 1000, 1.0));
-       _timer.Start (_viewer->time_until_next_frame(), wxTIMER_ONE_SHOT);
+       _timer.Start (time_until_next_frame(), wxTIMER_ONE_SHOT);
 
        if (_viewer->butler()) {
                _viewer->butler()->rethrow ();
index 22cad39..e1a8b73 100644 (file)
@@ -26,6 +26,7 @@
 void
 VideoView::clear ()
 {
+       boost::mutex::scoped_lock lm (_mutex);
        _player_video.first.reset ();
        _player_video.second = dcpomatic::DCPTime ();
 }
@@ -39,6 +40,8 @@ VideoView::get_next_frame (bool non_blocking)
        DCPOMATIC_ASSERT (_viewer->butler());
        _viewer->_gets++;
 
+       boost::mutex::scoped_lock lm (_mutex);
+
        do {
                Butler::Error e;
                _player_video = _viewer->butler()->get_video (!non_blocking, &e);
@@ -60,3 +63,23 @@ VideoView::get_next_frame (bool non_blocking)
 
        return true;
 }
+
+dcpomatic::DCPTime
+VideoView::one_video_frame () const
+{
+       return dcpomatic::DCPTime::from_frames (1, film()->video_frame_rate());
+}
+
+/* XXX_b: comment */
+int
+VideoView::time_until_next_frame () const
+{
+       dcpomatic::DCPTime const next = position() + one_video_frame();
+       dcpomatic::DCPTime const time = _viewer->audio_time().get_value_or(position());
+       std::cout << to_string(next) << " " << to_string(time) << " " << ((next.seconds() - time.seconds()) * 1000) << "\n";
+       if (next < time) {
+               return 0;
+       }
+       return (next.seconds() - time.seconds()) * 1000;
+}
+
index d6e76ad..d9ef2a6 100644 (file)
@@ -24,6 +24,7 @@
 #include "lib/dcpomatic_time.h"
 #include <boost/shared_ptr.hpp>
 #include <boost/signals2.hpp>
+#include <boost/thread.hpp>
 
 class Image;
 class wxWindow;
@@ -48,6 +49,8 @@ public:
 
        /* XXX_b: make pure */
        virtual void start () {}
+       /* XXX_b: make pure */
+       virtual void stop () {}
 
        void clear ();
 
@@ -59,23 +62,42 @@ public:
        virtual void display_player_video () {}
 
        dcpomatic::DCPTime position () const {
+               boost::mutex::scoped_lock lm (_mutex);
                return _player_video.second;
        }
 
+       void set_film (boost::shared_ptr<const Film> film) {
+               boost::mutex::scoped_lock lm (_mutex);
+               _film = film;
+       }
+
 protected:
        /* XXX_b: to remove */
        friend class FilmViewer;
 
        bool get_next_frame (bool non_blocking);
+       int time_until_next_frame () const;
+       dcpomatic::DCPTime one_video_frame () const;
+
+       boost::shared_ptr<const Film> film () const {
+               boost::mutex::scoped_lock lm (_mutex);
+               return _film;
+       }
 
        FilmViewer* _viewer;
        std::pair<boost::shared_ptr<PlayerVideo>, dcpomatic::DCPTime> _player_video;
 
+       /** Mutex protecting all the state in VideoView */
+       mutable boost::mutex _mutex;
+
 #ifdef DCPOMATIC_VARIANT_SWAROOP
        bool _in_watermark;
        int _watermark_x;
        int _watermark_y;
 #endif
+
+private:
+       boost::shared_ptr<const Film> _film;
 };
 
 #endif