Various fixes; simplification of FilmViewer; make image appear on first load of content.

8 files changed, 100 insertions, 154 deletions

diff --git a/src/lib/ffmpeg_decoder.cc b/src/lib/ffmpeg_decoder.cc
index bf0949130..3b7d727b8 100644
--- a/src/lib/ffmpeg_decoder.cc
+++ b/src/lib/ffmpeg_decoder.cc
@@ -67,6 +67,7 @@ FFmpegDecoder::FFmpegDecoder (shared_ptr<const Film> f, shared_ptr<const FFmpegC
 	, _decode_video (video)
 	, _decode_audio (audio)
 	, _pts_offset (0)
+	, _just_sought (false)
 {
 	setup_subtitle ();
 
@@ -298,15 +299,16 @@ FFmpegDecoder::seek_back ()
 void
 FFmpegDecoder::do_seek (VideoContent::Frame frame, bool backwards, bool accurate)
 {
-	int64_t const vt = frame * _ffmpeg_content->video_frame_rate() / av_q2d (_format_context->streams[_video_stream]->time_base);
+	int64_t const vt = frame / (_ffmpeg_content->video_frame_rate() * av_q2d (_format_context->streams[_video_stream]->time_base));
 	av_seek_frame (_format_context, _video_stream, vt, backwards ? AVSEEK_FLAG_BACKWARD : 0);
-	_video_position = frame;
 
 	avcodec_flush_buffers (video_codec_context());
 	if (_subtitle_codec_context) {
 		avcodec_flush_buffers (_subtitle_codec_context);
 	}
 
+	_just_sought = true;
+
 	if (accurate) {
 		while (1) {
 			int r = av_read_frame (_format_context, &_packet);
@@ -420,6 +422,15 @@ FFmpegDecoder::decode_video_packet ()
 		if (bet != AV_NOPTS_VALUE) {
 
 			double const pts = bet * av_q2d (_format_context->streams[_video_stream]->time_base) - _pts_offset;
+
+			if (_just_sought) {
+				/* We just did a seek, so disable any attempts to correct for where we
+				   are / should be.
+				*/
+				_video_position = pts * _ffmpeg_content->video_frame_rate ();
+				_just_sought = false;
+			}
+			
 			double const next = _video_position / _ffmpeg_content->video_frame_rate();
 			double const one_frame = 1 / _ffmpeg_content->video_frame_rate ();
 			double delta = pts - next;
@@ -445,6 +456,7 @@ FFmpegDecoder::decode_video_packet ()
 				/* This PTS is within a frame of being right; emit this (otherwise it will be dropped) */
 				video (image, false, _video_position);
 			}
+				
 		} else {
 			shared_ptr<const Film> film = _film.lock ();
 			assert (film);
diff --git a/src/lib/ffmpeg_decoder.h b/src/lib/ffmpeg_decoder.h
index 8f0482aad..2e64d8801 100644
--- a/src/lib/ffmpeg_decoder.h
+++ b/src/lib/ffmpeg_decoder.h
@@ -85,4 +85,5 @@ private:
 	bool _decode_audio;
 
 	double _pts_offset;
+	bool _just_sought;
 };
diff --git a/src/lib/film.cc b/src/lib/film.cc
index fa75ab1f1..7320676d7 100644
--- a/src/lib/film.cc
+++ b/src/lib/film.cc
@@ -128,6 +128,7 @@ Film::Film (string d)
 	}
 
 	set_directory (result.string ());
+	_log.reset (new FileLog ("log"));
 }
 
 Film::Film (Film const & o)
@@ -793,6 +794,7 @@ Film::add_content (shared_ptr<Content> c)
 		c->set_start (_playlist->video_end ());
 	}
 
+	cout << "actually adding content.\n";
 	_playlist->add (c);
 }
 
diff --git a/src/lib/image.cc b/src/lib/image.cc
index 722ff5d3c..0eabbe84d 100644
--- a/src/lib/image.cc
+++ b/src/lib/image.cc
@@ -45,6 +45,7 @@ extern "C" {
 
 using std::string;
 using std::min;
+using std::cout;
 using boost::shared_ptr;
 using libdcp::Size;
 
diff --git a/src/lib/player.cc b/src/lib/player.cc
index 9969fbf9e..7488364bd 100644
--- a/src/lib/player.cc
+++ b/src/lib/player.cc
@@ -96,6 +96,7 @@ Player::Player (shared_ptr<const Film> f, shared_ptr<const Playlist> p)
 {
 	_playlist->Changed.connect (bind (&Player::playlist_changed, this));
 	_playlist->ContentChanged.connect (bind (&Player::content_changed, this, _1, _2));
+	_film->Changed.connect (bind (&Player::film_changed, this, _1));
 	set_video_container_size (_film->container()->size (_film->full_frame ()));
 }
 
@@ -120,7 +121,7 @@ Player::pass ()
 	}
 
 #ifdef DEBUG_PLAYER
