[FFmpeg-devel] [PATCH] Support for variable frame duration

[Baptiste Coudurier]

Hi,

Henrik Gulbrandsen wrote:
> On Sat, 2008-04-26 at 14:06 +0200, Baptiste Coudurier wrote:
> 
> [...]
> 
>>> so it's not like there is a performance issue, and
>>> the next_dts computation is where it is because that's where sample is
>>> determined in the first place. 
>> But it's not at all where we need and use this information.
>>
>>> As far as I can see, there's no AVStream
>>> reference in MOVStreamContext, so I would have to save st in order to
>>> perform the computation outside the loop. Wouldn't this just introduce
>>> an unneeded variable and make the code harder to read?
>>>
>> I don't think so, s->streams[sc->ffindex].
> 
> Thanks for that pointer! Something like this, then?
> 
> The patch is certainly more compact, although the number of things that
> happen between the setting of sample and the new location would make me
> slightly nervous as the reader of this code. For instance, it's easy to
> miss that sc->current_sample++ in the middle.

Add comment :>

> Also, of course, this patch explicitly doesn't handle the ctts case.
> There are some tricky border cases there that I can't easily test, so
> that problem is left for somebody who actually needs the solution.

Ok.

> [...]
>
>      } else {
> +        AVStream *st = s->streams[sc->ffindex];
> +        int64_t next_dts = (sc->current_sample < sc->sample_count)?
> +            st->index_entries[sc->current_sample].timestamp : st->duration;
> +

Remove the empty line, add comment and it will be ok :>

[...]

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
SMARTJOG S.A.                                    http://www.smartjog.com
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
Phone: +33 1 49966312