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

Andreas Cadhalpun andreas.cadhalpun at googlemail.com
Sun Dec 27 21:29:25 CET 2015


On 27.12.2015 21:13, Hendrik Leppkes wrote:
> On Sun, Dec 27, 2015 at 9:03 PM, Andreas Cadhalpun
> <andreas.cadhalpun at googlemail.com> wrote:
>> On 27.12.2015 20:43, Hendrik Leppkes wrote:
>>> 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.
>>
>> I'd call this unknown timebase, as {0, 1} is the initialization value.
>> A {1, 0} timebase is certainly not valid.
>>
>>> All timebase checks are for num only.
>>
>> The check here was for den and there is a similar check in teletext_decode_frame.
> 
> And thats a bug since that can actually lead to div by 0 and crash.

Then please fix this bug also in teletext_decode_frame.

> The same timebase is checked 5 lines down for num already.

Indeed.

Best regards,
Andreas



More information about the ffmpeg-devel mailing list