Try to tidy up subtitle timing and seeks wrt source frames, DCP frames and rounding.

9 files changed, 67 insertions, 29 deletions

diff --git a/src/lib/decoder.cc b/src/lib/decoder.cc
index 7d4085045..61e63460b 100644
--- a/src/lib/decoder.cc
+++ b/src/lib/decoder.cc
@@ -57,8 +57,21 @@ Decoder::Decoder (boost::shared_ptr<Film> f, boost::shared_ptr<const DecodeOptio
 	_film_connection = f->Changed.connect (bind (&Decoder::film_changed, this, _1));
 }
 
+/** Seek.
+ *  @param p Position as a source timestamp in seconds.
+ *  @return true on error.
+ */
+bool
+Decoder::seek (double p)
+{
+	throw DecodeError ("decoder does not support seek");
+}
+
+/** Seek so that the next frame we will produce is the same as the last one.
+ *  @return true on error.
+ */
 bool
-Decoder::seek (SourceFrame f)
+Decoder::seek_to_last ()
 {
 	throw DecodeError ("decoder does not support seek");
 }
diff --git a/src/lib/decoder.h b/src/lib/decoder.h
index b8278ff80..3908afa2f 100644
--- a/src/lib/decoder.h
+++ b/src/lib/decoder.h
@@ -58,10 +58,8 @@ public:
 	virtual ~Decoder () {}
 
 	virtual bool pass () = 0;
-	/** Seek.
-	 *  @return true on error.
-	 */
-	virtual bool seek (SourceFrame);
+	virtual bool seek (double);
+	virtual bool seek_to_last ();
 
 	boost::signals2::signal<void()> OutputChanged;
 
diff --git a/src/lib/ffmpeg_decoder.cc b/src/lib/ffmpeg_decoder.cc
index f81f00b19..97d43c76c 100644
--- a/src/lib/ffmpeg_decoder.cc
+++ b/src/lib/ffmpeg_decoder.cc
@@ -543,24 +543,35 @@ FFmpegDecoder::filter_and_emit_video (AVFrame* frame)
 
 	list<shared_ptr<Image> > images = graph->process (frame);
 
-	SourceFrame const sf = av_q2d (_format_context->streams[_video_stream]->time_base)
-		* av_frame_get_best_effort_timestamp(_frame) * frames_per_second();
+	double const st = av_frame_get_best_effort_timestamp(_frame) * av_q2d (_format_context->streams[_video_stream]->time_base);
 
 	for (list<shared_ptr<Image> >::iterator i = images.begin(); i != images.end(); ++i) {
-		emit_video (*i, sf);
+		emit_video (*i, st);
 	}
 }
 
 bool
-FFmpegDecoder::seek (SourceFrame f)
+FFmpegDecoder::seek (double p)
 {
-	int64_t const vt = static_cast<int64_t>(f) / (av_q2d (_format_context->streams[_video_stream]->time_base) * frames_per_second());
+	return do_seek (p, false);
+}
 
-	/* This AVSEEK_FLAG_BACKWARD is a bit of a hack; without it, if we ask for a seek to the same place as last time
+bool
+FFmpegDecoder::seek_to_last ()
+{
+	/* This AVSEEK_FLAG_BACKWARD in do_seek is a bit of a hack; without it, if we ask for a seek to the same place as last time
 	   (used when we change decoder parameters and want to re-fetch the frame) we end up going forwards rather than
 	   staying in the same place.
 	*/
