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

[salsaman]

http://lives.sourceforge.net
https://www.ohloh.net/accounts/salsaman



On Mon, Jun 7, 2010 at 3:14 AM, Ilkka Ollakka <ileoo at videolan.org> wrote:

> On Sun, Jun 06, 2010 at 03:43:00PM -0300, salsaman wrote:
> > Updated patch. Please let me know if it is OK now.
>
> > Gabriel.
>
> Hi,
>
> Have to say, I don't know ogg that well, that I could really comment on
> code content that much (but looks ok ). Still few indent issues here and
> there, but nothing major.
>
> Like here:
> > @@ -323,15 +263,35 @@ static int Demux( demux_t * p_demux )
> >          /*
> >           * Demux an ogg page from the stream
> >           */
> > -        if( Ogg_ReadPage( p_demux, &oggpage ) != VLC_SUCCESS )
> > +      if( Ogg_ReadPage( p_demux ) != VLC_SUCCESS )
> >              return 0; /* EOF */
>
>
That is not my code, so I don't know why the indentation changed.




>
> Here:
>
> > +    /* if any streams have b_ignore_packets, don't decode anything
> > +     * except headers */
> > +    for( i_stream = 0; i_stream < p_sys->i_streams; i_stream++ )
> > +    {
> > +     logical_stream_t *p_stream = p_sys->pp_stream[i_stream];
> > +
> > +     if ( p_stream->b_ignore_packets )
> > +     {
> > +         b_ignore = true;
> > +     }
> > +
> > +     /* check also if we are pre-rolling as the result of a search */
> > +     if ( p_stream->i_skip_frames > 0 )
> > +     {
> > +         b_skipping = true;
> > +     }
> > +
> > +      }
>
>
OK. Looks like I missed the closing brace.



>
> Here:
>
> > -            if( ogg_stream_pagein( &p_stream->os, &oggpage ) != 0 )
> > +            if( ogg_stream_pagein( &p_stream->os, &p_sys->current_page )
> != 0 )
> > +         {
> > +             msg_Warn ( p_demux, "Got incomplete page" );
> >                  continue;
> > +         }
> > +
> > +         if ( p_sys->i_total_length > 0 )
> > +         {
> > +             if ( p_stream->fmt.i_codec == VLC_CODEC_THEORA )
> > +             {
> > +                 i_gpos = ogg_page_granulepos( &p_sys->current_page );
> > +                 oggseek_theora_index_entry_add ( p_stream, i_gpos,
> p_sys->i_input_position );
> > +             }
> > +         }
> > +
> >          }
>
>
What is the problem there ?





>
> Few comments still have those gf:-tags
>
> > +     /* gf: after a seek we only ignore all packets which end on the
> first page,
> > +      * unless we are seeking to the very first frame */
> > +
> > +     p_stream->b_ignore_packets = false;
> > +
>
>
Yes, you are right, I am sorry I must have overlooked them when I was trying
various things with the .emacs file.






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






> Here too
>
> > +    /* may need to preroll video frames after a seek */
> > +    if ( p_stream->i_skip_frames > 0 )
> > +    {
> > +     p_block->i_flags |= BLOCK_FLAG_PREROLL;
> > +     P_stream->i_skip_frames--;
> > +    }
>
>

That one looks fine in the code, maybe it is just the way the patch is
formatted ?




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





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




> Indent error
> > @@ -1417,6 +1621,29 @@ static int Ogg_BeginningOfStream( demux_t *p_demux
> )
> >          Ogg_LogicalStreamDelete( p_demux, p_ogg->p_old_stream );
> >          p_ogg->p_old_stream = NULL;
> >      }
> > +
> > +
> > +    /* get total frame count for video stream; we will need this for
> seeking */
> > +    p_ogg->i_total_frames = 0;
> > +
> > +    if ( p_ogg->i_total_length > 0 )
> > +    {
> > +
> > +     for( i_stream = 0 ; i_stream < p_ogg->i_streams; i_stream++ )
> > +     {
>
> Similar indent issues still in few places, not sure if the extras emacs
> stuff is out of date/broken in that sense or if you just forgot to
> reindent those files.
>
>
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.



BTW,
The extras/emacs stuff was not working. It did not do the opening brace 0
indent properly.


In the end I just added a line for the 0 brace indent to my .emacs file.


(I tried combining the two things together but it did not work.)



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