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

Ilkka Ollakka ileoo at videolan.org
Mon Jun 7 20:41:35 CEST 2010


On Mon, Jun 07, 2010 at 08:59:08AM -0300, salsaman wrote:
>    Ooops. Forgot the patch !

>    On Mon, Jun 7, 2010 at 8:58 AM, salsaman <[3]salsaman at gmail.com> wrote:

>      OK, I hope this fixes all of the formatting problems now.

Hi, still few left, I assume you have set tab-expanding to spaces (1 tab
to 4 spaces ) and using monospace-font to see indent issues more easily? 


> From 6f67cd54f2c39a6a987dfa91ba21ce153160d0b3 Mon Sep 17 00:00:00 2001
> From: salsaman <salsaman at gmail.com>
> Date: Fri, 4 Jun 2010 00:49:03 -0300
> Subject: [PATCH] ogg/theora seek changes for GSOC project

> diff --git a/modules/demux/ogg.c b/modules/demux/ogg.c
> index 57d0c02..0575bae 100644
> --- a/modules/demux/ogg.c
> +++ b/modules/demux/ogg.c

Indent issue 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 */


Same 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;
> +	}
> +
> +    }
> +
> +

And here too:
it goes like
      if( stuff )
{

}

while it should be more like
      if( stuff ) 
      {

      }
atleast when the file otherwise is more like that.

> -
> -            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 );
> +		}
> +	    }
> +	    
>          }

>          while( ogg_stream_packetout( &p_stream->os, &oggpacket ) > 0 )
> @@ -378,8 +351,21 @@ static int Demux( demux_t * p_demux )
>                  {
>                      p_stream->i_secondary_header_packets = 0;
>                  }

This seems to be also indent off comparing the ending } below

> +
> +		/* update start of data pointer...
> +		 *   I think this is correct...*/
> +		p_sys->i_data_start = stream_Tell( p_demux->s );
> +
>              }

And these comparing to the next if() case

