[FFmpeg-devel] [PATCH] avcodec/utils: fix check for invalid timebase when decoding subtitles

[Hendrik Leppkes]

On Sun, Dec 27, 2015 at 8:29 PM, Andreas Cadhalpun
<andreas.cadhalpun at googlemail.com> wrote:
> On 27.12.2015 20:10, Hendrik Leppkes wrote:
>> Invalid timebases have a zero numerator, not denominator.
>
> A timebase with zero numerator is probably invalid, but a timebase
> with zero denominator is not even well defined.
> So this comment doesn't seem quite right.
>
>> Fixes a integer divison by zero.
>> ---
>>  libavcodec/utils.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
>> index 19f3f0a..33295ed 100644
>> --- a/libavcodec/utils.c
>> +++ b/libavcodec/utils.c
>> @@ -2435,7 +2435,7 @@ int avcodec_decode_subtitle2(AVCodecContext *avctx, AVSubtitle *sub,
>>          } else {
>>              avctx->internal->pkt = &pkt_recoded;
>>
>> -            if (avctx->pkt_timebase.den && avpkt->pts != AV_NOPTS_VALUE)
>> +            if (avctx->pkt_timebase.num && avpkt->pts != AV_NOPTS_VALUE)
>>                  sub->pts = av_rescale_q(avpkt->pts,
>>                                          avctx->pkt_timebase, AV_TIME_BASE_Q);
>>              ret = avctx->codec->decode(avctx, sub, got_sub_ptr, &pkt_recoded);
>>
>
> If avctx->pkt_timebase.den is 0, calling av_rescale_q here will trigger the
> av_assert2(c > 0) in av_rescale_rnd, so removing this check isn't good.
>
> Why not check for both num and den?
>

We never check both, invalid timebase is {0, 1}, anything else needs
to have values in both. All timebase checks are for num only.
Its an assert2 for a reason (ie. a developer tool), its checked in an
error condition right after and returns INT64_MIN.