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

[Reimar Döffinger]

On Wed, Jun 30, 2010 at 05:37:36PM +0000, Joakim Plate wrote:
> 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.


You mean like:
------------------------------------------------------------------------
r17965 | reimar | 2009-03-14 22:46:00 +0100 (Sat, 14 Mar 2009) | 2 lines

Add AVFMT_GENERIC_INDEX support also for formats that do not use a parser.

------------------------------------------------------------------------

> > > +                // 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.

Nobody ever gave me a sample to actually test though :-)