<br clear="all"><br><a href="http://lives.sourceforge.net">http://lives.sourceforge.net</a><br><a href="https://www.ohloh.net/accounts/salsaman">https://www.ohloh.net/accounts/salsaman</a><br><br>
<br><br><div class="gmail_quote">On Mon, Jun 7, 2010 at 3:14 AM, Ilkka Ollakka <span dir="ltr"><<a href="mailto:ileoo@videolan.org">ileoo@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 Sun, Jun 06, 2010 at 03:43:00PM -0300, salsaman wrote:<br>
> Updated patch. Please let me know if it is OK now.<br>
<br>
> Gabriel.<br>
<br>
</div>Hi,<br>
<br>
Have to say, I don't know ogg that well, that I could really comment on<br>
code content that much (but looks ok ). Still few indent issues here and<br>
there, but nothing major.<br>
<br>
Like here:<br>
> @@ -323,15 +263,35 @@ static int Demux( demux_t * p_demux )<br>
>          /*<br>
>           * Demux an ogg page from the stream<br>
>           */<br>
> -        if( Ogg_ReadPage( p_demux, &oggpage ) != VLC_SUCCESS )<br>
> +      if( Ogg_ReadPage( p_demux ) != VLC_SUCCESS )<br>
>              return 0; /* EOF */<br>
<br></blockquote><div><br>That is not my code, so I don't know why the indentation changed.<br><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;">


<br>
Here:<br>
<br>
> +    /* if any streams have b_ignore_packets, don't decode anything<br>
> +     * except headers */<br>
> +    for( i_stream = 0; i_stream < p_sys->i_streams; i_stream++ )<br>
> +    {<br>
> +     logical_stream_t *p_stream = p_sys->pp_stream[i_stream];<br>
> +<br>
> +     if ( p_stream->b_ignore_packets )<br>
> +     {<br>
> +         b_ignore = true;<br>
> +     }<br>
> +<br>
> +     /* check also if we are pre-rolling as the result of a search */<br>
> +     if ( p_stream->i_skip_frames > 0 )<br>
> +     {<br>
> +         b_skipping = true;<br>
> +     }<br>
> +<br>
> +      }<br>
<br></blockquote><div> <br>OK. Looks like I missed the closing brace.<br></div><div><br> </div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">


<br>
Here:<br>
<br>
> -            if( ogg_stream_pagein( &p_stream->os, &oggpage ) != 0 )<br>
> +            if( ogg_stream_pagein( &p_stream->os, &p_sys->current_page ) != 0 )<br>
> +         {<br>
> +             msg_Warn ( p_demux, "Got incomplete page" );<br>
>                  continue;<br>
> +         }<br>
> +<br>
> +         if ( p_sys->i_total_length > 0 )<br>
> +         {<br>
> +             if ( p_stream->fmt.i_codec == VLC_CODEC_THEORA )<br>
> +             {<br>
> +                 i_gpos = ogg_page_granulepos( &p_sys->current_page );<br>
> +                 oggseek_theora_index_entry_add ( p_stream, i_gpos, p_sys->i_input_position );<br>
> +             }<br>
> +         }<br>
> +<br>
>          }<br>
<br></blockquote><div><br>What is the problem there ?<br><br><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;">
<br>
Few comments still have those gf:-tags<br>
<br>
> +     /* gf: after a seek we only ignore all packets which end on the first page,<br>
> +      * unless we are seeking to the very first frame */<br>
> +<br>
> +     p_stream->b_ignore_packets = false;<br>
> +<br>
<br></blockquote><div><br>Yes, you are right, I am sorry I must have overlooked them when I was trying various things with the .emacs file.<br><br><br><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;">


<br>
Indent error<br>
<br>
> @@ -673,7 +850,7 @@ static void Ogg_DecodePacket( demux_t *p_demux,<br>
>              p_stream->p_headers = realloc( p_stream->p_headers, p_stream->i_headers );<br>
>              if( p_stream->p_headers )<br>
>              {<br>
> -                memcpy( p_stream->p_headers + p_stream->i_headers - p_oggpacket->bytes,<br>
> +           memcpy( (unsigned char *)p_stream->p_headers + p_stream->i_headers - p_oggpacket->bytes,<br>
>                          p_oggpacket->packet, p_stream->i_headers );<br>
>              }<br>
>              else<br>
> @@ -791,6 +968,15 @@ static void Ogg_DecodePacket( demux_t *p_demux,<br>
<br></blockquote><div><br>Again, that is not my code, so I don't know why it changed, maybe when I was testing the .emacs files.<br><br><br><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;">


Here too<br>
<br>
> +    /* may need to preroll video frames after a seek */<br>
> +    if ( p_stream->i_skip_frames > 0 )<br>
> +    {<br>
> +     p_block->i_flags |= BLOCK_FLAG_PREROLL;<br>
> +     P_stream->i_skip_frames--;<br>
> +    }<br>
<br></blockquote><div><br><br>That one looks fine in the code, maybe it is just the way the patch is formatted ?<br><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;">


and here<br>
<br>
> +    while( Ogg_ReadPage( p_demux ) == VLC_SUCCESS )<br>
> +      {<br>
> +        if( ogg_page_bos( &p_ogg->current_page ) )<br>
> +       {<br>
> +<br>
> +<br>
> +         /* All is wonderful in our fine fine little world.<br>
> +          * We found the beginning of our first logical stream. */<br>
> +         while( ogg_page_bos( &p_ogg->current_page ) )<br>
> +           {<br>
>                  logical_stream_t *p_stream;<br>
<br></blockquote><div><br>OK, again not my code, but maybe I was testing the indentation there.<br><br><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;">


Seems like indent error here also, and forgotten gf-tag<br>
<br>
> +     /* gf: initialise kframe index */<br>
> +     p_stream->idx=NULL;<br>
> +     /* gf: end */<br>
> +<br>
> +<br>
>          /* Try first to reuse an old ES */<br>
>          if( p_old_stream &&<br>
<br></blockquote><div><br>OK, I removed the gf-tag, but I don't see any indent error in the original.<br><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;">


Indent error<br>
> @@ -1417,6 +1621,29 @@ static int Ogg_BeginningOfStream( demux_t *p_demux )<br>
>          Ogg_LogicalStreamDelete( p_demux, p_ogg->p_old_stream );<br>
>          p_ogg->p_old_stream = NULL;<br>
>      }<br>
> +<br>
> +<br>
> +    /* get total frame count for video stream; we will need this for seeking */<br>
> +    p_ogg->i_total_frames = 0;<br>
> +<br>
> +    if ( p_ogg->i_total_length > 0 )<br>
> +    {<br>
> +<br>
> +     for( i_stream = 0 ; i_stream < p_ogg->i_streams; i_stream++ )<br>
> +     {<br>
<br>
Similar indent issues still in few places, not sure if the extras emacs<br>
stuff is out of date/broken in that sense or if you just forgot to<br>
reindent those files.<br>
<font color="#888888"><br></font></blockquote><div><br>OK, it's not an indent error, but I think there is a white-space error, as I just went through pressing tab. I will fix that.<br><br><br><br>BTW,<br>The extras/emacs stuff was not working. It did not do the opening brace 0 indent properly.<br>

<br><br>In the end I just added a line for the 0 brace indent to my .emacs file.<br><br><br>(I tried combining the two things together but it did not work.)<br><br><br><br>Regards,<br>Gabriel.<br><br></div></div>