[FFmpeg-devel] [PATCH 3/3] ffmpeg: ensure that -fix_sub_duration doesnt create subtitles with zero duration

[Marton Balint]

On Sat, 23 Nov 2013, Nicolas George wrote:

> Le tridi 3 frimaire, an CCXXII, Marton Balint a écrit :
>> Signed-off-by: Marton Balint <cus at passwd.hu>
>> ---
>>  ffmpeg.c | 12 ++++++++----
>>  1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/ffmpeg.c b/ffmpeg.c
>> index 5ba9450..4bb88e6 100644
>> --- a/ffmpeg.c
>> +++ b/ffmpeg.c
>> @@ -1801,19 +1801,23 @@ static int transcode_subtitles(InputStream *ist, AVPacket *pkt, int *got_output)
>>      }
>>
>>      if (ist->fix_sub_duration) {
>> +        int end = 1;
>>          if (ist->prev_sub.got_output) {
>> -            int end = av_rescale(subtitle.pts - ist->prev_sub.subtitle.pts,
>> -                                 1000, AV_TIME_BASE);
>> +            end = av_rescale(subtitle.pts - ist->prev_sub.subtitle.pts,
>> +                             1000, AV_TIME_BASE);
>>              if (end < ist->prev_sub.subtitle.end_display_time) {
>>                  av_log(ist->st->codec, AV_LOG_DEBUG,
>> -                       "Subtitle duration reduced from %d to %d\n",
>> -                       ist->prev_sub.subtitle.end_display_time, end);
>> +                       "Subtitle duration reduced from %d to %d%s\n",
>> +                       ist->prev_sub.subtitle.end_display_time, end,
>> +                       end <= 0 ? ", dropping it" : "");
>>                  ist->prev_sub.subtitle.end_display_time = end;
>>              }
>>          }
>>          FFSWAP(int,        *got_output, ist->prev_sub.got_output);
>>          FFSWAP(int,        ret,         ist->prev_sub.ret);
>>          FFSWAP(AVSubtitle, subtitle,    ist->prev_sub.subtitle);
>> +        if (end <= 0)
>> +            goto out;
>>      }
>>
>>      if (!*got_output)
>
> This would treat differently subtitles with 0 duration depending on whether
> it comes from -fix_sub_duration or was there from the start. It it on
> purpose?
>
> Can you explain (or point to the mail where you already did, if so) where
> you are going with that? The encode-decode API of subtitles is a mess, but
> we will not manage to fix it unless we at least agree on how it should be.

Yes, it is on purpose. As w4m pointed out in response to my earlier patch:

https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2013-November/thread.html#151037

0 duration can also mean unknown (infinite) duration depending on the 
codecs used, so it would be wrong to drop them all.

However, when fix_sub_duration is used, and the duration fixing code is 
generating 0 duration, that is definitely zero, and not undefined or 
infinite, so it is safe to drop it.

Regards,
Marton