-	cout << "= PASS\n";
+	cout << "= PASS " << this << "\n";
 #endif	
 
         Time earliest_t = TIME_MAX;
@@ -381,7 +382,7 @@ Player::setup_pieces ()
 
 	Playlist::ContentList content = _playlist->content ();
 	sort (content.begin(), content.end(), ContentSorter ());
-	
+
 	for (Playlist::ContentList::iterator i = content.begin(); i != content.end(); ++i) {
 
 		shared_ptr<Piece> piece (new Piece (*i));
@@ -445,8 +446,13 @@ Player::content_changed (weak_ptr<Content> w, int p)
 		return;
 	}
 
-	if (p == ContentProperty::START || p == ContentProperty::LENGTH) {
+	if (
+		p == ContentProperty::START || p == ContentProperty::LENGTH ||
+		p == VideoContentProperty::VIDEO_CROP || p == VideoContentProperty::VIDEO_RATIO
+		) {
+		
 		_have_valid_pieces = false;
+		Changed ();
 	}
 }
 
@@ -454,6 +460,7 @@ void
 Player::playlist_changed ()
 {
 	_have_valid_pieces = false;
+	Changed ();
 }
 
 void
@@ -495,5 +502,20 @@ Player::emit_silence (OutputAudioFrame most)
 	_audio_position += _film->audio_frames_to_time (N);
 }
 
-	
-	
+void
+Player::film_changed (Film::Property p)
+{
+	/* Here we should notice Film properties that affect our output, and
+	   alert listeners that our output now would be different to how it was
+	   last time we were run.
+	*/
+
+	if (
+		p == Film::SCALER || p == Film::WITH_SUBTITLES ||
+		p == Film::SUBTITLE_SCALE || p == Film::SUBTITLE_OFFSET ||
+		p == Film::CONTAINER
+		) {
+		
+		Changed ();
+	}
+}
diff --git a/src/lib/player.h b/src/lib/player.h
index bbdb14ec2..3f8d59d29 100644
--- a/src/lib/player.h
+++ b/src/lib/player.h
@@ -26,6 +26,7 @@
 #include "playlist.h"
 #include "audio_buffers.h"
 #include "content.h"
+#include "film.h"
 
 class Job;
 class Film;
@@ -67,6 +68,12 @@ public:
 	/** Emitted when some audio data is ready */
 	boost::signals2::signal<void (boost::shared_ptr<const AudioBuffers>, Time)> Audio;
 
+	/** Emitted when something has changed such that if we went back and emitted
+	 *  the last frame again it would look different.  This is not emitted after
+	 *  a seek.
+	 */
+	boost::signals2::signal<void ()> Changed;
+
 private:
 
 	void process_video (boost::weak_ptr<Piece>, boost::shared_ptr<const Image>, bool, VideoContent::Frame);
@@ -79,6 +86,7 @@ private:
 	void emit_black ();
 	void emit_silence (OutputAudioFrame);
 	boost::shared_ptr<Resampler> resampler (boost::shared_ptr<AudioContent>);
+	void film_changed (Film::Property);
 
 	boost::shared_ptr<const Film> _film;
 	boost::shared_ptr<const Playlist> _playlist;
diff --git a/src/wx/film_viewer.cc b/src/wx/film_viewer.cc
index 8ef64d509..6469d0962 100644
--- a/src/wx/film_viewer.cc
+++ b/src/wx/film_viewer.cc
@@ -58,7 +58,7 @@ FilmViewer::FilmViewer (shared_ptr<Film> f, wxWindow* p)
 	, _slider (new wxSlider (this, wxID_ANY, 0, 0, 4096))
 	, _back_button (new wxButton (this, wxID_ANY, wxT("<")))
 	, _forward_button (new wxButton (this, wxID_ANY, wxT(">")))
-	, _frame (new wxStaticText (this, wxID_ANY, wxT("")))
+	, _frame_number (new wxStaticText (this, wxID_ANY, wxT("")))
 	, _timecode (new wxStaticText (this, wxID_ANY, wxT("")))
 	, _play_button (new wxToggleButton (this, wxID_ANY, _("Play")))
 	, _got_frame (false)
