[vlc-devel] Updated patch for ogg/theora seeking

[salsaman]

On Mon, Jun 7, 2010 at 4:42 PM, Jean-Baptiste Kempf <jb at videolan.org> wrote:

> On Mon, Jun 07, 2010 at 08:59:08AM -0300, salsaman wrote :
> > diff --git a/modules/codec/theora.c b/modules/codec/theora.c
>
> > +    /* need to skip some pages after a seek */
> > +    if ( p_block->i_flags & BLOCK_FLAG_PREROLL )
> > +        {
> > +            p_sys->b_skip_frame = true;
> > +    }
> > +    else p_sys->b_skip_frame = false;
> Wouldn't something like this, be better?
> p_sys->b_skip_frame = ( p_block->i_flags & BLOCK_FLAG_PREROLL );
>
>
>

Yes, you are right. I will change it.



> > +             /* update start of data pointer...
> > +              *   I think this is correct...*/
> > +             p_sys->i_data_start = stream_Tell( p_demux->s );
> Can anyone  comment on the correctness?
>
> > +    long l_seek_frame;
> Why long here?
>
> > +                 l_seek_frame = i64 * p_stream->f_rate;
> ?
>
>
>
It was long in the original code. Probably it could be int64.



> > +                 if ( p_stream->fmt.i_codec == VLC_CODEC_THEORA )
> > +                 {
> > +                     f_rate = p_stream->f_rate;
> > +                 }
> Does it apply to THEORA only or all video codecs? (dirac?vp8?)
>
>
I don't know yet...



> > +           for( i = 0; i < p_sys->i_streams; i++ )
> Side note: you can define loop counter inside for( in VLC code.
>
> > +  int i_page_size;
> Is int the best here?
>
>
int is fine. The page size is usually about 8Kb. There is a max somewhere in
the standard which I am sure is << INT32_MAX.



> > +struct _oggseek_index_entry
> I doubt this is legal: starting with _ . Aren't _* reserved?
>
>
I am not sure, but this is typedefed to demux_index_t anyway. In my own code
I generally use an _ to indicate use some typedefed version.

Gabriel.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20100607/a8ed6e2d/attachment.html>