[FFmpeg-devel] [PATCH 05/28] support seeking in nuv files

[Joakim Plate]

Reimar D?ffinger <Reimar.Doeffinger <at> gmx.de> writes:


> >                  // HACK: we have no idea if it is a keyframe,
> >                  // but if we mark none seeking will not work at all.
> >                  pkt->flags |= AV_PKT_FLAG_KEY;
> >                  pkt->pos = pos;
> >                  pkt->pts = AV_RL32(&hdr[4]);
> >                  pkt->stream_index = ctx->v_id;
> > +                if(hdr[2] == 0) {
> > +                    AVStream *st = s->streams[pkt->stream_index];
> > +                    pkt->flags |= PKT_FLAG_KEY;
> > +                    av_add_index_entry(st, pkt->pos, pkt->pts, size + 
HDRSIZE, 0, AVINDEX_KEYFRAME);
> > +                }
> 

> Also, the generic code is supposed to handle adding index entries in this way.
> And indeed we have
> >      .flags = AVFMT_GENERIC_INDEX,
> So this means we end having index entries double.
> Now, using hdr[2] to indicate keyframes looks like a good idea, but I
> tried that originally and there are just too many files around where it
> is not set or not set correctly.

Original thread on this patch:

http://thread.gmane.org/gmane.comp.video.ffmpeg.devel/68778/focus=68794

I just never got around to work more on it. (doubtfull i'll look at it more than 
commenting here either)

| On the note of AVFMT_GENERIC_INDEX, that only works if the stream is set to
| require parsing. Non parsed packets are not considered. Even if they have
| PKT_FLAG_KEY set.


> 
> > +                // TODO - add general support in av_gen_search, so it adds 
positions after reading timestamps
> > +                av_add_index_entry(s->streams[frametype == NUV_VIDEO ? ctx-
>v_id : ctx->a_id]
> > +                                 , pos, dts, size + HDRSIZE, 0, hdr[2] == 0 
? AVINDEX_KEYFRAME : 0);
> 
> Unless this makes a large speed difference I am against this.
> 

| Also too speed up seeks using av_seek_frame_binary, I added index entries
| when a timestamp is found using read_timestamp. However wouldn't it be
| better to add this to av_seek_frame_binary? perhaps only when
| AVFMT_GENERIC_INDEX is set in format.


If i remember correctly it made quite a difference.

/Joakim