> +	    /* see if we are ignoring packets because of a seek */
> +	    if ( b_ignore) continue;
> +
> +	    /* If any streams have i_skip_frames, only decode (pre-roll)
> +	     *  for those streams */
> +	    if ( b_skipping && p_stream->i_skip_frames == 0 ) continue;
> +
> +
>              if( p_stream->b_reinit )
>              {
>                  /* If synchro is re-initialized we need to drop all the packets
> @@ -414,13 +400,15 @@ static int Demux( demux_t * p_demux )
>                  }
>              }

This is still here

> -            Ogg_DecodePacket( p_demux, p_stream, &oggpacket );
> -        }
> +	    Ogg_DecodePacket( p_demux, p_stream, &oggpacket );
> +	}


Some not needed empty lines 
> +
> +
>      /* if a page was waiting, it's now processed */
>      p_sys->b_page_waiting = false;


Indent is also off in here comparing to the following lines:

> @@ -429,6 +417,11 @@ static int Demux( demux_t * p_demux )
>      {
>          logical_stream_t *p_stream = p_sys->pp_stream[i_stream];

> +	/* 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;
> +
>          if( p_stream->fmt.i_cat == SPU_ES )
>              continue;
>          if( p_stream->i_interpolated_pcr < 0 )

And same for here

> +        case DEMUX_GET_LENGTH:
> +            pi64 = (int64_t*)va_arg( args, int64_t * );
> +            if( p_sys->i_total_frames > 0 )
> +            {
> +
> +		for( i = 0; i < p_sys->i_streams; i++ )
> +		{
> +		    logical_stream_t *p_stream = p_sys->pp_stream[i];
> +		    l_seek_frame = i64 * p_stream->f_rate;
> +		    if ( p_stream->fmt.i_codec == VLC_CODEC_THEORA )
> +		    {
> +			f_rate = p_stream->f_rate;
> +		    }
> +		}
> +
> +		*pi64 = INT64_C(1000000) * p_sys->i_total_frames / f_rate;
> +		return VLC_SUCCESS;
> +	    }
> +	    else
> +		return VLC_EGENERIC;
> +
>          case DEMUX_HAS_UNSUPPORTED_META:
>              pb_bool = (bool*)va_arg( args, bool* );
>              *pb_bool = true;
> @@ -474,7 +494,71 @@ static int Control( demux_t *p_demux, int i_query, va_list args )
>              return VLC_SUCCESS;

And here

>          case DEMUX_SET_TIME:
> -            return VLC_EGENERIC;
> +	  /* forbid seeking if we haven't initialized all logical bitstreams yet;
> +	     if we allowed, some headers would not get backed up and decoder init
> +	     would fail, making that logical stream unusable */
> +	  if( p_sys->i_bos > 0 )
> +	  {
> +	      return VLC_EGENERIC;
> +	  }
> +
> +	  /* length 0, probably a remote stream */
> +	  if ( p_sys->i_total_length == 0 )
> +	  {
> +	      return VLC_EGENERIC;
> +	  }
> +
> +
> +	  /* get target frame and seek to the seek point */
> +	  i64 = (int64_t)va_arg( args, int64_t );
> +	  if ( i64 > 0 )
> +	  {
> +	      for( i = 0; i < p_sys->i_streams; i++ )
> +	      {
> +		  logical_stream_t *p_stream = p_sys->pp_stream[i];
> +		  
> +		  if (p_stream->fmt.i_cat == VIDEO_ES)
> +		  {
> +		      l_seek_frame = i64 * p_stream->f_rate / INT64_C(1000000);
> +		      
> +		      if ( p_sys->i_total_frames > 0 && l_seek_frame >= p_sys->i_total_frames )
> +		      {
> +			  l_seek_frame = p_sys->i_total_frames - 1;
> +		      }
> +		      
> +		      if ( p_stream->fmt.i_codec == VLC_CODEC_THEORA )
> +		      {
> +			  b_seek_done = true;
> +			  i_retval = oggseek_find_frame( p_demux, p_stream, l_seek_frame );
> +			  es_out_Control ( p_demux->out, ES_OUT_SET_NEXT_DISPLAY_TIME, 
> +					   VLC_TS_0 + l_seek_frame * 
> +					   INT64_C(1000000) / p_stream->f_rate );
> +		      }
> +		      
> +		      break;
> +		  }
> +	      }
> +	      
> +	  }
> +
> +	  if ( b_seek_done )
> +	  {
> +	      for( i = 0; i < p_sys->i_streams; i++ )
> +	      {
> +		  logical_stream_t *p_stream = p_sys->pp_stream[i];
> +		  /* we'll trash all the data until we find the next pcr */
> +		  p_stream->b_reinit = true;
> +		  p_stream->i_pcr = -1;
> +		  p_stream->i_interpolated_pcr = -1;
> +		  ogg_stream_reset( &p_stream->os );
> +	      }
> +	      
> +	      ogg_sync_reset( &p_sys->oy );
> +	      
> +	      return i_retval;
> +	  }
> +
> +	  return VLC_EGENERIC;

>          case DEMUX_SET_POSITION:
>              /* forbid seeking if we haven't initialized all logical bitstreams yet;
> @@ -485,17 +569,65 @@ static int Control( demux_t *p_demux, int i_query, va_list args )
>                  return VLC_EGENERIC;
>              }

And here:

> -            for( i = 0; i < p_sys->i_streams; i++ )
> -            {
> -                logical_stream_t *p_stream = p_sys->pp_stream[i];
> +	    /* length 0, probably a remote stream */
> +	    if ( p_sys->i_total_length == 0 )
> +	    {
> +		return VLC_EGENERIC;
> +	    }
> +
> +
> +	    f = (double)va_arg( args, double );
> +	    if( f < 0.0 || f > 1.0 )
> +	    {
> +		return VLC_EGENERIC;
> +	    }
> +
> +
> +	    /* get target frame and seek to the seek point */
> +	    if ( p_sys->i_total_frames > 0 )
> +	    {
> +		
> +		for( i = 0; i < p_sys->i_streams; i++ )
> +		{
> +		    logical_stream_t *p_stream = p_sys->pp_stream[i];
> +		    
> +		    if (p_stream->fmt.i_cat == VIDEO_ES)
> +		    {
> +			l_seek_frame = f * ( p_sys->i_total_frames - 1 );
> +			
> +			if ( p_stream->fmt.i_codec == VLC_CODEC_THEORA )
> +			{
> +			    b_seek_done = true;
> +			    i_retval = oggseek_find_frame( p_demux, p_stream, l_seek_frame );
> +			    es_out_Control ( p_demux->out, ES_OUT_SET_NEXT_DISPLAY_TIME, 
> +					     VLC_TS_0 + l_seek_frame * 
> +					     INT64_C(1000000) / p_stream->f_rate );
> +			}
> +			
> +			break;
> +		    }
> +		}
> +		
> +		if ( b_seek_done )
> +		{
> +		    for( i = 0; i < p_sys->i_streams; i++ )
> +		    {
> +			logical_stream_t *p_stream = p_sys->pp_stream[i];
> +			/* we'll trash all the data until we find the next pcr */
> +			p_stream->b_reinit = true;
> +			p_stream->i_pcr = -1;
> +			p_stream->i_interpolated_pcr = -1;
> +			ogg_stream_reset( &p_stream->os );
> +		    }
> +		    
> +		    ogg_sync_reset( &p_sys->oy );
> +
> +		    return i_retval;
> +		}
> +	    }
> +
> +


Same here, block is indended somewhat oddly, also P_stream should be
p_stream ?

> +
> +    /* 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--;
> +    }
> +
> +

Here too indention is littlebit odd

> +    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;
> -
> +		
>                  p_stream = malloc( sizeof(logical_stream_t) );
>                  if( !p_stream )
>                      return VLC_ENOMEM;
> -
> +		
>                  TAB_APPEND( p_ogg->i_streams, p_ogg->pp_stream, p_stream );
> -
> +		
>                  memset( p_stream, 0, sizeof(logical_stream_t) );
>                  p_stream->p_headers = 0;
>                  p_stream->i_secondary_header_packets = 0;

Compare these ^^ lines to the lines below

> -
> +		
> +		p_stream->i_keyframe_offset = 0;
> +		p_stream->i_skip_frames = 0;
> +		p_stream->b_ignore_packets = false;
> +		
>                  es_format_Init( &p_stream->fmt, 0, 0 );
>                  es_format_Init( &p_stream->fmt_old, 0, 0 );


> -                if( Ogg_ReadPage( p_demux, &oggpage ) != VLC_SUCCESS )
> +                if( Ogg_ReadPage( p_demux ) != VLC_SUCCESS )
>                      return VLC_EGENERIC;

Same here, seemed to be indent littlebit off regarding lines before

> +
> +		/* set i_data_start now, before we read in the next page
> +		 * we will update this later if we get more bos packets */
> +		p_ogg->i_data_start = stream_Tell( p_demux->s );
> +
>              }


>          p_stream->p_es = NULL;

And this:

> +	/* initialise kframe index */
> +	p_stream->idx=NULL;
> +
> +

> @@ -1417,6 +1620,29 @@ static int Ogg_BeginningOfStream( demux_t *p_demux )
>          Ogg_LogicalStreamDelete( p_demux, p_ogg->p_old_stream );
>          p_ogg->p_old_stream = NULL;
>      }