-	int const r = av_seek_frame (_format_context, _video_stream, vt, (f == last_source_frame() ? AVSEEK_FLAG_BACKWARD : 0));
+	return do_seek (last_source_time(), true);
+}
+
+bool
+FFmpegDecoder::do_seek (double p, bool backwards)
+{
+	int64_t const vt = p / av_q2d (_format_context->streams[_video_stream]->time_base);
+
+	int const r = av_seek_frame (_format_context, _video_stream, vt, backwards ? AVSEEK_FLAG_BACKWARD : 0);
 	
 	avcodec_flush_buffers (_video_codec_context);
 	if (_subtitle_codec_context) {
diff --git a/src/lib/ffmpeg_decoder.h b/src/lib/ffmpeg_decoder.h
index 2011ef72f..89534a38c 100644
--- a/src/lib/ffmpeg_decoder.h
+++ b/src/lib/ffmpeg_decoder.h
@@ -100,11 +100,13 @@ public:
 	void set_audio_stream (boost::shared_ptr<AudioStream>);
 	void set_subtitle_stream (boost::shared_ptr<SubtitleStream>);
 
-	bool seek (SourceFrame);
+	bool seek (double);
+	bool seek_to_last ();
 
 private:
 
 	bool pass ();
+	bool do_seek (double p, bool);
 	PixelFormat pixel_format () const;
 	AVSampleFormat audio_sample_format () const;
 	int bytes_per_audio_sample () const;
diff --git a/src/lib/imagemagick_decoder.cc b/src/lib/imagemagick_decoder.cc
index 5713e68f9..131eaa500 100644
--- a/src/lib/imagemagick_decoder.cc
+++ b/src/lib/imagemagick_decoder.cc
@@ -108,8 +108,22 @@ ImageMagickDecoder::pixel_format () const
 }
 
 bool
-ImageMagickDecoder::seek (SourceFrame f)
+ImageMagickDecoder::seek_to_last ()
 {
+	if (_iter == _files.end()) {
+		_iter = _files.begin();
+	} else {
+		--_iter;
+	}
+
+	return false;
+}
+
+bool
+ImageMagickDecoder::seek (double t)
+{
+	int const f = t * frames_per_second();
+	
 	_iter = _files.begin ();
 	for (int i = 0; i < f; ++i) {
 		if (_iter == _files.end()) {
diff --git a/src/lib/imagemagick_decoder.h b/src/lib/imagemagick_decoder.h
index cf417d373..6f426f308 100644
--- a/src/lib/imagemagick_decoder.h
+++ b/src/lib/imagemagick_decoder.h
@@ -56,7 +56,8 @@ public:
 		return false;
 	}
 
-	bool seek (SourceFrame);
+	bool seek (double);
+	bool seek_to_last ();
 
 protected:
 	bool pass ();
diff --git a/src/lib/subtitle.cc b/src/lib/subtitle.cc
index 1af277255..182e30ed3 100644
--- a/src/lib/subtitle.cc
+++ b/src/lib/subtitle.cc
@@ -39,7 +39,7 @@ TimedSubtitle::TimedSubtitle (AVSubtitle const & sub)
 	/* Subtitle PTS in seconds (within the source, not taking into account any of the
 	   source that we may have chopped off for the DCP)
 	*/
-	double const packet_time = ((sub.pts / AV_TIME_BASE) + float (sub.pts % AV_TIME_BASE) / 1e6);
+	double const packet_time = static_cast<double> (sub.pts) / AV_TIME_BASE;
 	
 	/* hence start time for this sub */
 	_from = packet_time + (double (sub.start_display_time) / 1e3);
@@ -77,7 +77,7 @@ TimedSubtitle::TimedSubtitle (AVSubtitle const & sub)
 	_subtitle.reset (new Subtitle (Position (rect->x, rect->y), image));
 }	
 
-/** @param t Time in seconds from the start of the film */
+/** @param t Time in seconds from the start of the source */
 bool
 TimedSubtitle::displayed_at (double t) const
 {
diff --git a/src/lib/video_decoder.cc b/src/lib/video_decoder.cc
index f4501bbf3..e723610b3 100644
--- a/src/lib/video_decoder.cc
+++ b/src/lib/video_decoder.cc
@@ -31,7 +31,7 @@ using boost::optional;
 VideoDecoder::VideoDecoder (shared_ptr<Film> f, shared_ptr<const DecodeOptions> o, Job* j)
 	: Decoder (f, o, j)
 	, _video_frame (0)
-	, _last_source_frame (0)
+	, _last_source_time (0)
 {
 
 }
@@ -39,19 +39,18 @@ VideoDecoder::VideoDecoder (shared_ptr<Film> f, shared_ptr<const DecodeOptions>
 /** Called by subclasses to tell the world that some video data is ready.
  *  We find a subtitle then emit it for listeners.
  *  @param image frame to emit.
- *  @param f Frame within the source.
+ *  @param t Time of the frame within the source, in seconds.
  */
 void
-VideoDecoder::emit_video (shared_ptr<Image> image, SourceFrame f)
+VideoDecoder::emit_video (shared_ptr<Image> image, double t)
 {
 	shared_ptr<Subtitle> sub;
-	if (_timed_subtitle && _timed_subtitle->displayed_at (f / _film->frames_per_second())) {
-		_film->log()->log (String::compose ("putting subtitle using %1 instead of %2", f, video_frame()));
+	if (_timed_subtitle && _timed_subtitle->displayed_at (t)) {
 		sub = _timed_subtitle->subtitle ();
 	}
 
 	signal_video (image, sub);
-	_last_source_frame = f;
+	_last_source_time = t;
 }
 
 void
diff --git a/src/lib/video_decoder.h b/src/lib/video_decoder.h
index f682941d1..97bbb0e48 100644
--- a/src/lib/video_decoder.h
+++ b/src/lib/video_decoder.h
@@ -45,7 +45,7 @@ public:
 
 	void set_progress () const;
 	
-	SourceFrame video_frame () const {
+	int video_frame () const {
 		return _video_frame;
 	}
 
@@ -57,15 +57,15 @@ public:
 		return _subtitle_streams;
 	}
 
-	SourceFrame last_source_frame () const {
-		return _last_source_frame;
+	double last_source_time () const {
+		return _last_source_time;
 	}
 
 protected:
 	
 	virtual PixelFormat pixel_format () const = 0;
 
-	void emit_video (boost::shared_ptr<Image>, SourceFrame);
+	void emit_video (boost::shared_ptr<Image>, double);
 	void emit_subtitle (boost::shared_ptr<TimedSubtitle>);
 	void repeat_last_video ();
 
@@ -77,8 +77,8 @@ protected:
 private:
 	void signal_video (boost::shared_ptr<Image>, boost::shared_ptr<Subtitle>);
 
-	SourceFrame _video_frame;
-	SourceFrame _last_source_frame;
+	int _video_frame;
+	double _last_source_time;
 	
 	boost::shared_ptr<TimedSubtitle> _timed_subtitle;