<br><br><div class="gmail_quote">On Mon, Jun 7, 2010 at 4:42 PM, Jean-Baptiste Kempf <span dir="ltr"><<a href="mailto:jb@videolan.org">jb@videolan.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">

<div class="im">On Mon, Jun 07, 2010 at 08:59:08AM -0300, salsaman wrote :<br>
</div>> diff --git a/modules/codec/theora.c b/modules/codec/theora.c<br>
<br>
> +    /* need to skip some pages after a seek */<br>
<div class="im">> +    if ( p_block->i_flags & BLOCK_FLAG_PREROLL )<br>
> +        {<br>
> +            p_sys->b_skip_frame = true;<br>
> +    }<br>
</div>> +    else p_sys->b_skip_frame = false;<br>
Wouldn't something like this, be better?<br>
p_sys->b_skip_frame = ( p_block->i_flags & BLOCK_FLAG_PREROLL );<br>
<br>
<br></blockquote><div><br><br>Yes, you are right. I will change it.<br><br> </div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
> +             /* update start of data pointer...<br>
<div class="im">> +              *   I think this is correct...*/<br>
> +             p_sys->i_data_start = stream_Tell( p_demux->s );<br>
</div>Can anyone  comment on the correctness?<br>
<br>
> +    long l_seek_frame;<br>
Why long here?<br>
<br>
> +                 l_seek_frame = i64 * p_stream->f_rate;<br>
?<br>
<br>
<br></blockquote><div><br>It was long in the original code. Probably it could be int64.<br><br> </div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">


> +                 if ( p_stream->fmt.i_codec == VLC_CODEC_THEORA )<br>
> +                 {<br>
> +                     f_rate = p_stream->f_rate;<br>
> +                 }<br>
Does it apply to THEORA only or all video codecs? (dirac?vp8?)<br>
<br></blockquote><div><br>I don't know yet...<br><br> </div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
> +           for( i = 0; i < p_sys->i_streams; i++ )<br>
Side note: you can define loop counter inside for( in VLC code.<br>
<br>
> +  int i_page_size;<br>
Is int the best here?<br>
<br></blockquote><div><br>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.<br><br> </div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">


> +struct _oggseek_index_entry<br>
I doubt this is legal: starting with _ . Aren't _* reserved?<br>
<br></blockquote><div><br>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.<br><br>Gabriel.<br><br></div></div>