[FFmpeg-devel] [PATCH] Fix possible SIGFPEs with bad mov timings. Based on chromium patch 35_mov_bad_timings.patch

Baptiste Coudurier baptiste.coudurier
Thu Nov 19 20:33:26 CET 2009


On 11/19/2009 10:38 AM, Albert J. Wong (???) wrote:
> On Thu, Nov 19, 2009 at 9:12 AM, Ronald S.
> Bultje<rsbultje at gmail.com>wrote:
>
>> Hi,
>>
>> On Thu, Nov 19, 2009 at 1:45 AM, Baptiste Coudurier
>> <baptiste.coudurier at gmail.com>  wrote:
>>> I don't like the<  0 check, but I guess we don't have much choice
>>> here.
>>
>
> Yeah...also, the time_scale value is just a plain int itself, which
> means that negatives are at least "valid" programmatically regardless
> of spec. And having it be negative is pretty unexpected downstream.
> Combine that with the ease with which you can get signed-unsigned
> promotions accidentally in C, and this just becomes an annoying
> risk.
>
> Er...
>>
>> A) AVURational (bikeshed!!!!111) B) uint32_t num = READ_BE32(..),
>> den = READ_BE32(..); if (num&  0x80000000 || den&  0x80000000) {
>> den>>= 1; num>>= 1 }
>>
>
> Good point. Having a timescale in the 4-billion range is not likely
> going to be very interesting.  I personally prefer the erroring out
> scenario approach you outlined below rather than trying to "correct"
> the timescale from one value w/o a great experience into another
> value w/o a great experience. I've attached another patch to that
> effect.  Let me know what you guys think.

Specs are using a 32 unsigned integer.
Either we support it completely, or we choose an acceptable workaround.

> 35_mov_bad_timings_upstream.patch
>
>
> diff --git libavformat/mov.c libavformat/mov.c index 6a8c149..09a72f9
> 100644 --- libavformat/mov.c +++ libavformat/mov.c @@ -1624,11
> +1624,12 @@ static int mov_read_trak(MOVContext *c, ByteIOContext
> *pb, MOVAtom atom) return 0; }
>
> -    if (!sc->time_scale) { -        av_log(c->fc, AV_LOG_WARNING,
> "stream %d, timescale not set\n", st->index); +    if
> (c->time_scale<= 0) +        return -AVERROR_INVALIDDATA; +    if
> (sc->time_scale<= 0) { +        av_log(c->fc, AV_LOG_WARNING, "stream
> %d, timescale %d invalid\n", st->index, +
> sc->time_scale); sc->time_scale = c->time_scale; -        if
> (!sc->time_scale) -            sc->time_scale = 1; }
>
> av_set_pts_info(st, 64, 1, sc->time_scale);

Like I said I'm against erroring out. The file could be still demuxed to 
raw, completely ignoring timestamps.

-- 
Baptiste COUDURIER
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
FFmpeg maintainer                                  http://www.ffmpeg.org



More information about the ffmpeg-devel mailing list