[Ffmpeg-devel] Re: [PATCH] seeking in GXF

[Reimar Döffinger]

Hello,
On Mon, Jul 31, 2006 at 07:33:03PM +0200, Michael Niedermayer wrote:
> On Mon, Jul 31, 2006 at 05:44:47PM +0200, Reimar D?ffinger wrote:
> [...]
> > +static void gxf_material_tags(ByteIOContext *pb, int *len, st_info_t *si) {
[...]
> somehow i think this is more complex then really needed ...

Probably. I changed it in a way that looks simpler to me, but still does
almost the same (the previous version did not reset the values for
different tracks).
Maybe the whole idea with using st_info_t was stupid, but I can't think
of a solution that isn't ugly ATM. Well, if I ever have one I can still
fix in SVN.

> > +    static const AVRational map[] = {{0, 0}, {60, 1}, {60000, 1001}, {50, 1},
> > +        {30, 1}, {30000, 1001}, {25, 1}, {24, 1}, {24000, 1001}};
> 
> ff_frame_rate_tab[] in mpeg12data.h

Shocking. It's really those values just in reverse...

> > +    int idx =  ff_log2_tab[(flags & 0x7c0) >> 6];
> 
> why dont you use av_log2() ?

Well, while reading common.h I saw av_log2, av_log2_16bit, and thus
assumed if there are only 8 relevant bits I'm supposed to use the array
directly. Anyway, changed.

> > +        av_add_index_entry(st, get_le32(pb) * 1024, i * fields_per_map + 1, 0, 0, 0);
> 
> the multiplications are int and will overflow

I hope I added some casts at the right place now.

> > +    if (maxlen < 200 * 1024) maxlen = 200 * 1024;
> 
> FFMAX()

Ts, and I always said we need the useful stuff in libavutil in MPlayer
and then don't even use it ;-)

I also made the seek function return -1 if the found timestamp is not
close enough (+- 4 fields), thus falling back to binary search via
gxf_read_timestamp - hopefully will help in case of broken index.

I decided to apply, feel free to flame about any remaining issues on
-cvslog *g*

Greetings,
Reimar D?ffinger