[FFmpeg-devel] [PATCH] ffmpeg: Preserve input subtitle packet duration.

[Nicolas George]

Le sextidi 16 fructidor, an CCXX, Philip Langdale a écrit :
> My original implementation for setting out output packet duration
> derived the value from the AVSubtitle object, but the value stored
> in there has been rescaled to ASS 1/100s timebase, while the
> normalized timebase for both input and output packets is 1/1000s,
> so there's a gratuitous loss of precision, which results in output
> durations not corresponding exactly to input durations - which can
> impact subtitles where one is supposed to exactly follow another.
> 
> The simple solution is to carry the original input duration over
> to the output packet (with rescaling, of course), and ignore the
> AVSubtitle value.
> 
> Signed-off-by: Philip Langdale <philipl at overt.org>
> ---
>  ffmpeg.c |    8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/ffmpeg.c b/ffmpeg.c
> index 2763db6..4d8a1f9 100644
> --- a/ffmpeg.c
> +++ b/ffmpeg.c
> @@ -689,7 +689,8 @@ static void do_subtitle_out(AVFormatContext *s,
>                              OutputStream *ost,
>                              InputStream *ist,
>                              AVSubtitle *sub,
> -                            int64_t pts)
> +                            int64_t pts,
> +                            int32_t duration)
>  {
>      int subtitle_out_max_size = 1024 * 1024;
>      int subtitle_out_size, nb, i;
> @@ -743,7 +744,7 @@ static void do_subtitle_out(AVFormatContext *s,
>          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.duration = av_rescale_q(sub->end_display_time, (AVRational){ 1, 1000 }, ost->st->time_base);
> +        pkt.duration = av_rescale_q(duration, (AVRational){ 1, 1000 }, ost->st->time_base);
>          if (enc->codec_id == AV_CODEC_ID_DVB_SUBTITLE) {
>              /* XXX: the pts correction is handled here. Maybe handling
>                 it in the codec would be better */
> @@ -1691,7 +1692,8 @@ static int transcode_subtitles(InputStream *ist, AVPacket *pkt, int *got_output)
>          if (!check_output_constraints(ist, ost) || !ost->encoding_needed)
>              continue;
>  
> -        do_subtitle_out(output_files[ost->file_index]->ctx, ost, ist, &subtitle, pts);
> +        do_subtitle_out(output_files[ost->file_index]->ctx, ost, ist,
> +                        &subtitle, pts, pkt->duration);
>      }
>  
>      avsubtitle_free(&subtitle);

I am sorry, but it seems completely wrong to me. In particular, it
completely undoes the work of the fix_sub_duration option. It will also
break when the duration is encoded in the subtitle bitstream and not in the
format bitstream, like in DVD subtitles.

Can you show examples bad behaviours you are trying to fix?

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120901/fe53ff43/attachment.asc>