@@ -79,7 +79,7 @@ FilmViewer::FilmViewer (shared_ptr<Film> f, wxWindow* p)
 	wxBoxSizer* h_sizer = new wxBoxSizer (wxHORIZONTAL);
 
 	wxBoxSizer* time_sizer = new wxBoxSizer (wxVERTICAL);
-	time_sizer->Add (_frame, 0, wxEXPAND);
+	time_sizer->Add (_frame_number, 0, wxEXPAND);
 	time_sizer->Add (_timecode, 0, wxEXPAND);
 
 	h_sizer->Add (_back_button, 0, wxALL, 2);
@@ -90,7 +90,7 @@ FilmViewer::FilmViewer (shared_ptr<Film> f, wxWindow* p)
 
 	_v_sizer->Add (h_sizer, 0, wxEXPAND | wxALL, 6);
 
-	_frame->SetMinSize (wxSize (84, -1));
+	_frame_number->SetMinSize (wxSize (84, -1));
 	_back_button->SetMinSize (wxSize (32, -1));
 	_forward_button->SetMinSize (wxSize (32, -1));
 
@@ -112,37 +112,6 @@ FilmViewer::FilmViewer (shared_ptr<Film> f, wxWindow* p)
 }
 
 void
-FilmViewer::film_changed (Film::Property p)
-{
-	switch (p) {
-	case Film::CONTAINER:
-		calculate_sizes ();
-		update_from_raw ();
-		break;
-	case Film::CONTENT:
-	{
-		calculate_sizes ();
-		wxScrollEvent ev;
-		slider_moved (ev);
-		break;
-	}
-	case Film::WITH_SUBTITLES:
-	case Film::SUBTITLE_OFFSET:
-	case Film::SUBTITLE_SCALE:
-		update_from_decoder ();
-		raw_to_display ();
-		_panel->Refresh ();
-		_panel->Update ();
-		break;
-	case Film::SCALER:
-		update_from_decoder ();
-		break;
-	default:
-		break;
-	}
-}
-
-void
 FilmViewer::set_film (shared_ptr<Film> f)
 {
 	if (_film == f) {
@@ -151,10 +120,7 @@ FilmViewer::set_film (shared_ptr<Film> f)
 
 	_film = f;
 
-	_raw_frame.reset ();
-	_display_frame.reset ();
-	_panel->Refresh ();
-	_panel->Update ();
+	_frame.reset ();
 
 	if (!_film) {
 		return;
@@ -162,29 +128,22 @@ FilmViewer::set_film (shared_ptr<Film> f)
 
 	_player = f->player ();
 	_player->disable_audio ();
-	_player->Video.connect (bind (&FilmViewer::process_video, this, _1, _2, _3));
-	
-	_film->Changed.connect (boost::bind (&FilmViewer::film_changed, this, _1));
-	_film->ContentChanged.connect (boost::bind (&FilmViewer::film_content_changed, this, _1, _2));
-
-	film_changed (Film::CONTENT);
-	film_changed (Film::CONTAINER);
-	film_changed (Film::WITH_SUBTITLES);
-	film_changed (Film::SUBTITLE_OFFSET);
-	film_changed (Film::SUBTITLE_SCALE);
+	_player->Video.connect (boost::bind (&FilmViewer::process_video, this, _1, _2, _3));
+	_player->Changed.connect (boost::bind (&FilmViewer::player_changed, this));
+
+	fetch_current_frame_again ();
 }
 
 void
-FilmViewer::update_from_decoder ()
+FilmViewer::fetch_current_frame_again ()
 {
 	if (!_player) {
 		return;
 	}
 
+	/* This will cause a Player::Changed to be emitted */
 	_player->seek (_player->video_position() - _film->video_frames_to_time (1));
-	get_frame ();
-	_panel->Refresh ();
-	_panel->Update ();
+	fetch_next_frame ();
 }
 
 void
@@ -194,7 +153,7 @@ FilmViewer::timer (wxTimerEvent &)
 		return;
 	}
 	
-	get_frame ();
+	fetch_next_frame ();
 
 	if (_film->length()) {
 		int const new_slider_position = 4096 * _player->video_position() / _film->length();
@@ -202,9 +161,6 @@ FilmViewer::timer (wxTimerEvent &)
 			_slider->SetValue (new_slider_position);
 		}
 	}
-
-	_panel->Refresh ();
-	_panel->Update ();
 }
 
 
