A little thread safety.

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 = 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)));
 }
+
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 <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;
 
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
@@ -268,6 +268,31 @@ GLVideoView::start ()
 }
 
 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 <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;
 };
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 <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