From ac25cd82d5d29c79b46033a742aaea33c700a524 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Sun, 20 Oct 2019 23:03:27 +0200 Subject: Move _player_video into VideoView. --- src/wx/video_view.cc | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 src/wx/video_view.cc (limited to 'src/wx/video_view.cc') diff --git a/src/wx/video_view.cc b/src/wx/video_view.cc new file mode 100644 index 000000000..eb85079c3 --- /dev/null +++ b/src/wx/video_view.cc @@ -0,0 +1,28 @@ +/* + Copyright (C) 2019 Carl Hetherington + + This file is part of DCP-o-matic. + + DCP-o-matic is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. + + DCP-o-matic is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with DCP-o-matic. If not, see . + +*/ + +#include "video_view.h" + +void +VideoView::clear () +{ + _player_video.first.reset (); + _player_video.second = dcpomatic::DCPTime (); +} -- cgit v1.2.3 From 5eb8b5c3a1566aef638e9d9df03b88d320735092 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Mon, 21 Oct 2019 00:55:52 +0200 Subject: Barely-functioning GL playback with new arrangement. --- src/lib/cross.cc | 11 ++++++ src/lib/cross.h | 1 + src/tools/dcpomatic.cc | 9 ++++- src/tools/dcpomatic_player.cc | 9 ++++- src/tools/wscript | 2 ++ src/wx/film_viewer.h | 2 ++ src/wx/gl_video_view.cc | 78 ++++++++++++++++++++++++++++++++++++++++--- src/wx/gl_video_view.h | 18 +++++++--- src/wx/simple_video_view.cc | 37 ++++++-------------- src/wx/video_view.cc | 34 +++++++++++++++++++ src/wx/video_view.h | 2 ++ wscript | 3 ++ 12 files changed, 168 insertions(+), 38 deletions(-) (limited to 'src/wx/video_view.cc') diff --git a/src/lib/cross.cc b/src/lib/cross.cc index 8d82f7a51..5d35d5a4b 100644 --- a/src/lib/cross.cc +++ b/src/lib/cross.cc @@ -75,6 +75,17 @@ dcpomatic_sleep_seconds (int s) #endif } +void +dcpomatic_sleep_milliseconds (int ms) +{ +#ifdef DCPOMATIC_POSIX + usleep (ms * 1000); +#endif +#ifdef DCPOMATIC_WINDOWS + Sleep (ms); +#endif +} + /** @return A string of CPU information (model name etc.) */ string cpu_info () diff --git a/src/lib/cross.h b/src/lib/cross.h index 67709463a..584fa2b42 100644 --- a/src/lib/cross.h +++ b/src/lib/cross.h @@ -39,6 +39,7 @@ class Log; struct AVIOContext; void dcpomatic_sleep_seconds (int); +void dcpomatic_sleep_milliseconds (int); extern std::string cpu_info (); extern void run_ffprobe (boost::filesystem::path, boost::filesystem::path); extern std::list > mount_info (); diff --git a/src/tools/dcpomatic.cc b/src/tools/dcpomatic.cc index 68bf20732..47851a218 100644 --- a/src/tools/dcpomatic.cc +++ b/src/tools/dcpomatic.cc @@ -92,6 +92,9 @@ #include #include #include +#ifdef __WXGTK__ +#include +#endif #ifdef __WXMSW__ #include #endif @@ -1503,7 +1506,11 @@ public: : wxApp () , _frame (0) , _splash (0) - {} + { +#ifdef DCPOMATIC_LINUX + XInitThreads (); +#endif + } private: diff --git a/src/tools/dcpomatic_player.cc b/src/tools/dcpomatic_player.cc index f68d0ead2..6bd5b36ff 100644 --- a/src/tools/dcpomatic_player.cc +++ b/src/tools/dcpomatic_player.cc @@ -66,6 +66,9 @@ #include #include #include +#ifdef __WXGTK__ +#include +#endif #ifdef __WXOSX__ #include #endif @@ -1044,7 +1047,11 @@ public: App () : wxApp () , _frame (0) - {} + { +#ifdef DCPOMATIC_LINUX + XInitThreads (); +#endif + } private: diff --git a/src/tools/wscript b/src/tools/wscript index 8fd23cfb3..3b2c0a04c 100644 --- a/src/tools/wscript +++ b/src/tools/wscript @@ -68,6 +68,8 @@ def build(bld): obj.uselib += ' WXWIDGETS' if not bld.env.TARGET_OSX: obj.uselib += ' GL GLU' + if bld.env.TARGET_LINUX: + obj.uselib += ' X11' obj.includes = ['..'] obj.use = ['libdcpomatic2', 'libdcpomatic2-wx'] obj.source = '%s.cc' % t diff --git a/src/wx/film_viewer.h b/src/wx/film_viewer.h index 55356f188..a59468c36 100644 --- a/src/wx/film_viewer.h +++ b/src/wx/film_viewer.h @@ -153,6 +153,8 @@ private: /* XXX_b: to remove */ friend class SimpleVideoView; + friend class GLVideoView; + friend class VideoView; void video_view_sized (); void calculate_sizes (); diff --git a/src/wx/gl_video_view.cc b/src/wx/gl_video_view.cc index a0f83db6d..c69ab210a 100644 --- a/src/wx/gl_video_view.cc +++ b/src/wx/gl_video_view.cc @@ -23,6 +23,8 @@ #include "lib/image.h" #include "lib/dcpomatic_assert.h" #include "lib/exceptions.h" +#include "lib/cross.h" +#include "lib/player_video.h" #include #include @@ -52,9 +54,9 @@ using boost::optional; GLVideoView::GLVideoView (FilmViewer* viewer, wxWindow *parent) : VideoView (viewer) , _vsync_enabled (false) + , _thread (0) { _canvas = new wxGLCanvas (parent, wxID_ANY, 0, wxDefaultPosition, wxDefaultSize, wxFULL_REPAINT_ON_RESIZE); - _context = new wxGLContext (_canvas); _canvas->Bind (wxEVT_PAINT, boost::bind(&GLVideoView::paint, this)); _canvas->Bind (wxEVT_SIZE, boost::bind(boost::ref(Sized))); @@ -93,8 +95,13 @@ GLVideoView::GLVideoView (FilmViewer* viewer, wxWindow *parent) GLVideoView::~GLVideoView () { + if (_thread) { + _thread->interrupt (); + _thread->join (); + } + delete _thread; + glDeleteTextures (1, &_id); - delete _context; } static void @@ -109,11 +116,14 @@ static void void GLVideoView::paint () { + /* XXX_b: can't do this yet */ +#if 0 _viewer->state_timer().set("paint-panel"); _canvas->SetCurrent (*_context); wxPaintDC dc (_canvas); draw (); _viewer->state_timer().unset(); +#endif } void @@ -122,8 +132,9 @@ GLVideoView::update () if (!_canvas->IsShownOnScreen()) { return; } - wxClientDC dc (_canvas); - draw (); + /* XXX_b */ +// wxClientDC dc (_canvas); +// draw (); } void @@ -248,3 +259,62 @@ GLVideoView::set_image (shared_ptr image) glTexParameterf (GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE); check_gl_error ("glTexParameterf"); } + +void +GLVideoView::start () +{ + _thread = new boost::thread (boost::bind(&GLVideoView::thread, this)); +} + +void +GLVideoView::thread () +try +{ + /* XXX_b: check all calls and signal emissions in this method & protect them if necessary */ + { + boost::mutex::scoped_lock lm (_context_mutex); + _context = new wxGLContext (_canvas); + _canvas->SetCurrent (*_context); + } + + while (true) { + if (!_viewer->film() || !_viewer->playing()) { + dcpomatic_sleep_milliseconds (40); + continue; + } + + dcpomatic::DCPTime const next = _viewer->position() + _viewer->one_video_frame(); + + if (next >= _viewer->film()->length()) { + _viewer->stop (); + _viewer->Finished (); + return; + } + + get_next_frame (false); + set_image (_player_video.first->image(bind(&PlayerVideo::force, _1, AV_PIX_FMT_RGB24), false, true)); + draw (); + _viewer->_video_position = _player_video.second; + + std::cout << "sleep " << _viewer->time_until_next_frame() << "\n"; + dcpomatic_sleep_milliseconds (_viewer->time_until_next_frame()); + } + + { + boost::mutex::scoped_lock lm (_context_mutex); + delete _context; + } +} +catch (boost::thread_interrupted& e) +{ + /* XXX_b: store exceptions here */ + delete _context; + return; +} + +wxGLContext * +GLVideoView::context () const +{ + boost::mutex::scoped_lock lm (_context_mutex); + return _context; +} diff --git a/src/wx/gl_video_view.h b/src/wx/gl_video_view.h index ba4c7cfdc..e32a1ede9 100644 --- a/src/wx/gl_video_view.h +++ b/src/wx/gl_video_view.h @@ -23,6 +23,7 @@ #include #include #include +#include #undef None #undef Success @@ -37,18 +38,25 @@ public: return _canvas; } void update (); + void start (); bool vsync_enabled () const { return _vsync_enabled; } private: - void paint (); - void draw (); + void paint (); + void draw (); + void thread (); + wxGLContext* context () const; wxGLCanvas* _canvas; - wxGLContext* _context; - GLuint _id; - boost::optional _size; + + wxGLContext* _context; + mutable boost::mutex _context_mutex; + + GLuint _id; + boost::optional _size; bool _vsync_enabled; + boost::thread* _thread; }; diff --git a/src/wx/simple_video_view.cc b/src/wx/simple_video_view.cc index c195bbc35..1e97adb30 100644 --- a/src/wx/simple_video_view.cc +++ b/src/wx/simple_video_view.cc @@ -159,7 +159,7 @@ SimpleVideoView::timer () } 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 (_viewer->time_until_next_frame(), wxTIMER_ONE_SHOT); if (_viewer->butler()) { _viewer->butler()->rethrow (); @@ -180,33 +180,16 @@ SimpleVideoView::start () bool SimpleVideoView::get (bool lazy) { - DCPOMATIC_ASSERT (_viewer->butler()); - _viewer->_gets++; - - do { - Butler::Error e; - _player_video = _viewer->butler()->get_video (!lazy, &e); - if (!_player_video.first && e == Butler::AGAIN) { - if (lazy) { - /* No video available; return saying we failed */ - return false; - } else { - /* Player was suspended; come back later */ - signal_manager->when_idle (boost::bind(&SimpleVideoView::get, this, false)); - return false; - } + bool r = get_next_frame (lazy); + if (!r) { + if (lazy) { + /* No video available; return saying we failed */ + return false; + } else { + /* Player was suspended; come back later */ + signal_manager->when_idle (boost::bind(&SimpleVideoView::get, this, false)); + return false; } - } while ( - _player_video.first && - _viewer->film()->three_d() && - _viewer->_eyes != _player_video.first->eyes() && - _player_video.first->eyes() != EYES_BOTH - ); - - try { - _viewer->butler()->rethrow (); - } catch (DecodeError& e) { - error_dialog (get(), e.what()); } display_player_video (); diff --git a/src/wx/video_view.cc b/src/wx/video_view.cc index eb85079c3..22cad3979 100644 --- a/src/wx/video_view.cc +++ b/src/wx/video_view.cc @@ -19,6 +19,9 @@ */ #include "video_view.h" +#include "wx_util.h" +#include "film_viewer.h" +#include "lib/butler.h" void VideoView::clear () @@ -26,3 +29,34 @@ VideoView::clear () _player_video.first.reset (); _player_video.second = dcpomatic::DCPTime (); } + +/** @param non_blocking true to return false quickly if no video is available quickly. + * @return false if we gave up because it would take too long, otherwise true. + */ +bool +VideoView::get_next_frame (bool non_blocking) +{ + DCPOMATIC_ASSERT (_viewer->butler()); + _viewer->_gets++; + + do { + Butler::Error e; + _player_video = _viewer->butler()->get_video (!non_blocking, &e); + if (!_player_video.first && e == Butler::AGAIN) { + return false; + } + } while ( + _player_video.first && + _viewer->film()->three_d() && + _viewer->_eyes != _player_video.first->eyes() && + _player_video.first->eyes() != EYES_BOTH + ); + + try { + _viewer->butler()->rethrow (); + } catch (DecodeError& e) { + error_dialog (get(), e.what()); + } + + return true; +} diff --git a/src/wx/video_view.h b/src/wx/video_view.h index f4a8ea22b..827d1bf73 100644 --- a/src/wx/video_view.h +++ b/src/wx/video_view.h @@ -64,6 +64,8 @@ protected: /* XXX_b: to remove */ friend class FilmViewer; + bool get_next_frame (bool non_blocking); + FilmViewer* _viewer; std::pair, dcpomatic::DCPTime> _player_video; diff --git a/wscript b/wscript index 9f45778e9..fe1431316 100644 --- a/wscript +++ b/wscript @@ -444,6 +444,9 @@ def configure(conf): conf.env['CXXFLAGS_AVCODEC'] = [] conf.env['CXXFLAGS_AVUTIL'] = [] + if conf.env.TARGET_LINUX: + conf.env.LIB_X11 = ['X11'] + # Boost if conf.options.static_boost: conf.env.STLIB_BOOST_THREAD = ['boost_thread'] -- cgit v1.2.3 From 046d84f45621f7e128cb30160a315f98881c6f4b Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Sun, 17 Nov 2019 18:13:36 +0100 Subject: 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(-) (limited to 'src/wx/video_view.cc') 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 -- cgit v1.2.3 From 86a866d5f3f5bf2fec67d1c813524479c6727eab Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Tue, 19 Nov 2019 17:07:35 +0100 Subject: Clean up access to stuff from Film. --- src/lib/film.cc | 7 +++++++ src/lib/film.h | 7 +++++++ src/lib/playlist.cc | 11 ++++++++++- src/lib/playlist.h | 2 ++ src/wx/film_viewer.cc | 16 ++++++++++++++-- src/wx/film_viewer.h | 1 + src/wx/gl_video_view.cc | 40 ++++------------------------------------ src/wx/gl_video_view.h | 5 ----- src/wx/simple_video_view.cc | 4 ++-- src/wx/video_view.cc | 2 +- src/wx/video_view.h | 22 ++++++++++++++++------ 11 files changed, 64 insertions(+), 53 deletions(-) (limited to 'src/wx/video_view.cc') diff --git a/src/lib/film.cc b/src/lib/film.cc index 2a50e8c81..aa71834a1 100644 --- a/src/lib/film.cc +++ b/src/lib/film.cc @@ -169,6 +169,7 @@ Film::Film (optional dir) _playlist_change_connection = _playlist->Change.connect (bind (&Film::playlist_change, this, _1)); _playlist_order_changed_connection = _playlist->OrderChanged.connect (bind (&Film::playlist_order_changed, this)); _playlist_content_change_connection = _playlist->ContentChange.connect (bind (&Film::playlist_content_change, this, _1, _2, _3, _4)); + _playlist_length_change_connection = _playlist->LengthChange.connect (bind(&Film::playlist_length_change, this)); if (dir) { /* Make state.directory a complete path without ..s (where possible) @@ -1292,6 +1293,12 @@ Film::playlist_content_change (ChangeType type, weak_ptr c, int p, bool } } +void +Film::playlist_length_change () +{ + LengthChange (); +} + void Film::playlist_change (ChangeType type) { diff --git a/src/lib/film.h b/src/lib/film.h index 68f8b5334..c72251880 100644 --- a/src/lib/film.h +++ b/src/lib/film.h @@ -390,6 +390,11 @@ public: /** Emitted when some property of our content has changed */ mutable boost::signals2::signal, int, bool)> ContentChange; + /** Emitted when the film's length might have changed; this is not like a normal + property as its value is derived from the playlist, so it has its own signal. + */ + mutable boost::signals2::signal LengthChange; + /** Emitted when we have something important to tell the user */ boost::signals2::signal Message; @@ -409,6 +414,7 @@ private: void playlist_change (ChangeType); void playlist_order_changed (); void playlist_content_change (ChangeType type, boost::weak_ptr, int, bool frequent); + void playlist_length_change (); void maybe_add_content (boost::weak_ptr, boost::weak_ptr, bool disable_audio_analysis); void audio_analysis_finished (); void check_settings_consistency (); @@ -486,6 +492,7 @@ private: boost::signals2::scoped_connection _playlist_change_connection; boost::signals2::scoped_connection _playlist_order_changed_connection; boost::signals2::scoped_connection _playlist_content_change_connection; + boost::signals2::scoped_connection _playlist_length_change_connection; std::list _job_connections; std::list _audio_analysis_connections; diff --git a/src/lib/playlist.cc b/src/lib/playlist.cc index 9e96c693a..73a3214d3 100644 --- a/src/lib/playlist.cc +++ b/src/lib/playlist.cc @@ -113,6 +113,9 @@ Playlist::content_change (weak_ptr weak_film, ChangeType type, weak_ if (changed) { OrderChanged (); } + + /* The length might have changed, and that's good enough for this signal */ + LengthChange (); } } @@ -281,6 +284,8 @@ Playlist::add (shared_ptr film, shared_ptr c) } Change (CHANGE_TYPE_DONE); + + LengthChange (); } void @@ -312,6 +317,8 @@ Playlist::remove (shared_ptr c) } /* This won't change order, so it does not need a sort */ + + LengthChange (); } void @@ -334,9 +341,11 @@ Playlist::remove (ContentList c) } } + Change (CHANGE_TYPE_DONE); + /* This won't change order, so it does not need a sort */ - Change (CHANGE_TYPE_DONE); + LengthChange (); } class FrameRateCandidate diff --git a/src/lib/playlist.h b/src/lib/playlist.h index d7db75d0f..b6e23b4a5 100644 --- a/src/lib/playlist.h +++ b/src/lib/playlist.h @@ -78,6 +78,8 @@ public: /** Emitted when content has been added to or removed from the playlist; implies OrderChanged */ mutable boost::signals2::signal Change; mutable boost::signals2::signal OrderChanged; + /** Emitted when the length might have changed; may sometimes be emitted when it has not */ + mutable boost::signals2::signal LengthChange; mutable boost::signals2::signal, int, bool)> ContentChange; diff --git a/src/wx/film_viewer.cc b/src/wx/film_viewer.cc index f3250dbfa..31093cb0a 100644 --- a/src/wx/film_viewer.cc +++ b/src/wx/film_viewer.cc @@ -157,7 +157,6 @@ FilmViewer::set_film (shared_ptr film) _film = film; - _video_view->set_film (_film); _video_view->clear (); _closed_captions_dialog->clear (); @@ -184,6 +183,7 @@ FilmViewer::set_film (shared_ptr film) _player->set_play_referenced (); _film->Change.connect (boost::bind (&FilmViewer::film_change, this, _1, _2)); + _film->LengthChange.connect (boost::bind(&FilmViewer::film_length_change, this)); _player->Change.connect (boost::bind (&FilmViewer::player_change, this, _1, _2, _3)); /* Keep about 1 second's worth of history samples */ @@ -384,11 +384,23 @@ FilmViewer::player_change (ChangeType type, int property, bool frequent) void FilmViewer::film_change (ChangeType type, Film::Property p) { - if (type == CHANGE_TYPE_DONE && p == Film::AUDIO_CHANNELS) { + if (type != CHANGE_TYPE_DONE) { + return; + } + + if (p == Film::AUDIO_CHANNELS) { recreate_butler (); + } else if (p == Film::VIDEO_FRAME_RATE) { + _video_view->set_video_frame_rate (_film->video_frame_rate()); } } +void +FilmViewer::film_length_change () +{ + _video_view->set_length (_film->length()); +} + /** Re-get the current frame slowly by seeking */ void FilmViewer::slow_refresh () diff --git a/src/wx/film_viewer.h b/src/wx/film_viewer.h index eaf46f1e6..e42c37a8d 100644 --- a/src/wx/film_viewer.h +++ b/src/wx/film_viewer.h @@ -165,6 +165,7 @@ private: void film_change (ChangeType, Film::Property); void recreate_butler (); void config_changed (Config::Property); + void film_length_change (); dcpomatic::DCPTime time () const; boost::optional audio_time () const; diff --git a/src/wx/gl_video_view.cc b/src/wx/gl_video_view.cc index c3a611283..3cf58757d 100644 --- a/src/wx/gl_video_view.cc +++ b/src/wx/gl_video_view.cc @@ -55,7 +55,6 @@ GLVideoView::GLVideoView (FilmViewer* viewer, wxWindow *parent) : VideoView (viewer) , _vsync_enabled (false) , _thread (0) - , _one_shot (false) { _canvas = new wxGLCanvas (parent, wxID_ANY, 0, wxDefaultPosition, wxDefaultSize, wxFULL_REPAINT_ON_RESIZE); _canvas->Bind (wxEVT_PAINT, boost::bind(&GLVideoView::paint, this)); @@ -278,20 +277,6 @@ GLVideoView::stop () _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 @@ -303,20 +288,12 @@ try _canvas->SetCurrent (*_context); } - while (true) { - if (!film() && !one_shot()) { - /* XXX: this should be an indefinite wait until - one of our conditions becomes true. - */ - dcpomatic_sleep_milliseconds (40); - continue; - } - - set_one_shot (false); + std::cout << "Here we go " << video_frame_rate() << " " << to_string(length()) << "\n"; + while (true) { dcpomatic::DCPTime const next = position() + one_video_frame(); - if (next >= film()->length()) { + if (next >= length()) { _viewer->stop (); _viewer->emit_finished (); continue; @@ -359,15 +336,6 @@ GLVideoView::context () const bool GLVideoView::display_next_frame (bool non_blocking) { - bool const g = get_next_frame (non_blocking); - set_one_shot (true); - return g; + return get_next_frame (non_blocking); } -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 cf42432a9..73db3535d 100644 --- a/src/wx/gl_video_view.h +++ b/src/wx/gl_video_view.h @@ -53,9 +53,6 @@ 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; @@ -66,6 +63,4 @@ 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 a00524f7d..dcf30cd1a 100644 --- a/src/wx/simple_video_view.cc +++ b/src/wx/simple_video_view.cc @@ -145,14 +145,14 @@ SimpleVideoView::update () void SimpleVideoView::timer () { - if (!film() || !_viewer->playing()) { + if (!_viewer->playing()) { return; } display_next_frame (false); DCPTime const next = _viewer->position() + _viewer->one_video_frame(); - if (next >= film()->length()) { + if (next >= length()) { _viewer->stop (); _viewer->Finished (); return; diff --git a/src/wx/video_view.cc b/src/wx/video_view.cc index e1a8b7306..6478ff2a6 100644 --- a/src/wx/video_view.cc +++ b/src/wx/video_view.cc @@ -67,7 +67,7 @@ VideoView::get_next_frame (bool non_blocking) dcpomatic::DCPTime VideoView::one_video_frame () const { - return dcpomatic::DCPTime::from_frames (1, film()->video_frame_rate()); + return dcpomatic::DCPTime::from_frames (1, video_frame_rate()); } /* XXX_b: comment */ diff --git a/src/wx/video_view.h b/src/wx/video_view.h index d9ef2a65f..06067130c 100644 --- a/src/wx/video_view.h +++ b/src/wx/video_view.h @@ -39,6 +39,7 @@ public: #ifdef DCPOMATIC_VARIANT_SWAROOP , _in_watermark (false) #endif + , _video_frame_rate (0) {} virtual ~VideoView () {} @@ -66,9 +67,14 @@ public: return _player_video.second; } - void set_film (boost::shared_ptr film) { + void set_video_frame_rate (int r) { boost::mutex::scoped_lock lm (_mutex); - _film = film; + _video_frame_rate = r; + } + + void set_length (dcpomatic::DCPTime len) { + boost::mutex::scoped_lock lm (_mutex); + _length = len; } protected: @@ -78,10 +84,13 @@ protected: bool get_next_frame (bool non_blocking); int time_until_next_frame () const; dcpomatic::DCPTime one_video_frame () const; - - boost::shared_ptr film () const { + int video_frame_rate () const { + boost::mutex::scoped_lock lm (_mutex); + return _video_frame_rate; + } + dcpomatic::DCPTime length () const { boost::mutex::scoped_lock lm (_mutex); - return _film; + return _length; } FilmViewer* _viewer; @@ -97,7 +106,8 @@ protected: #endif private: - boost::shared_ptr _film; + int _video_frame_rate; + dcpomatic::DCPTime _length; }; #endif -- cgit v1.2.3 From 5ee3fcbd31065275faad4c06a7805dcbcb338812 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Wed, 20 Nov 2019 00:57:37 +0100 Subject: Add comment. --- src/wx/video_view.cc | 1 + 1 file changed, 1 insertion(+) (limited to 'src/wx/video_view.cc') diff --git a/src/wx/video_view.cc b/src/wx/video_view.cc index 6478ff2a6..f2886a48c 100644 --- a/src/wx/video_view.cc +++ b/src/wx/video_view.cc @@ -55,6 +55,7 @@ VideoView::get_next_frame (bool non_blocking) _player_video.first->eyes() != EYES_BOTH ); + /* XXX_b: this is called from the GL thread so it shouldn't be opening error dialogs */ try { _viewer->butler()->rethrow (); } catch (DecodeError& e) { -- cgit v1.2.3 From 4f7e9f125716a27ed9e2a8e30f067100cbee773a Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Wed, 20 Nov 2019 21:25:55 +0100 Subject: Fix display when there is no film. --- src/wx/gl_video_view.cc | 65 +++++++++++++++++++++++++++++-------------------- src/wx/gl_video_view.h | 2 ++ src/wx/video_view.cc | 12 +++++++-- src/wx/video_view.h | 1 + 4 files changed, 52 insertions(+), 28 deletions(-) (limited to 'src/wx/video_view.cc') diff --git a/src/wx/gl_video_view.cc b/src/wx/gl_video_view.cc index 236b40518..a1be8ca30 100644 --- a/src/wx/gl_video_view.cc +++ b/src/wx/gl_video_view.cc @@ -61,6 +61,7 @@ GLVideoView::GLVideoView (FilmViewer* viewer, wxWindow *parent) _canvas = new wxGLCanvas (parent, wxID_ANY, 0, wxDefaultPosition, wxDefaultSize, wxFULL_REPAINT_ON_RESIZE); _canvas->Bind (wxEVT_PAINT, boost::bind(&GLVideoView::paint, this)); _canvas->Bind (wxEVT_SIZE, boost::bind(boost::ref(Sized))); + _canvas->Bind (wxEVT_CREATE, boost::bind(&GLVideoView::create, this)); #if defined(DCPOMATIC_LINUX) && defined(DCPOMATIC_HAVE_GLX_SWAP_INTERVAL_EXT) if (_canvas->IsExtensionSupported("GLX_EXT_swap_control")) { @@ -116,14 +117,7 @@ check_gl_error (char const * last) void GLVideoView::paint () { - /* XXX_b: can't do this yet */ -#if 0 - _viewer->state_timer().set("paint-panel"); - _canvas->SetCurrent (*_context); - wxPaintDC dc (_canvas); - draw (); - _viewer->state_timer().unset(); -#endif + request_one_shot (); } void @@ -169,9 +163,11 @@ GLVideoView::draw () glTranslatef (0, 0, 0); + dcp::Size const out_size = _viewer->out_size (); + if (_size) { + /* Render our image (texture) */ glBegin (GL_QUADS); - glTexCoord2f (0, 1); glVertex2f (0, _size->height); glTexCoord2f (1, 1); @@ -180,11 +176,19 @@ GLVideoView::draw () glVertex2f (_size->width, 0); glTexCoord2f (0, 0); glVertex2f (0, 0); - + glEnd (); + } else { + /* No image, so just fill with black */ + glBegin (GL_QUADS); + glColor3ub (0, 0, 0); + glVertex2f (0, 0); + glVertex2f (out_size.width, 0); + glVertex2f (out_size.width, out_size.height); + glVertex2f (0, out_size.height); + glVertex2f (0, 0); glEnd (); } - dcp::Size const out_size = _viewer->out_size (); wxSize const canvas_size = _canvas->GetSize (); if (!_viewer->pad_black() && out_size.width < canvas_size.GetWidth()) { @@ -263,10 +267,6 @@ GLVideoView::set_image (shared_ptr image) void GLVideoView::start () { - if (!_thread) { - _thread = new boost::thread (boost::bind(&GLVideoView::thread, this)); - } - boost::mutex::scoped_lock lm (_playing_mutex); _playing = true; _playing_condition.notify_all (); @@ -287,8 +287,6 @@ try _context = new wxGLContext (_canvas); _canvas->SetCurrent (*_context); - std::cout << "Here we go " << video_frame_rate() << " " << to_string(length()) << "\n"; - while (true) { boost::mutex::scoped_lock lm (_playing_mutex); while (!_playing && !_one_shot) { @@ -297,16 +295,18 @@ try _one_shot = false; lm.unlock (); - dcpomatic::DCPTime const next = position() + one_video_frame(); + if (length() != dcpomatic::DCPTime()) { + dcpomatic::DCPTime const next = position() + one_video_frame(); - if (next >= length()) { - _viewer->stop (); - _viewer->emit_finished (); - continue; - } + if (next >= length()) { + _viewer->stop (); + _viewer->emit_finished (); + continue; + } - get_next_frame (false); - set_image (player_video().first->image(bind(&PlayerVideo::force, _1, AV_PIX_FMT_RGB24), false, true)); + get_next_frame (false); + set_image (player_video().first->image(bind(&PlayerVideo::force, _1, AV_PIX_FMT_RGB24), false, true)); + } draw (); while (time_until_next_frame() < 5) { @@ -330,9 +330,22 @@ bool GLVideoView::display_next_frame (bool non_blocking) { bool const r = get_next_frame (non_blocking); + request_one_shot (); + return r; +} + +void +GLVideoView::request_one_shot () +{ boost::mutex::scoped_lock lm (_playing_mutex); _one_shot = true; _playing_condition.notify_all (); - return r; } +void +GLVideoView::create () +{ + if (!_thread) { + _thread = new boost::thread (boost::bind(&GLVideoView::thread, this)); + } +} diff --git a/src/wx/gl_video_view.h b/src/wx/gl_video_view.h index 22b6d8513..a9264f05d 100644 --- a/src/wx/gl_video_view.h +++ b/src/wx/gl_video_view.h @@ -53,6 +53,8 @@ private: void paint (); void draw (); void thread (); + void request_one_shot (); + void create (); wxGLCanvas* _canvas; wxGLContext* _context; diff --git a/src/wx/video_view.cc b/src/wx/video_view.cc index f2886a48c..f8d44dc70 100644 --- a/src/wx/video_view.cc +++ b/src/wx/video_view.cc @@ -37,6 +37,10 @@ VideoView::clear () bool VideoView::get_next_frame (bool non_blocking) { + if (_length == dcpomatic::DCPTime()) { + return true; + } + DCPOMATIC_ASSERT (_viewer->butler()); _viewer->_gets++; @@ -71,13 +75,17 @@ VideoView::one_video_frame () const return dcpomatic::DCPTime::from_frames (1, video_frame_rate()); } -/* XXX_b: comment */ +/** @return Time in ms until the next frame is due */ int VideoView::time_until_next_frame () const { + if (length() == dcpomatic::DCPTime()) { + /* There's no content, so this doesn't matter */ + return 0; + } + 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; } diff --git a/src/wx/video_view.h b/src/wx/video_view.h index 8d763204c..142cfd022 100644 --- a/src/wx/video_view.h +++ b/src/wx/video_view.h @@ -114,6 +114,7 @@ private: std::pair, dcpomatic::DCPTime> _player_video; int _video_frame_rate; + /** length of the film we are playing, or 0 if there is none */ dcpomatic::DCPTime _length; }; -- cgit v1.2.3 From 83c9e9c858072ab919916269790dcc65565fdd25 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Wed, 20 Nov 2019 22:18:33 +0100 Subject: Don't try opening error dialogues from the GL thread. --- src/wx/gl_video_view.cc | 16 ++++++++++++++++ src/wx/gl_video_view.h | 3 +++ src/wx/simple_video_view.cc | 6 ++++++ src/wx/video_view.cc | 7 ------- 4 files changed, 25 insertions(+), 7 deletions(-) (limited to 'src/wx/video_view.cc') diff --git a/src/wx/gl_video_view.cc b/src/wx/gl_video_view.cc index a1be8ca30..836b3eb35 100644 --- a/src/wx/gl_video_view.cc +++ b/src/wx/gl_video_view.cc @@ -20,11 +20,13 @@ #include "gl_video_view.h" #include "film_viewer.h" +#include "wx_util.h" #include "lib/image.h" #include "lib/dcpomatic_assert.h" #include "lib/exceptions.h" #include "lib/cross.h" #include "lib/player_video.h" +#include "lib/butler.h" #include #include @@ -63,6 +65,10 @@ GLVideoView::GLVideoView (FilmViewer* viewer, wxWindow *parent) _canvas->Bind (wxEVT_SIZE, boost::bind(boost::ref(Sized))); _canvas->Bind (wxEVT_CREATE, boost::bind(&GLVideoView::create, this)); + _canvas->Bind (wxEVT_TIMER, boost::bind(&GLVideoView::check_for_butler_errors, this)); + _timer.reset (new wxTimer(_canvas)); + _timer->Start (2000); + #if defined(DCPOMATIC_LINUX) && defined(DCPOMATIC_HAVE_GLX_SWAP_INTERVAL_EXT) if (_canvas->IsExtensionSupported("GLX_EXT_swap_control")) { /* Enable vsync */ @@ -105,6 +111,16 @@ GLVideoView::~GLVideoView () glDeleteTextures (1, &_id); } +void +GLVideoView::check_for_butler_errors () +{ + try { + _viewer->butler()->rethrow (); + } catch (DecodeError& e) { + error_dialog (get(), e.what()); + } +} + static void check_gl_error (char const * last) { diff --git a/src/wx/gl_video_view.h b/src/wx/gl_video_view.h index a9264f05d..614393024 100644 --- a/src/wx/gl_video_view.h +++ b/src/wx/gl_video_view.h @@ -55,6 +55,7 @@ private: void thread (); void request_one_shot (); void create (); + void check_for_butler_errors (); wxGLCanvas* _canvas; wxGLContext* _context; @@ -68,4 +69,6 @@ private: boost::condition _playing_condition; bool _playing; bool _one_shot; + + boost::shared_ptr _timer; }; diff --git a/src/wx/simple_video_view.cc b/src/wx/simple_video_view.cc index 7aeb317b2..f928770ad 100644 --- a/src/wx/simple_video_view.cc +++ b/src/wx/simple_video_view.cc @@ -194,6 +194,12 @@ SimpleVideoView::display_next_frame (bool non_blocking) display_player_video (); + try { + _viewer->butler()->rethrow (); + } catch (DecodeError& e) { + error_dialog (get(), e.what()); + } + return true; } diff --git a/src/wx/video_view.cc b/src/wx/video_view.cc index f8d44dc70..ede0708c2 100644 --- a/src/wx/video_view.cc +++ b/src/wx/video_view.cc @@ -59,13 +59,6 @@ VideoView::get_next_frame (bool non_blocking) _player_video.first->eyes() != EYES_BOTH ); - /* XXX_b: this is called from the GL thread so it shouldn't be opening error dialogs */ - try { - _viewer->butler()->rethrow (); - } catch (DecodeError& e) { - error_dialog (get(), e.what()); - } - return true; } -- cgit v1.2.3 From 5aff601c454fa756c0ab71ae4bcf8f7f4ce28737 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Wed, 20 Nov 2019 23:07:55 +0100 Subject: Move _dropped into VideoView. --- src/wx/film_viewer.cc | 8 ++++++-- src/wx/film_viewer.h | 5 +---- src/wx/gl_video_view.cc | 3 +++ src/wx/simple_video_view.cc | 3 ++- src/wx/video_view.cc | 17 +++++++++++++++++ src/wx/video_view.h | 24 ++++++++++++++---------- 6 files changed, 43 insertions(+), 17 deletions(-) (limited to 'src/wx/video_view.cc') diff --git a/src/wx/film_viewer.cc b/src/wx/film_viewer.cc index c1d04be03..cb9c85b3d 100644 --- a/src/wx/film_viewer.cc +++ b/src/wx/film_viewer.cc @@ -86,7 +86,6 @@ FilmViewer::FilmViewer (wxWindow* p) , _playing (false) , _suspended (0) , _latency_history_count (0) - , _dropped (0) , _closed_captions_dialog (new ClosedCaptionsDialog(p, this)) , _outline_content (false) , _eyes (EYES_LEFT) @@ -326,7 +325,6 @@ FilmViewer::start () _audio.startStream (); } - _dropped = 0; _playing = true; _video_view->start (); Started (position()); @@ -656,3 +654,9 @@ FilmViewer::emit_finished () emit (boost::bind(boost::ref(Finished))); } +int +FilmViewer::dropped () const +{ + return _video_view->dropped (); +} + diff --git a/src/wx/film_viewer.h b/src/wx/film_viewer.h index e42c37a8d..93a311981 100644 --- a/src/wx/film_viewer.h +++ b/src/wx/film_viewer.h @@ -92,9 +92,7 @@ public: void slow_refresh (); - int dropped () const { - return _dropped; - } + int dropped () const; int audio_callback (void* out, unsigned int frames); @@ -200,7 +198,6 @@ private: mutable boost::mutex _latency_history_mutex; int _latency_history_count; - int _dropped; boost::optional _dcp_decode_reduction; ClosedCaptionsDialog* _closed_captions_dialog; diff --git a/src/wx/gl_video_view.cc b/src/wx/gl_video_view.cc index 934a91ad5..55c6cc969 100644 --- a/src/wx/gl_video_view.cc +++ b/src/wx/gl_video_view.cc @@ -275,6 +275,8 @@ GLVideoView::set_image (shared_ptr image) void GLVideoView::start () { + VideoView::start (); + boost::mutex::scoped_lock lm (_playing_mutex); _playing = true; _playing_condition.notify_all (); @@ -319,6 +321,7 @@ try while (time_until_next_frame() < 5) { get_next_frame (true); + add_dropped (); } boost::this_thread::interruption_point (); diff --git a/src/wx/simple_video_view.cc b/src/wx/simple_video_view.cc index f928770ad..33e2834c5 100644 --- a/src/wx/simple_video_view.cc +++ b/src/wx/simple_video_view.cc @@ -169,6 +169,7 @@ SimpleVideoView::timer () void SimpleVideoView::start () { + VideoView::start (); timer (); } @@ -216,7 +217,7 @@ SimpleVideoView::display_player_video () /* Too late; just drop this frame before we try to get its image (which will be the time-consuming part if this frame is J2K). */ - ++_viewer->_dropped; + add_dropped (); return; } diff --git a/src/wx/video_view.cc b/src/wx/video_view.cc index ede0708c2..d54487a93 100644 --- a/src/wx/video_view.cc +++ b/src/wx/video_view.cc @@ -23,6 +23,17 @@ #include "film_viewer.h" #include "lib/butler.h" +VideoView::VideoView (FilmViewer* viewer) + : _viewer (viewer) +#ifdef DCPOMATIC_VARIANT_SWAROOP + , _in_watermark (false) +#endif + , _video_frame_rate (0) + , _dropped (0) +{ + +} + void VideoView::clear () { @@ -85,3 +96,9 @@ VideoView::time_until_next_frame () const return (next.seconds() - time.seconds()) * 1000; } +void +VideoView::start () +{ + boost::mutex::scoped_lock lm (_mutex); + _dropped = 0; +} diff --git a/src/wx/video_view.h b/src/wx/video_view.h index 656d8621e..8d9dce68d 100644 --- a/src/wx/video_view.h +++ b/src/wx/video_view.h @@ -34,14 +34,7 @@ class PlayerVideo; class VideoView { public: - VideoView (FilmViewer* viewer) - : _viewer (viewer) -#ifdef DCPOMATIC_VARIANT_SWAROOP - , _in_watermark (false) -#endif - , _video_frame_rate (0) - {} - + VideoView (FilmViewer* viewer); virtual ~VideoView () {} virtual void set_image (boost::shared_ptr image) = 0; @@ -51,8 +44,7 @@ public: */ virtual void update () = 0; - /* XXX_b: make pure */ - virtual void start () {} + virtual void start (); /* XXX_b: make pure */ virtual void stop () {} @@ -65,6 +57,11 @@ public: /* XXX_b: to remove */ virtual void display_player_video () {} + int dropped () const { + boost::mutex::scoped_lock lm (_mutex); + return _dropped; + } + dcpomatic::DCPTime position () const { boost::mutex::scoped_lock lm (_mutex); return _player_video.second; @@ -103,6 +100,11 @@ protected: return _player_video; } + void add_dropped () { + boost::mutex::scoped_lock lm (_mutex); + ++_dropped; + } + FilmViewer* _viewer; #ifdef DCPOMATIC_VARIANT_SWAROOP @@ -119,6 +121,8 @@ private: int _video_frame_rate; /** length of the film we are playing, or 0 if there is none */ dcpomatic::DCPTime _length; + + int _dropped; }; #endif -- cgit v1.2.3 From 9001a63be211fd8e97431f8fc07c66af01554f5a Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Thu, 21 Nov 2019 00:05:12 +0100 Subject: Move _state_timer into VideoView. --- src/wx/film_viewer.cc | 3 --- src/wx/film_viewer.h | 13 ++++--------- src/wx/simple_video_view.cc | 12 +++++++----- src/wx/video_view.cc | 1 + src/wx/video_view.h | 7 +++++++ 5 files changed, 19 insertions(+), 17 deletions(-) (limited to 'src/wx/video_view.cc') diff --git a/src/wx/film_viewer.cc b/src/wx/film_viewer.cc index cb9c85b3d..c323c8281 100644 --- a/src/wx/film_viewer.cc +++ b/src/wx/film_viewer.cc @@ -93,7 +93,6 @@ FilmViewer::FilmViewer (wxWindow* p) #ifdef DCPOMATIC_VARIANT_SWAROOP , _background_image (false) #endif - , _state_timer ("viewer") , _gets (0) , _idle_get (false) { @@ -228,9 +227,7 @@ FilmViewer::recreate_butler () void FilmViewer::refresh_view () { - _state_timer.set ("update-view"); _video_view->update (); - _state_timer.unset (); } void diff --git a/src/wx/film_viewer.h b/src/wx/film_viewer.h index c6fbd66e4..0699b12ae 100644 --- a/src/wx/film_viewer.h +++ b/src/wx/film_viewer.h @@ -107,18 +107,14 @@ public: } #endif - StateTimer const & state_timer () const { - return _state_timer; - } - - StateTimer& state_timer () { - return _state_timer; - } - int gets () const { return _gets; } + StateTimer const & state_timer () const { + return _video_view->state_timer (); + } + /* Some accessors and utility methods that VideoView classes need */ dcp::Size out_size () const { return _out_size; @@ -207,7 +203,6 @@ private: bool _background_image; #endif - StateTimer _state_timer; int _gets; /** true if an get() is required next time we are idle */ diff --git a/src/wx/simple_video_view.cc b/src/wx/simple_video_view.cc index e71d89412..437aed4fa 100644 --- a/src/wx/simple_video_view.cc +++ b/src/wx/simple_video_view.cc @@ -56,7 +56,7 @@ SimpleVideoView::SimpleVideoView (FilmViewer* viewer, wxWindow* parent) void SimpleVideoView::paint () { - _viewer->state_timer().set("paint-panel"); + _state_timer.set("paint-panel"); wxPaintDC dc (_panel); dcp::Size const out_size = _viewer->out_size (); @@ -130,14 +130,16 @@ SimpleVideoView::paint () dc.SetBrush (*wxTRANSPARENT_BRUSH); dc.DrawRectangle (_inter_position.x, _inter_position.y + (panel_size.GetHeight() - out_size.height) / 2, _inter_size.width, _inter_size.height); } - _viewer->state_timer().unset(); + _state_timer.unset(); } void SimpleVideoView::update () { + _state_timer.set ("update-view"); _panel->Refresh (); _panel->Update (); + _state_timer.unset (); } void @@ -237,15 +239,15 @@ SimpleVideoView::display_player_video () * image and convert it (from whatever the user has said it is) to RGB. */ - _viewer->_state_timer.set ("get image"); + _state_timer.set ("get image"); set_image ( player_video().first->image(bind(&PlayerVideo::force, _1, AV_PIX_FMT_RGB24), false, true) ); - _viewer->_state_timer.set ("ImageChanged"); + _state_timer.set ("ImageChanged"); _viewer->ImageChanged (player_video().first); - _viewer->_state_timer.unset (); + _state_timer.unset (); _inter_position = player_video().first->inter_position (); _inter_size = player_video().first->inter_size (); diff --git a/src/wx/video_view.cc b/src/wx/video_view.cc index d54487a93..e65888656 100644 --- a/src/wx/video_view.cc +++ b/src/wx/video_view.cc @@ -28,6 +28,7 @@ VideoView::VideoView (FilmViewer* viewer) #ifdef DCPOMATIC_VARIANT_SWAROOP , _in_watermark (false) #endif + , _state_timer ("viewer") , _video_frame_rate (0) , _dropped (0) { diff --git a/src/wx/video_view.h b/src/wx/video_view.h index 8d9dce68d..4a94aa879 100644 --- a/src/wx/video_view.h +++ b/src/wx/video_view.h @@ -22,6 +22,7 @@ #define DCPOMATIC_VIDEO_VIEW_H #include "lib/dcpomatic_time.h" +#include "lib/timer.h" #include #include #include @@ -62,6 +63,10 @@ public: return _dropped; } + StateTimer const & state_timer () const { + return _state_timer; + } + dcpomatic::DCPTime position () const { boost::mutex::scoped_lock lm (_mutex); return _player_video.second; @@ -113,6 +118,8 @@ protected: int _watermark_y; #endif + StateTimer _state_timer; + private: /** Mutex protecting all the state in VideoView */ mutable boost::mutex _mutex; -- cgit v1.2.3 From b3b371294ed5e6cc18ef64ba1b06ca76726b903a Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Thu, 21 Nov 2019 22:38:33 +0100 Subject: Remove some friends from FilmViewer. --- src/wx/closed_captions_dialog.cc | 19 ++++++++++++++++--- src/wx/closed_captions_dialog.h | 5 +++-- src/wx/film_viewer.cc | 11 ++++++++--- src/wx/film_viewer.h | 18 ++++-------------- src/wx/simple_video_view.cc | 2 -- src/wx/video_view.cc | 6 ++++-- src/wx/video_view.h | 18 ++++++++++++++++++ 7 files changed, 53 insertions(+), 26 deletions(-) (limited to 'src/wx/video_view.cc') diff --git a/src/wx/closed_captions_dialog.cc b/src/wx/closed_captions_dialog.cc index 061262cdd..afcf5fa87 100644 --- a/src/wx/closed_captions_dialog.cc +++ b/src/wx/closed_captions_dialog.cc @@ -53,6 +53,7 @@ ClosedCaptionsDialog::ClosedCaptionsDialog (wxWindow* parent, FilmViewer* viewer , _display (new wxPanel(this, wxID_ANY, wxDefaultPosition, wxSize(640, (640 / 10) + 64))) , _track (new wxChoice(this, wxID_ANY)) , _current_in_lines (false) + , _timer (this) { _lines.resize (CLOSED_CAPTION_LINES); @@ -65,18 +66,30 @@ ClosedCaptionsDialog::ClosedCaptionsDialog (wxWindow* parent, FilmViewer* viewer sizer->Add (track_sizer, 0, wxALL, DCPOMATIC_SIZER_GAP); sizer->Add (_display, 1, wxEXPAND); + Bind (wxEVT_SHOW, boost::bind(&ClosedCaptionsDialog::shown, this, _1)); + Bind (wxEVT_TIMER, boost::bind(&ClosedCaptionsDialog::update, this)); _display->Bind (wxEVT_PAINT, boost::bind(&ClosedCaptionsDialog::paint, this)); _track->Bind (wxEVT_CHOICE, boost::bind(&ClosedCaptionsDialog::track_selected, this)); SetSizerAndFit (sizer); } +void +ClosedCaptionsDialog::shown (wxShowEvent ev) +{ + if (ev.IsShown ()) { + _timer.Start (40); + } else { + _timer.Stop (); + } +} + void ClosedCaptionsDialog::track_selected () { _current = optional (); _viewer->slow_refresh (); - update (_last_update); + update (); } void @@ -131,9 +144,9 @@ private: }; void -ClosedCaptionsDialog::update (DCPTime time) +ClosedCaptionsDialog::update () { - _last_update = time; + DCPTime const time = _viewer->time (); if (_current_in_lines && _current && _current->period.to > time) { /* Current one is fine */ diff --git a/src/wx/closed_captions_dialog.h b/src/wx/closed_captions_dialog.h index fb4e9d59b..5c366ca7b 100644 --- a/src/wx/closed_captions_dialog.h +++ b/src/wx/closed_captions_dialog.h @@ -31,11 +31,12 @@ class ClosedCaptionsDialog : public wxDialog public: explicit ClosedCaptionsDialog (wxWindow* parent, FilmViewer* viewer); - void update (dcpomatic::DCPTime); void clear (); void set_film_and_butler (boost::shared_ptr, boost::weak_ptr); private: + void shown (wxShowEvent); + void update (); void paint (); void track_selected (); @@ -47,5 +48,5 @@ private: std::vector _lines; std::vector _tracks; boost::weak_ptr _butler; - dcpomatic::DCPTime _last_update; + wxTimer _timer; }; diff --git a/src/wx/film_viewer.cc b/src/wx/film_viewer.cc index 7437612af..dcbfedc81 100644 --- a/src/wx/film_viewer.cc +++ b/src/wx/film_viewer.cc @@ -88,12 +88,10 @@ FilmViewer::FilmViewer (wxWindow* p) , _latency_history_count (0) , _closed_captions_dialog (new ClosedCaptionsDialog(p, this)) , _outline_content (false) - , _eyes (EYES_LEFT) , _pad_black (false) #ifdef DCPOMATIC_VARIANT_SWAROOP , _background_image (false) #endif - , _gets (0) , _idle_get (false) { switch (Config::instance()->video_view_type()) { @@ -234,7 +232,7 @@ FilmViewer::set_outline_content (bool o) void FilmViewer::set_eyes (Eyes e) { - _eyes = e; + _video_view->set_eyes (e); slow_refresh (); } @@ -651,3 +649,10 @@ FilmViewer::dropped () const return _video_view->dropped (); } +int +FilmViewer::gets () const +{ + return _video_view->gets (); +} + + diff --git a/src/wx/film_viewer.h b/src/wx/film_viewer.h index 21195e747..ec481f34e 100644 --- a/src/wx/film_viewer.h +++ b/src/wx/film_viewer.h @@ -92,7 +92,11 @@ public: void slow_refresh (); + dcpomatic::DCPTime time () const; + boost::optional audio_time () const; + int dropped () const; + int gets () const; int audio_callback (void* out, unsigned int frames); @@ -107,10 +111,6 @@ public: } #endif - int gets () const { - return _gets; - } - StateTimer const & state_timer () const { return _video_view->state_timer (); } @@ -143,11 +143,6 @@ public: private: - /* XXX_b: to remove */ - friend class SimpleVideoView; - friend class GLVideoView; - friend class VideoView; - void video_view_sized (); void calculate_sizes (); void player_change (ChangeType type, int, bool); @@ -158,8 +153,6 @@ private: void config_changed (Config::Property); void film_length_change (); - dcpomatic::DCPTime time () const; - boost::optional audio_time () const; dcpomatic::DCPTime uncorrected_time () const; Frame average_latency () const; @@ -192,7 +185,6 @@ private: ClosedCaptionsDialog* _closed_captions_dialog; bool _outline_content; - Eyes _eyes; /** true to pad the viewer panel with black, false to use the normal window background colour. */ @@ -202,8 +194,6 @@ private: bool _background_image; #endif - int _gets; - /** true if an get() is required next time we are idle */ bool _idle_get; diff --git a/src/wx/simple_video_view.cc b/src/wx/simple_video_view.cc index 63dc10200..22e4db887 100644 --- a/src/wx/simple_video_view.cc +++ b/src/wx/simple_video_view.cc @@ -253,6 +253,4 @@ SimpleVideoView::display_player_video () _inter_size = player_video().first->inter_size (); update (); - - _viewer->closed_captions_dialog()->update (_viewer->time()); } diff --git a/src/wx/video_view.cc b/src/wx/video_view.cc index e65888656..5dd857fab 100644 --- a/src/wx/video_view.cc +++ b/src/wx/video_view.cc @@ -30,7 +30,9 @@ VideoView::VideoView (FilmViewer* viewer) #endif , _state_timer ("viewer") , _video_frame_rate (0) + , _eyes (EYES_LEFT) , _dropped (0) + , _gets (0) { } @@ -54,7 +56,7 @@ VideoView::get_next_frame (bool non_blocking) } DCPOMATIC_ASSERT (_viewer->butler()); - _viewer->_gets++; + add_get (); boost::mutex::scoped_lock lm (_mutex); @@ -67,7 +69,7 @@ VideoView::get_next_frame (bool non_blocking) } while ( _player_video.first && _viewer->film()->three_d() && - _viewer->_eyes != _player_video.first->eyes() && + _eyes != _player_video.first->eyes() && _player_video.first->eyes() != EYES_BOTH ); diff --git a/src/wx/video_view.h b/src/wx/video_view.h index 4a94aa879..fd2684e41 100644 --- a/src/wx/video_view.h +++ b/src/wx/video_view.h @@ -23,6 +23,7 @@ #include "lib/dcpomatic_time.h" #include "lib/timer.h" +#include "lib/types.h" #include #include #include @@ -63,6 +64,11 @@ public: return _dropped; } + int gets () const { + boost::mutex::scoped_lock lm (_mutex); + return _gets; + } + StateTimer const & state_timer () const { return _state_timer; } @@ -82,6 +88,11 @@ public: _length = len; } + void set_eyes (Eyes eyes) { + boost::mutex::scoped_lock lm (_mutex); + _eyes = eyes; + } + protected: /* XXX_b: to remove */ friend class FilmViewer; @@ -110,6 +121,11 @@ protected: ++_dropped; } + void add_get () { + boost::mutex::scoped_lock lm (_mutex); + ++_gets; + } + FilmViewer* _viewer; #ifdef DCPOMATIC_VARIANT_SWAROOP @@ -128,8 +144,10 @@ private: int _video_frame_rate; /** length of the film we are playing, or 0 if there is none */ dcpomatic::DCPTime _length; + Eyes _eyes; int _dropped; + int _gets; }; #endif -- cgit v1.2.3 From f77529bdfa01ae13f889442900988fc401b63c62 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Thu, 21 Nov 2019 23:43:23 +0100 Subject: Various cleanups and thread-safety. --- src/wx/film_viewer.cc | 20 +++++++++++++++++--- src/wx/film_viewer.h | 3 ++- src/wx/gl_video_view.cc | 43 +++++++++++++++++++++++++++---------------- src/wx/gl_video_view.h | 4 ++++ src/wx/video_view.cc | 15 ++++++++++----- src/wx/video_view.h | 10 +++++++++- 6 files changed, 69 insertions(+), 26 deletions(-) (limited to 'src/wx/video_view.cc') diff --git a/src/wx/film_viewer.cc b/src/wx/film_viewer.cc index dcbfedc81..df66a8ade 100644 --- a/src/wx/film_viewer.cc +++ b/src/wx/film_viewer.cc @@ -334,6 +334,8 @@ FilmViewer::stop () _playing = false; _video_view->stop (); Stopped (position()); + + _video_view->rethrow (); return true; } @@ -379,6 +381,8 @@ FilmViewer::film_change (ChangeType type, Film::Property p) recreate_butler (); } else if (p == Film::VIDEO_FRAME_RATE) { _video_view->set_video_frame_rate (_film->video_frame_rate()); + } else if (p == Film::THREE_D) { + _video_view->set_three_d (_film->three_d()); } } @@ -636,11 +640,21 @@ FilmViewer::set_pad_black (bool p) _pad_black = p; } -/* May be called from a non-UI thread */ +/** Called when a player has finished the current film. + * May be called from a non-UI thread. + */ void -FilmViewer::emit_finished () +FilmViewer::finished () { - emit (boost::bind(boost::ref(Finished))); + emit (boost::bind(&FilmViewer::ui_finished, this)); +} + +/** Called by finished() in the UI thread */ +void +FilmViewer::ui_finished () +{ + stop (); + Finished (); } int diff --git a/src/wx/film_viewer.h b/src/wx/film_viewer.h index ec481f34e..60cde60d0 100644 --- a/src/wx/film_viewer.h +++ b/src/wx/film_viewer.h @@ -131,13 +131,13 @@ public: ClosedCaptionsDialog* closed_captions_dialog () const { return _closed_captions_dialog; } + void finished (); 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; @@ -152,6 +152,7 @@ private: void recreate_butler (); void config_changed (Config::Property); void film_length_change (); + void ui_finished (); 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 ce152787a..869d555cb 100644 --- a/src/wx/gl_video_view.cc +++ b/src/wx/gl_video_view.cc @@ -133,8 +133,11 @@ check_gl_error (char const * last) void GLVideoView::update () { - if (!_canvas->IsShownOnScreen()) { - return; + { + boost::mutex::scoped_lock lm (_canvas_mutex); + if (!_canvas->IsShownOnScreen()) { + return; + } } request_one_shot (); } @@ -155,16 +158,22 @@ GLVideoView::draw (Position inter_position, dcp::Size inter_size) check_gl_error ("glDisable GL_DEPTH_TEST"); glBlendFunc (GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA); - if (_canvas->GetSize().x < 64 || _canvas->GetSize().y < 0) { + wxSize canvas_size; + { + boost::mutex::scoped_lock lm (_canvas_mutex); + canvas_size = _canvas->GetSize (); + } + + if (canvas_size.GetWidth() < 64 || canvas_size.GetHeight() < 0) { return; } - glViewport (0, 0, _canvas->GetSize().x, _canvas->GetSize().y); + glViewport (0, 0, canvas_size.GetWidth(), canvas_size.GetHeight()); check_gl_error ("glViewport"); glMatrixMode (GL_PROJECTION); glLoadIdentity (); - gluOrtho2D (0, _canvas->GetSize().x, _canvas->GetSize().y, 0); + gluOrtho2D (0, canvas_size.GetWidth(), canvas_size.GetHeight(), 0); check_gl_error ("gluOrtho2d"); glMatrixMode (GL_MODELVIEW); glLoadIdentity (); @@ -197,8 +206,6 @@ GLVideoView::draw (Position inter_position, dcp::Size inter_size) glEnd (); } - wxSize const canvas_size = _canvas->GetSize (); - if (!_viewer->pad_black() && out_size.width < canvas_size.GetWidth()) { glBegin (GL_QUADS); /* XXX: these colours are right for GNOME; may need adjusting for other OS */ @@ -241,6 +248,8 @@ GLVideoView::draw (Position inter_position, dcp::Size inter_size) } glFlush(); + + boost::mutex::scoped_lock lm (_canvas_mutex); _canvas->SwapBuffers(); } @@ -291,9 +300,11 @@ void GLVideoView::thread () try { - /* XXX_b: check all calls and signal emissions in this method & protect them if necessary */ - _context = new wxGLContext (_canvas); - _canvas->SetCurrent (*_context); + { + boost::mutex::scoped_lock lm (_canvas_mutex); + _context = new wxGLContext (_canvas); //local + _canvas->SetCurrent (*_context); + } while (true) { boost::mutex::scoped_lock lm (_playing_mutex); @@ -309,12 +320,12 @@ try dcpomatic::DCPTime const next = position() + one_video_frame(); if (next >= length()) { - _viewer->stop (); - _viewer->emit_finished (); + _viewer->finished (); continue; } get_next_frame (false); + //-- set_image (player_video().first->image(bind(&PlayerVideo::force, _1, AV_PIX_FMT_RGB24), false, true)); inter_position = player_video().first->inter_position(); inter_size = player_video().first->inter_size(); @@ -330,13 +341,13 @@ try dcpomatic_sleep_milliseconds (time_until_next_frame()); } - delete _context; + /* XXX: leaks _context, but that seems preferable to deleting it here + * without also deleting the wxGLCanvas. + */ } catch (boost::thread_interrupted& e) { - /* XXX_b: store exceptions here */ - delete _context; - return; + store_current (); } bool diff --git a/src/wx/gl_video_view.h b/src/wx/gl_video_view.h index 4f509049b..827b12861 100644 --- a/src/wx/gl_video_view.h +++ b/src/wx/gl_video_view.h @@ -57,6 +57,10 @@ private: void create (); void check_for_butler_errors (); + /* Mutex for use of _canvas; it's only contended when our ::thread + is started up so this may be overkill. + */ + boost::mutex _canvas_mutex; wxGLCanvas* _canvas; wxGLContext* _context; diff --git a/src/wx/video_view.cc b/src/wx/video_view.cc index 5dd857fab..ee14f4a33 100644 --- a/src/wx/video_view.cc +++ b/src/wx/video_view.cc @@ -23,6 +23,8 @@ #include "film_viewer.h" #include "lib/butler.h" +using boost::shared_ptr; + VideoView::VideoView (FilmViewer* viewer) : _viewer (viewer) #ifdef DCPOMATIC_VARIANT_SWAROOP @@ -31,6 +33,7 @@ VideoView::VideoView (FilmViewer* viewer) , _state_timer ("viewer") , _video_frame_rate (0) , _eyes (EYES_LEFT) + , _three_d (false) , _dropped (0) , _gets (0) { @@ -45,30 +48,32 @@ VideoView::clear () _player_video.second = dcpomatic::DCPTime (); } -/** @param non_blocking true to return false quickly if no video is available quickly. +/** Could be called from any thread. + * @param non_blocking true to return false quickly if no video is available quickly. * @return false if we gave up because it would take too long, otherwise true. */ bool VideoView::get_next_frame (bool non_blocking) { - if (_length == dcpomatic::DCPTime()) { + if (length() == dcpomatic::DCPTime()) { return true; } - DCPOMATIC_ASSERT (_viewer->butler()); + shared_ptr butler = _viewer->butler (); + DCPOMATIC_ASSERT (butler); add_get (); boost::mutex::scoped_lock lm (_mutex); do { Butler::Error e; - _player_video = _viewer->butler()->get_video (!non_blocking, &e); + _player_video = butler->get_video (!non_blocking, &e); if (!_player_video.first && e == Butler::AGAIN) { return false; } } while ( _player_video.first && - _viewer->film()->three_d() && + _three_d && _eyes != _player_video.first->eyes() && _player_video.first->eyes() != EYES_BOTH ); diff --git a/src/wx/video_view.h b/src/wx/video_view.h index fd2684e41..5d5d33163 100644 --- a/src/wx/video_view.h +++ b/src/wx/video_view.h @@ -24,16 +24,18 @@ #include "lib/dcpomatic_time.h" #include "lib/timer.h" #include "lib/types.h" +#include "lib/exception_store.h" #include #include #include +#include class Image; class wxWindow; class FilmViewer; class PlayerVideo; -class VideoView +class VideoView : public ExceptionStore, public boost::noncopyable { public: VideoView (FilmViewer* viewer); @@ -93,6 +95,11 @@ public: _eyes = eyes; } + void set_three_d (bool t) { + boost::mutex::scoped_lock lm (_mutex); + _three_d = t; + } + protected: /* XXX_b: to remove */ friend class FilmViewer; @@ -145,6 +152,7 @@ private: /** length of the film we are playing, or 0 if there is none */ dcpomatic::DCPTime _length; Eyes _eyes; + bool _three_d; int _dropped; int _gets; -- cgit v1.2.3 From 166f44ff1b500f684417d660bb349d35383996ee Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Fri, 22 Nov 2019 00:10:35 +0100 Subject: More tidying up. --- src/wx/film_viewer.cc | 11 +---------- src/wx/gl_video_view.cc | 1 - src/wx/simple_video_view.cc | 12 ++++++------ src/wx/simple_video_view.h | 2 +- src/wx/video_view.cc | 17 +++++++++++++++++ src/wx/video_view.h | 28 +++++++++++++++------------- 6 files changed, 40 insertions(+), 31 deletions(-) (limited to 'src/wx/video_view.cc') diff --git a/src/wx/film_viewer.cc b/src/wx/film_viewer.cc index df66a8ade..c7b32ff26 100644 --- a/src/wx/film_viewer.cc +++ b/src/wx/film_viewer.cc @@ -406,16 +406,7 @@ FilmViewer::slow_refresh () bool FilmViewer::quick_refresh () { - if (!_video_view->_player_video.first) { - return false; - } - - if (!_video_view->_player_video.first->reset_metadata (_film, _player->video_container_size(), _film->frame_size())) { - return false; - } - - _video_view->display_player_video (); - return true; + return _video_view->refresh_metadata (_film, _player->video_container_size(), _film->frame_size()); } void diff --git a/src/wx/gl_video_view.cc b/src/wx/gl_video_view.cc index 869d555cb..2da16f1df 100644 --- a/src/wx/gl_video_view.cc +++ b/src/wx/gl_video_view.cc @@ -325,7 +325,6 @@ try } get_next_frame (false); - //-- set_image (player_video().first->image(bind(&PlayerVideo::force, _1, AV_PIX_FMT_RGB24), false, true)); inter_position = player_video().first->inter_position(); inter_size = player_video().first->inter_size(); diff --git a/src/wx/simple_video_view.cc b/src/wx/simple_video_view.cc index 22e4db887..6eabbc0b0 100644 --- a/src/wx/simple_video_view.cc +++ b/src/wx/simple_video_view.cc @@ -134,9 +134,9 @@ SimpleVideoView::paint () } void -SimpleVideoView::update () +SimpleVideoView::refresh_panel () { - _state_timer.set ("update-view"); + _state_timer.set ("refresh-panel"); _panel->Refresh (); _panel->Update (); _state_timer.unset (); @@ -193,7 +193,7 @@ SimpleVideoView::display_next_frame (bool non_blocking) } } - display_player_video (); + update (); try { _viewer->butler()->rethrow (); @@ -205,11 +205,11 @@ SimpleVideoView::display_next_frame (bool non_blocking) } void -SimpleVideoView::display_player_video () +SimpleVideoView::update () { if (!player_video().first) { set_image (shared_ptr()); - update (); + refresh_panel (); return; } @@ -252,5 +252,5 @@ SimpleVideoView::display_player_video () _inter_position = player_video().first->inter_position (); _inter_size = player_video().first->inter_size (); - update (); + refresh_panel (); } diff --git a/src/wx/simple_video_view.h b/src/wx/simple_video_view.h index 86451fa66..a6a5cf47f 100644 --- a/src/wx/simple_video_view.h +++ b/src/wx/simple_video_view.h @@ -43,9 +43,9 @@ public: bool display_next_frame (bool non_blocking); private: + void refresh_panel (); void paint (); void timer (); - void display_player_video (); wxPanel* _panel; boost::shared_ptr _image; diff --git a/src/wx/video_view.cc b/src/wx/video_view.cc index ee14f4a33..7e9e1a947 100644 --- a/src/wx/video_view.cc +++ b/src/wx/video_view.cc @@ -110,3 +110,20 @@ VideoView::start () boost::mutex::scoped_lock lm (_mutex); _dropped = 0; } + +bool +VideoView::refresh_metadata (shared_ptr film, dcp::Size video_container_size, dcp::Size film_frame_size) +{ + boost::mutex::scoped_lock lm (_mutex); + if (!_player_video.first) { + return false; + } + + if (!_player_video.first->reset_metadata (film, video_container_size, film_frame_size)) { + return false; + } + + update (); + return true; +} + diff --git a/src/wx/video_view.h b/src/wx/video_view.h index 5d5d33163..ad492bd43 100644 --- a/src/wx/video_view.h +++ b/src/wx/video_view.h @@ -41,25 +41,25 @@ public: VideoView (FilmViewer* viewer); virtual ~VideoView () {} - virtual void set_image (boost::shared_ptr image) = 0; + /** @return the thing displaying the image */ virtual wxWindow* get () const = 0; - /** Redraw the view after something has changed like content outlining, - * the film being removed, etc. - */ + /** Re-make and display the image from the current _player_video */ virtual void update () = 0; - + /** Called when playback starts */ virtual void start (); - /* XXX_b: make pure */ + /** Called when playback stops */ virtual void stop () {} + /** Get the next frame and display it; used after seek */ + virtual bool display_next_frame (bool) = 0; void clear (); + bool refresh_metadata (boost::shared_ptr film, dcp::Size video_container_size, dcp::Size film_frame_size); + /** Emitted from the GUI thread when our display changes in size */ boost::signals2::signal Sized; - virtual bool display_next_frame (bool) = 0; - /* XXX_b: to remove */ - virtual void display_player_video () {} + /* Accessors for FilmViewer */ int dropped () const { boost::mutex::scoped_lock lm (_mutex); @@ -80,6 +80,11 @@ public: return _player_video.second; } + + /* Setters for FilmViewer so it can tell us our state and + * we can then use (thread) safely. + */ + void set_video_frame_rate (int r) { boost::mutex::scoped_lock lm (_mutex); _video_frame_rate = r; @@ -101,9 +106,6 @@ public: } 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; @@ -144,7 +146,7 @@ protected: StateTimer _state_timer; private: - /** Mutex protecting all the state in VideoView */ + /** Mutex protecting all the state in this class */ mutable boost::mutex _mutex; std::pair, dcpomatic::DCPTime> _player_video; -- cgit v1.2.3 From bdb0887facaf9cb16a7fcfab722cd83171fff8bd Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Fri, 22 Nov 2019 00:37:36 +0100 Subject: Fix some crashes. --- src/wx/film_viewer.cc | 3 +++ src/wx/gl_video_view.cc | 7 ++++--- src/wx/video_view.cc | 4 +++- 3 files changed, 10 insertions(+), 4 deletions(-) (limited to 'src/wx/video_view.cc') diff --git a/src/wx/film_viewer.cc b/src/wx/film_viewer.cc index c7b32ff26..f921f83bd 100644 --- a/src/wx/film_viewer.cc +++ b/src/wx/film_viewer.cc @@ -406,6 +406,9 @@ FilmViewer::slow_refresh () bool FilmViewer::quick_refresh () { + if (!_video_view || !_film) { + return true; + } return _video_view->refresh_metadata (_film, _player->video_container_size(), _film->frame_size()); } diff --git a/src/wx/gl_video_view.cc b/src/wx/gl_video_view.cc index 2da16f1df..bfc611d37 100644 --- a/src/wx/gl_video_view.cc +++ b/src/wx/gl_video_view.cc @@ -325,9 +325,10 @@ try } get_next_frame (false); - set_image (player_video().first->image(bind(&PlayerVideo::force, _1, AV_PIX_FMT_RGB24), false, true)); - inter_position = player_video().first->inter_position(); - inter_size = player_video().first->inter_size(); + shared_ptr pv = player_video().first; + set_image (pv->image(bind(&PlayerVideo::force, _1, AV_PIX_FMT_RGB24), false, true)); + inter_position = pv->inter_position(); + inter_size = pv->inter_size(); } draw (inter_position, inter_size); diff --git a/src/wx/video_view.cc b/src/wx/video_view.cc index 7e9e1a947..7f93f765e 100644 --- a/src/wx/video_view.cc +++ b/src/wx/video_view.cc @@ -60,7 +60,9 @@ VideoView::get_next_frame (bool non_blocking) } shared_ptr butler = _viewer->butler (); - DCPOMATIC_ASSERT (butler); + if (!butler) { + return false; + } add_get (); boost::mutex::scoped_lock lm (_mutex); -- cgit v1.2.3 From 89e92b3e7effafd2ca3aa1e9300777f2d2fb6183 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Sat, 23 Nov 2019 10:42:16 +0100 Subject: Don't busy-wait when there's nothing to play. --- src/wx/gl_video_view.cc | 8 ++++++-- src/wx/simple_video_view.cc | 2 +- src/wx/video_view.cc | 8 +++++--- src/wx/video_view.h | 2 +- 4 files changed, 13 insertions(+), 7 deletions(-) (limited to 'src/wx/video_view.cc') diff --git a/src/wx/gl_video_view.cc b/src/wx/gl_video_view.cc index ddae9bb3c..a461939a7 100644 --- a/src/wx/gl_video_view.cc +++ b/src/wx/gl_video_view.cc @@ -334,13 +334,17 @@ try } draw (inter_position, inter_size); - while (time_until_next_frame() < 5) { + while (true) { + optional n = time_until_next_frame(); + if (!n || *n > 5) { + break; + } get_next_frame (true); add_dropped (); } boost::this_thread::interruption_point (); - dcpomatic_sleep_milliseconds (time_until_next_frame()); + dcpomatic_sleep_milliseconds (time_until_next_frame().get_value_or(0)); } /* XXX: leaks _context, but that seems preferable to deleting it here diff --git a/src/wx/simple_video_view.cc b/src/wx/simple_video_view.cc index 6eabbc0b0..e349b865c 100644 --- a/src/wx/simple_video_view.cc +++ b/src/wx/simple_video_view.cc @@ -159,7 +159,7 @@ SimpleVideoView::timer () } LOG_DEBUG_PLAYER("%1 -> %2; delay %3", next.seconds(), _viewer->time().seconds(), max((next.seconds() - _viewer->time().seconds()) * 1000, 1.0)); - _timer.Start (time_until_next_frame(), wxTIMER_ONE_SHOT); + _timer.Start (time_until_next_frame().get_value_or(0), wxTIMER_ONE_SHOT); if (_viewer->butler()) { _viewer->butler()->rethrow (); diff --git a/src/wx/video_view.cc b/src/wx/video_view.cc index 7f93f765e..4edc2cd23 100644 --- a/src/wx/video_view.cc +++ b/src/wx/video_view.cc @@ -22,8 +22,10 @@ #include "wx_util.h" #include "film_viewer.h" #include "lib/butler.h" +#include using boost::shared_ptr; +using boost::optional; VideoView::VideoView (FilmViewer* viewer) : _viewer (viewer) @@ -89,13 +91,13 @@ VideoView::one_video_frame () const return dcpomatic::DCPTime::from_frames (1, video_frame_rate()); } -/** @return Time in ms until the next frame is due */ -int +/** @return Time in ms until the next frame is due, or empty if nothing is due */ +optional VideoView::time_until_next_frame () const { if (length() == dcpomatic::DCPTime()) { /* There's no content, so this doesn't matter */ - return 0; + return optional(); } dcpomatic::DCPTime const next = position() + one_video_frame(); diff --git a/src/wx/video_view.h b/src/wx/video_view.h index ad492bd43..f9e067043 100644 --- a/src/wx/video_view.h +++ b/src/wx/video_view.h @@ -107,7 +107,7 @@ public: protected: bool get_next_frame (bool non_blocking); - int time_until_next_frame () const; + boost::optional time_until_next_frame () const; dcpomatic::DCPTime one_video_frame () const; int video_frame_rate () const { -- cgit v1.2.3