@@ -213,12 +169,14 @@ FilmViewer::paint_panel (wxPaintEvent &)
 {
 	wxPaintDC dc (_panel);
 
-	if (!_display_frame || !_film || !_out_size.width || !_out_size.height) {
+	if (!_frame || !_film || !_out_size.width || !_out_size.height) {
 		dc.Clear ();
 		return;
 	}
 
-	wxImage frame (_out_size.width, _out_size.height, _display_frame->data()[0], true);
+	shared_ptr<SimpleImage> packed_frame (new SimpleImage (_frame, false));
+
+	wxImage frame (_out_size.width, _out_size.height, packed_frame->data()[0], true);
 	wxBitmap frame_bitmap (frame);
 	dc.DrawBitmap (frame_bitmap, 0, 0);
 
@@ -243,15 +201,10 @@ FilmViewer::paint_panel (wxPaintEvent &)
 void
 FilmViewer::slider_moved (wxScrollEvent &)
 {
-	cout << "slider " << _slider->GetValue() << " " << _film->length() << "\n";
-	
 	if (_film && _player) {
 		_player->seek (_slider->GetValue() * _film->length() / 4096);
+		fetch_next_frame ();
 	}
-	
-	get_frame ();
-	_panel->Refresh ();
-	_panel->Update ();
 }
 
 void
@@ -259,35 +212,10 @@ FilmViewer::panel_sized (wxSizeEvent& ev)
 {
 	_panel_size.width = ev.GetSize().GetWidth();
 	_panel_size.height = ev.GetSize().GetHeight();
-	calculate_sizes ();
-	update_from_raw ();
+	fetch_current_frame_again ();
 }
 
 void
-FilmViewer::update_from_raw ()
-{
-	if (!_raw_frame) {
-		return;
-	}
-
-	raw_to_display ();
-	
-	_panel->Refresh ();
-	_panel->Update ();
-}
-
-void
-FilmViewer::raw_to_display ()
-{
-	if (!_raw_frame || _out_size.width < 64 || _out_size.height < 64 || !_film) {
-		return;
-	}
-
-	/* Get a compacted image as we have to feed it to wxWidgets */
-	_display_frame.reset (new SimpleImage (_raw_frame, false));
-}	
-
-void
 FilmViewer::calculate_sizes ()
 {
 	if (!_film || !_player) {
@@ -314,7 +242,6 @@ FilmViewer::calculate_sizes ()
 	_out_size.height = max (64, _out_size.height);
 
 	_player->set_video_container_size (_out_size);
-	update_from_decoder ();
 }
 
 void
@@ -340,15 +267,13 @@ FilmViewer::check_play_state ()
 void
 FilmViewer::process_video (shared_ptr<const Image> image, bool, Time t)
 {
-	_raw_frame = image;
-
-	raw_to_display ();
+	_frame = image;
 
 	_got_frame = true;
 
 	double const fps = _film->dcp_video_frame_rate ();
 	/* Count frame number from 1 ... not sure if this is the best idea */
-	_frame->SetLabel (wxString::Format (wxT("%d"), int (rint (t * fps / TIME_HZ)) + 1));
+	_frame_number->SetLabel (wxString::Format (wxT("%d"), int (rint (t * fps / TIME_HZ)) + 1));
 
 	double w = static_cast<double>(t) / TIME_HZ;
 	int const h = (w / 3600);
@@ -361,36 +286,28 @@ FilmViewer::process_video (shared_ptr<const Image> image, bool, Time t)
 	_timecode->SetLabel (wxString::Format (wxT("%02d:%02d:%02d:%02d"), h, m, s, f));
 }
 
-/** Get a new _raw_frame from the decoder and then do
- *  raw_to_display ().
- */
+/** Ask the player to emit its next frame, then update our display */
 void
-FilmViewer::get_frame ()
+FilmViewer::fetch_next_frame ()
 {
-	/* Clear our raw frame in case we don't get a new one */
-	_raw_frame.reset ();
+	/* Clear our frame in case we don't get a new one */
+	_frame.reset ();
 
 	if (!_player) {
-		_display_frame.reset ();
 		return;
 	}
 
 	try {
 		_got_frame = false;
-		while (!_got_frame) {
-			if (_player->pass ()) {
-				/* We didn't get a frame before the decoder gave up,
-				   so clear our display frame.
-				*/
-				_display_frame.reset ();
-				break;
-			}
-		}
+		while (!_got_frame && !_player->pass ());
 	} catch (DecodeError& e) {
 		_play_button->SetValue (false);
 		check_play_state ();
 		error_dialog (this, wxString::Format (_("Could not decode video for view (%s)"), std_to_wx(e.what()).data()));
 	}
+
+	_panel->Refresh ();
+	_panel->Update ();
 }
 
 void
@@ -414,18 +331,6 @@ FilmViewer::active_jobs_changed (bool a)
 }
 
 void
-FilmViewer::film_content_changed (weak_ptr<Content>, int p)
-{
-	if (p == ContentProperty::LENGTH) {
-		/* Force an update to our frame */
-		wxScrollEvent ev;
-		slider_moved (ev);
-	} else if (p == VideoContentProperty::VIDEO_CROP || p == VideoContentProperty::VIDEO_RATIO) {
-		update_from_decoder ();
-	}		
-}
-
-void
 FilmViewer::back_clicked (wxCommandEvent &)
 {
 	if (!_player) {
@@ -433,9 +338,7 @@ FilmViewer::back_clicked (wxCommandEvent &)
 	}
 	
 	_player->seek_back ();
-	get_frame ();
-	_panel->Refresh ();
-	_panel->Update ();
+	fetch_next_frame ();
 }
 
 void
@@ -445,7 +348,12 @@ FilmViewer::forward_clicked (wxCommandEvent &)
 		return;
 	}
 
-	get_frame ();
-	_panel->Refresh ();
-	_panel->Update ();
+	fetch_next_frame ();
+}
+
+void
+FilmViewer::player_changed ()
+{
+	calculate_sizes ();
+	fetch_current_frame_again ();
 }