Same for here, for-loop seems to have wrong indent

> +
> +
> +    /* 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++ )
> +	{
> +	    logical_stream_t *p_stream = p_ogg->pp_stream[i_stream];
> +
> +	    if ( p_stream->fmt.i_codec == VLC_CODEC_THEORA )
> +	    {
> +		p_ogg->i_total_frames = oggseek_get_last_frame( p_demux, p_stream );
> +
> +		msg_Dbg( p_demux, "Total frames %"PRId64, p_ogg->i_total_frames );
> +
> +	    }
> +	}
> +    }
> +
> +


> @@ -1457,6 +1683,11 @@ static void Ogg_LogicalStreamDelete( demux_t *p_demux, logical_stream_t *p_strea

Same for here inside block

> +    if ( p_stream->idx != NULL)
> +    {
> +	oggseek_index_entries_free( p_stream->idx );
> +    }
> +

> @@ -1616,6 +1853,14 @@ static void Ogg_ReadTheoraHeader( logical_stream_t *p_stream,

And here

> +    if ( i_version >= 3002001 )
> +    {
> +	p_stream->i_keyframe_offset = 1;
> +    }
> +
>      p_stream->f_rate = ((float)i_fps_numerator) / i_fps_denominator;
>  }

> diff --git a/modules/demux/oggseek.c b/modules/demux/oggseek.c
> new file mode 100644
> index 0000000..6fc0807
> --- /dev/null
> +++ b/modules/demux/oggseek.c
> @@ -0,0 +1,846 @@
> +void oggseek_index_entries_free ( demux_index_entry_t *idx )
> +{

Here the same

> +    while ( idx != NULL )
> +    {
> +	idx_next = idx->next;
> +	free( idx );
> +	idx = idx_next;
> +    }
> +}
> +

And here

> +
> +    if ( idx == NULL )
> +    {
> +	demux_index_entry_t *ie = index_entry_new();
> +	ie->i_granulepos = i_granule;
> +	ie->i_pagepos = i_pagepos;
> +	p_stream->idx=ie;
> +	return ie;
> +    }
> +
> +
> +    while ( idx != NULL )
> +    {
> +	i_gpos = idx->i_granulepos;
> +
> +	i_kframe = i_gpos >> p_stream->i_granule_shift;
> +	if ( i_kframe > i_tframe ) break;
> +

Compare that earlier if to this below (while while() seems to be off):

> +	if ( i_kframe == i_tkframe )
> +	{
> +	    /* entry exists, update it if applicable, and return it */
> +	    i_frame = i_kframe + i_gpos - ( i_kframe << p_stream->i_granule_shift );
> +	    if ( i_frame < i_tframe )
> +	    {
> +		idx->i_granulepos = i_granule;
> +		idx->i_pagepos = i_pagepos;
> +	    }
> +
> +	    return idx;
> +	}
> +
> +	last_idx = idx;
> +	idx = idx->next;
> +    }
> +

Skipping lines, I think you noticed the pattern?

Theres few leftover fprintf-debug lines here and there (all commented
out).

-- 
Ilkka Ollakka
For a man to truly understand rejection, he must first be ignored by a
cat.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20100607/d9dd2e2f/attachment.sig>


More information about the vlc-devel mailing list