<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>