From: Carl Hetherington Date: Tue, 18 Dec 2012 21:13:10 +0000 (+0000) Subject: Try to tidy up subtitle timing and seeks wrt source frames, DCP frames and rounding. X-Git-Tag: v2.0.48~1394^2 X-Git-Url: https://git.carlh.net/gitweb/?a=commitdiff_plain;h=0330d9b2924767d9240c5a25e9ed4327eb0a73bd;p=dcpomatic.git Try to tidy up subtitle timing and seeks wrt source frames, DCP frames and rounding. --- 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 f, boost::shared_ptrChanged.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 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 > 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 >::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(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); void set_subtitle_stream (boost::shared_ptr); - 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 (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 f, shared_ptr 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 f, shared_ptr /** 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, SourceFrame f) +VideoDecoder::emit_video (shared_ptr image, double t) { shared_ptr 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, SourceFrame); + void emit_video (boost::shared_ptr, double); void emit_subtitle (boost::shared_ptr); void repeat_last_video (); @@ -77,8 +77,8 @@ protected: private: void signal_video (boost::shared_ptr, boost::shared_ptr); - SourceFrame _video_frame; - SourceFrame _last_source_frame; + int _video_frame; + double _last_source_time; boost::shared_ptr _timed_subtitle; diff --git a/src/wx/film_viewer.cc b/src/wx/film_viewer.cc index 891d1671b..650bf7216 100644 --- a/src/wx/film_viewer.cc +++ b/src/wx/film_viewer.cc @@ -150,7 +150,13 @@ FilmViewer::set_film (shared_ptr f) void FilmViewer::decoder_changed () { - seek_and_update (_decoders.video->last_source_frame ()); + if (_decoders.video->seek_to_last ()) { + return; + } + + get_frame (); + _panel->Refresh (); + _panel->Update (); } void @@ -166,7 +172,7 @@ FilmViewer::timer (wxTimerEvent& ev) get_frame (); if (_film->length()) { - int const new_slider_position = 4096 * _decoders.video->last_source_frame() / _film->length().get(); + int const new_slider_position = 4096 * _decoders.video->last_source_time() / (_film->length().get() * _film->frames_per_second()); if (new_slider_position != _slider->GetValue()) { _slider->SetValue (new_slider_position); } @@ -199,23 +205,14 @@ FilmViewer::paint_panel (wxPaintEvent& ev) void FilmViewer::slider_moved (wxCommandEvent& ev) { - if (!_film) { + if (!_film || !_film->length()) { return; } - if (_film->length()) { - seek_and_update (_slider->GetValue() * _film->length().get() / 4096); - } -} - -void -FilmViewer::seek_and_update (SourceFrame f) -{ - if (_decoders.video->seek (f)) { - cout << "could not s&u to " << f << "\n"; + if (_decoders.video->seek (_slider->GetValue() * _film->length().get() / (4096 * _film->frames_per_second()))) { return; } - + get_frame (); _panel->Refresh (); _panel->Update (); diff --git a/src/wx/film_viewer.h b/src/wx/film_viewer.h index 5a791a8e2..723214ed1 100644 --- a/src/wx/film_viewer.h +++ b/src/wx/film_viewer.h @@ -53,7 +53,6 @@ private: void check_play_state (); void update_from_raw (); void decoder_changed (); - void seek_and_update (SourceFrame); void raw_to_display (); void get_frame (); void active_jobs_changed (bool);