[FFmpeg-devel] [PATCH] Seeking and resync support in nuv demuxer

[Reimar Döffinger]

On Sat, Jun 07, 2008 at 03:22:44PM +0200, elupus wrote:
> On Wed, 28 May 2008 21:55:30 +0200, Reimar D?ffinger wrote:
> > On Wed, May 28, 2008 at 02:10:24AM +0200, elupus wrote:
> >> [26 quoted lines suppressed]
> > 
> > Just a quick comment: IMO you really should use the "standard" way to
> > search for a pattern:
> > use a uint32_t and shift in each character and compare against MKTAG(...).
> > On match, for comparing the remaining 'j' you can then each time read in
> > and compare a full 4 bytes, so no need for an extra loop.
> 
> So here's a updated patch that uses MKTAG and shifts in data. Not sure it
> ends up much simpler.

Well, I think you did it the complicated way.

> +/**
> + * \brief looks for the string RTjjjjjjjjjj in the stream too resync reading
> + * \return TRUE if the syncword is found.

1, not TRUE.

> +static int nuv_resync(AVFormatContext *s, int64_t pos_limit) {
> +    ByteIOContext *pb = s->pb;
> +    uint32_t tag;
> +
> +    tag = get_be32(pb);
> +    while(!url_feof(pb) && url_ftell(pb) < pos_limit) {
> +        if(tag != MKBETAG('R','T','j','j')) {
> +            tag = (tag << 8) | get_byte(pb);
> +            continue;
> +        }
> +        tag = get_be32(pb);
> +        if(tag != MKBETAG('j','j','j','j'))
> +          continue;
> +
> +        tag = get_be32(pb);
> +        if(tag != MKBETAG('j','j','j','j'))
> +          continue;
> +
> +        return 1;
> +    }
> +    return 0;

Should something like
uint32_t tag = 0;
while (...) {
  tag = (tag << 8) | get_byte(pb);
  if (                 tag == MKBETAG('R','T','j','j') &&
      (tag = get_be32(pb)) == MKBETAG('j','j','j','j') &&
      (tag = get_be32(pb)) == MKBETAG('j','j','j','j'))
    return 1;
}

can maybe be done less ugly, but it should not be much more code than
this, and in particular I am against continues, breaks etc. unless they
decidedly simplify the code.

> +                // TODO - add general support in av_gen_search, so it adds positions after reading timestamps

I have the feeling there is a very good reason why this is not done.
At least that it leads to a very partial index seems "dangerous" to me.

> +                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);

And either way I really think this should not be done here at least.

Greetings,
Reimar D?ffinger