diff --git a/src/wx/film_viewer.h b/src/wx/film_viewer.h
index 6c18c7c5b..7587911e8 100644
--- a/src/wx/film_viewer.h
+++ b/src/wx/film_viewer.h
@@ -34,20 +34,16 @@ class RGBPlusAlphaImage;
  *
  *  The film takes the following path through the viewer:
  *
- *  1.  get_frame() asks our _player to decode some data.  If it does, process_video()
+ *  1.  fetch_next_frame() asks our _player to decode some data.  If it does, process_video()
  *      will be called.
  *
- *  2.  process_video() takes the image from the decoder (_raw_frame) and calls raw_to_display().
- * 
- *  3.  raw_to_display() copies _raw_frame to _display_frame, processing it and scaling it.
+ *  2.  process_video() takes the image from the player (_frame).
  *
- *  4.  calling _panel->Refresh() and _panel->Update() results in paint_panel() being called;
- *      this creates frame_bitmap from _display_frame and blits it to the display.
+ *  3.  fetch_next_frame() calls _panel->Refresh() and _panel->Update() which results in
+ *      paint_panel() being called; this creates frame_bitmap from _frame and blits it to the display.
  *
- * update_from_decoder() asks the player to re-emit its current frame on the next pass(), and then
+ * fetch_current_frame_again() asks the player to re-emit its current frame on the next pass(), and then
  * starts from step #1.
- *
- * update_from_raw() starts at step #3, then calls _panel->Refresh and _panel->Update.
  */
 class FilmViewer : public wxPanel
 {
@@ -57,8 +53,6 @@ public:
 	void set_film (boost::shared_ptr<Film>);
 
 private:
-	void film_changed (Film::Property);
-	void film_content_changed (boost::weak_ptr<Content>, int);
 	void paint_panel (wxPaintEvent &);
 	void panel_sized (wxSizeEvent &);
 	void slider_moved (wxScrollEvent &);
@@ -67,13 +61,12 @@ private:
 	void process_video (boost::shared_ptr<const Image>, bool, Time);
 	void calculate_sizes ();
 	void check_play_state ();
-	void update_from_raw ();
-	void update_from_decoder ();
-	void raw_to_display ();
-	void get_frame ();
+	void fetch_current_frame_again ();
+	void fetch_next_frame ();
 	void active_jobs_changed (bool);
 	void back_clicked (wxCommandEvent &);
 	void forward_clicked (wxCommandEvent &);
+	void player_changed ();
 
 	boost::shared_ptr<Film> _film;
 	boost::shared_ptr<Player> _player;
@@ -83,13 +76,12 @@ private:
 	wxSlider* _slider;
 	wxButton* _back_button;
 	wxButton* _forward_button;
-	wxStaticText* _frame;
+	wxStaticText* _frame_number;
 	wxStaticText* _timecode;
 	wxToggleButton* _play_button;
 	wxTimer _timer;
 
-	boost::shared_ptr<const Image> _raw_frame;
-	boost::shared_ptr<const Image> _display_frame;
+	boost::shared_ptr<const Image> _frame;
 	bool _got_frame;
 
 	/** Size of our output (including padding if we have any) */