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

Clément Bœsch ubitux at gmail.com
Wed Aug 1 19:58:45 CEST 2012


On Wed, Aug 01, 2012 at 07:46:02PM +0200, Nicolas George wrote:
> Le quintidi 15 thermidor, an CCXX, Clément Bœsch a écrit :
> > 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.
> 
> I do not think you can rely on that (ffmpeg just happens to set the
> time_base to 1/1000, which is the same as hardcoded in Matroska), but that
> is just an additional rescale.
> 
> > 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;
> 
> Why this change? It looked right before.
> 

That is the assertion I was talking about; I thought that since that this
rescale was not required anymore after the change.

> Also, I can imagine some obscure subtitle codec that have perforce
> start_display_time = 200ms (i.e. the subtitle is always muxed 200ms ahead):
> the decoder would then adjust sub->pts accordingly. Or something.
> 

Well, to me the avcodec_encode_subtitle() is not supposed to change sub
content, so that value shouldn't have change anyway.

Thought, I reverted that chunk locally because of your first comment.

[...]

-- 
Clément B.
-------------- next part --------------
From 5caafea6d45deef92ed7cc003274e4ae8d91596a 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 | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/ffmpeg.c b/ffmpeg.c
index f85d8e0..cc1b32f 100644
--- a/ffmpeg.c
+++ b/ffmpeg.c
@@ -1683,12 +1683,15 @@ 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;
-- 
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/7217307b/attachment.asc>


More information about the ffmpeg-devel mailing list