[FFmpeg-devel] [PATCH] ffmpeg: honor -ss and -t parameters with muxed subtitles.

[Clément Bœsch]

On Wed, Aug 01, 2012 at 07:01:15PM +0200, Nicolas George wrote:
> Le quintidi 15 thermidor, an CCXX, Clément Bœsch a écrit :
> > This patch fixes two things:
> > 
> >  - in case of subtitles, check_recording_time() is comparing the current
> >    PTS to the recording time (-t option, set to INT_MAX by default), so
> >    the -ss option needs to be taken into account. It is not required in
> >    do_{audio,video}_out() because this adjustment is set while polling
> >    the filtergraph (see poll_filters()).
> > 
> >  - the second fix is also an adjustment of the PTS send to the encoder
> >    (and later transmitted to the muxer) so the TS in the output make
> >    sense and are not kept verbatim.
> > 
> > Note: this only works for muxers honoring the PTS, such as
> > lavf/matroskaenc. But for other such as the ASS muxer which just does a
> > verbatim copy, or the SubRip muxer which doesn't write the TS in some
> > cases, it will not work yet.
> > ---
> >  ffmpeg.c | 11 ++++++++++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> > 
> > diff --git a/ffmpeg.c b/ffmpeg.c
> > index f85d8e0..c6fafe4 100644
> > --- a/ffmpeg.c
> > +++ b/ffmpeg.c
> > @@ -1684,13 +1684,22 @@ static void do_subtitle_out(AVFormatContext *s,
> >          nb = 1;
> >  
> >      for (i = 0; i < nb; i++) {
> > -        ost->sync_opts = av_rescale_q(pts, ist->st->time_base, enc->time_base);
> > +        ost->sync_opts = av_rescale_q(pts, ist->st->time_base, enc->time_base)
> > +                       /* start time adjustment so -ss and -t works */
> > +                       - av_rescale_q(output_files[ost->file_index]->start_time, AV_TIME_BASE_Q, enc->time_base);
> > +
> >          if (!check_recording_time(ost))
> >              return;
> >  
> >          sub->pts = av_rescale_q(pts, ist->st->time_base, AV_TIME_BASE_Q);
> >          // start_display_time is required to be 0
> >          sub->pts               += av_rescale_q(sub->start_display_time, (AVRational){ 1, 1000 }, AV_TIME_BASE_Q);
> > +
> > +        /* The following adjustment is only done for audio & video types when
> > +         * dealing with lavfi. This should be dropped when subtitles are part
> > +         * of the lavfi loop */
> > +        sub->pts -= output_files[ost->file_index]->start_time;
> > +
> >          sub->end_display_time  -= sub->start_display_time;
> >          sub->start_display_time = 0;
> >          subtitle_out_size = avcodec_encode_subtitle(enc, subtitle_out,
> 
> I believe you could do simpler by adding at the beginning:
> 
> pts = av_rescale_q(pts, ist->st->time_base, AV_TIME_BASE_Q) - start_time;
> 
> and then working with it. But do not put the conversion in the nb loop, or
> it will be done twice.
> 

Ah good point, nice idea. See new patch.

It still works with my test case (./ffmpeg -i in.mkv -ss 00:04:10 -t 30
-c:s ass -y out.mkv, and it also works with -c:s copy).

Note: this version is assuming enc->time_base and ost->st->time_base have
the same value.

[...]

-- 
Clément B.
-------------- next part --------------
From c23b79af9fd2bb7c9dec018eb610377227d485e4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20B=C5=93sch?= <ubitux at gmail.com>
Date: Wed, 1 Aug 2012 18:39:46 +0200
Subject: [PATCH] ffmpeg: honor -ss and -t parameters with muxed subtitles.

This patch fixes two things:

 - in case of subtitles, check_recording_time() is comparing the current
   PTS to the recording time (-t option, set to INT_MAX by default), so
   the -ss option needs to be taken into account. It is not required in
   do_{audio,video}_out() because this adjustment is set while polling
   the filtergraph (see poll_filters()).

 - It also adjusts the PTS sent to the encoder (and later transmitted to
   the muxer) so the TS in the output make sense and are not kept
   verbatim.

Note: this only works for muxers honoring the PTS, such as
lavf/matroskaenc. But for other such as the ASS muxer which just does a
verbatim copy, or the SubRip muxer which doesn't write the TS in some
cases, it will not work yet.
---
 ffmpeg.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/ffmpeg.c b/ffmpeg.c
index f85d8e0..620903e 100644
--- a/ffmpeg.c
+++ b/ffmpeg.c
@@ -1683,12 +1683,16 @@ static void do_subtitle_out(AVFormatContext *s,
     else
         nb = 1;
 
+    /* shift timestamp to honor -ss and make check_recording_time() work with -t */
+    pts = av_rescale_q(pts, ist->st->time_base, AV_TIME_BASE_Q)
+        - output_files[ost->file_index]->start_time;
     for (i = 0; i < nb; i++) {
-        ost->sync_opts = av_rescale_q(pts, ist->st->time_base, enc->time_base);
+        ost->sync_opts = av_rescale_q(pts, AV_TIME_BASE_Q, enc->time_base);
+
         if (!check_recording_time(ost))
             return;
 
-        sub->pts = av_rescale_q(pts, ist->st->time_base, AV_TIME_BASE_Q);
+        sub->pts = pts;
         // start_display_time is required to be 0
         sub->pts               += av_rescale_q(sub->start_display_time, (AVRational){ 1, 1000 }, AV_TIME_BASE_Q);
         sub->end_display_time  -= sub->start_display_time;
@@ -1703,7 +1707,7 @@ static void do_subtitle_out(AVFormatContext *s,
         av_init_packet(&pkt);
         pkt.data = subtitle_out;
         pkt.size = subtitle_out_size;
-        pkt.pts  = av_rescale_q(sub->pts, AV_TIME_BASE_Q, ost->st->time_base);
+        pkt.pts  = ost->sync_opts;
         pkt.duration = av_rescale_q(sub->end_display_time, (AVRational){ 1, 1000 }, ost->st->time_base);
         if (enc->codec_id == CODEC_ID_DVB_SUBTITLE) {
             /* XXX: the pts correction is handled here. Maybe handling
-- 
1.7.11.3

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120801/8777f25d/attachment.asc>