From 046d84f45621f7e128cb30160a315f98881c6f4b Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Sun, 17 Nov 2019 18:13:36 +0100 Subject: [PATCH] A little thread safety. --- src/wx/film_viewer.cc | 32 ++++++++++++-------- src/wx/film_viewer.h | 7 +++-- src/wx/gl_video_view.cc | 58 +++++++++++++++++++++++++++++++------ src/wx/gl_video_view.h | 6 ++++ src/wx/simple_video_view.cc | 6 ++-- src/wx/video_view.cc | 23 +++++++++++++++ src/wx/video_view.h | 22 ++++++++++++++ 7 files changed, 128 insertions(+), 26 deletions(-) diff --git a/src/wx/film_viewer.cc b/src/wx/film_viewer.cc index f40ed229f..f3250dbfa 100644 --- a/src/wx/film_viewer.cc +++ b/src/wx/film_viewer.cc @@ -156,9 +156,9 @@ FilmViewer::set_film (shared_ptr film) } _film = film; - _video_view->clear (); - _video_view->set_image (shared_ptr()); + _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 +FilmViewer::audio_time () const +{ + if (!_audio.isStreamRunning()) { + return optional(); + } + + return DCPTime::from_seconds (const_cast(&_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))); } + diff --git a/src/wx/film_viewer.h b/src/wx/film_viewer.h index beb1a5d74..eaf46f1e6 100644 --- a/src/wx/film_viewer.h +++ b/src/wx/film_viewer.h @@ -27,6 +27,7 @@ #include "lib/config.h" #include "lib/player_text.h" #include "lib/timer.h" +#include "lib/signaller.h" #include #include @@ -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 () const { return _butler; } - int time_until_next_frame () const; boost::signals2::signal)> ImageChanged; boost::signals2::signal Started; boost::signals2::signal Stopped; /** While playing back we reached the end of the film (emitted from GUI thread) */ boost::signals2::signal Finished; + void emit_finished (); boost::signals2::signal PlaybackPermitted; @@ -165,6 +167,7 @@ private: void config_changed (Config::Property); dcpomatic::DCPTime time () const; + boost::optional audio_time () const; dcpomatic::DCPTime uncorrected_time () const; Frame average_latency () const; diff --git a/src/wx/gl_video_view.cc b/src/wx/gl_video_view.cc index ebf8b8fe2..c3a611283 100644 --- a/src/wx/gl_video_view.cc +++ b/src/wx/gl_video_view.cc @@ -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()); +} + + diff --git a/src/wx/gl_video_view.h b/src/wx/gl_video_view.h index 4ad4b1283..cf42432a9 100644 --- a/src/wx/gl_video_view.h +++ b/src/wx/gl_video_view.h @@ -19,6 +19,7 @@ */ #include "video_view.h" +#include "lib/signaller.h" #include #include #include @@ -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 _size; bool _vsync_enabled; boost::thread* _thread; + mutable boost::mutex _one_shot_mutex; bool _one_shot; }; diff --git a/src/wx/simple_video_view.cc b/src/wx/simple_video_view.cc index e66ed815e..a00524f7d 100644 --- a/src/wx/simple_video_view.cc +++ b/src/wx/simple_video_view.cc @@ -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 (); diff --git a/src/wx/video_view.cc b/src/wx/video_view.cc index 22cad3979..e1a8b7306 100644 --- a/src/wx/video_view.cc +++ b/src/wx/video_view.cc @@ -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; +} + diff --git a/src/wx/video_view.h b/src/wx/video_view.h index d6e76ada7..d9ef2a65f 100644 --- a/src/wx/video_view.h +++ b/src/wx/video_view.h @@ -24,6 +24,7 @@ #include "lib/dcpomatic_time.h" #include #include +#include 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 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 film () const { + boost::mutex::scoped_lock lm (_mutex); + return _film; + } FilmViewer* _viewer; std::pair, 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 _film; }; #endif -- 2.30.2