[vlc-devel] [PATCH] TS demux: get time and length
Laurent Aimar
fenrir at elivagar.org
Thu Sep 15 21:58:00 CEST 2011
Hi,
On Thu, Sep 15, 2011 at 11:47:02PM +0900, Naohiro KORIYAMA wrote:
> 2011/9/15 Jean-Baptiste Kempf <jb at videolan.org>:
> > On Wed, Sep 14, 2011 at 01:22:18PM +0900, Naohiro KORIYAMA wrote :
> >> By previous patch, If I move time slider, wrong position is selected.
> >> I changed DEMUX_GET_POSITION to be calculated by time, but I think it
> >> is better to be calculated by stream position.
> diff --git a/modules/demux/ts.c b/modules/demux/ts.c
> index 441fb9b..e5dfd33 100644
> --- a/modules/demux/ts.c
> +++ b/modules/demux/ts.c
> @@ -336,6 +336,16 @@ struct demux_sys_t
> /* how many TS packet we read at once */
> int i_ts_read;
>
> + /* to determine length and time */
> + mtime_t i_first_pcr;
> + int64_t i_first_streampos;
> + mtime_t i_current_pcr;
> + int64_t i_current_streampos;
> + mtime_t i_last_pcr;
> + int64_t i_last_streampos;
> + mtime_t i_time;
> + int i_mux_rate;
> +
> /* All pid */
> ts_pid_t pid[8192];
>
> @@ -401,6 +411,7 @@ static inline int PIDGet( block_t *p )
>
> static bool GatherPES( demux_t *p_demux, ts_pid_t *pid, block_t *p_bk );
>
> +static void GetLastPCR( demux_t* p_demux );
> static void PCRHandle( demux_t *p_demux, ts_pid_t *, block_t * );
>
> static iod_descriptor_t *IODNew( int , uint8_t * );
> @@ -1219,7 +1230,6 @@ static int Control( demux_t *p_demux, int i_query, va_list args )
> return VLC_EGENERIC;
>
> return VLC_SUCCESS;
> -#if 0
>
> case DEMUX_GET_TIME:
> pi64 = (int64_t*)va_arg( args, int64_t * );
> @@ -1241,19 +1251,7 @@ static int Control( demux_t *p_demux, int i_query, va_list args )
> }
> *pi64 = 0;
> return VLC_EGENERIC;
> -#else
> - case DEMUX_GET_TIME:
> - pi64 = (int64_t*)va_arg( args, int64_t * );
> - if( DVBEventInformation( p_demux, pi64, NULL ) )
> - *pi64 = 0;
> - return VLC_SUCCESS;
>
> - case DEMUX_GET_LENGTH:
> - pi64 = (int64_t*)va_arg( args, int64_t * );
> - if( DVBEventInformation( p_demux, NULL, pi64 ) )
> - *pi64 = 0;
> - return VLC_SUCCESS;
> -#endif
I think you should still prefer the value coming from DVBEventInformation()
at least when p_sys->b_dvb_meta and p_sys->b_access_control are true, and
probably also when GetLastPCR() failed.
> +static void GetLastPCR( demux_t* p_demux )
> +{
> + demux_sys_t *p_sys = p_demux->p_sys;
You should first test for STREAM_CAN_SEEK capability and abort if not.
> + int64_t i_current_pos = stream_Tell( p_demux->s );
> +
> + int64_t i_last_pos = stream_Size( p_demux->s ) - p_sys->i_packet_size;
> + int64_t i_pos = i_last_pos - p_sys->i_packet_size * 1000; /* FIXME how many packets to get last pcr? */
> + if ( i_pos < 0 )
> + return;
> +
> + stream_Seek( p_demux->s, i_pos );
You should check for the return value.
> +
> + block_t *p_pkt;
You can move it down.
> + while( vlc_object_alive (p_demux) )
> + {
> + if( !( p_pkt = stream_Block( p_demux->s, p_sys->i_packet_size ) ) )
> + {
> + msg_Dbg( p_demux, "eof ?" );
> + break;
> + }
> + if( p_pkt->p_buffer[0] != 0x47 )
> + {
> + msg_Warn( p_demux, "lost synchro" );
> + block_Release( p_pkt );
> + while( vlc_object_alive (p_demux) )
> + {
> + const uint8_t *p_peek;
> + int i_peek, i_skip = 0;
> +
> + i_peek = stream_Peek( p_demux->s, &p_peek,
> + p_sys->i_packet_size * 10 );
> + if( i_peek < p_sys->i_packet_size + 1 )
> + {
> + msg_Dbg( p_demux, "eof ?" );
> + break;
> + }
> +
> + while( i_skip < i_peek - p_sys->i_packet_size )
> + {
> + if( p_peek[i_skip] == 0x47 &&
> + p_peek[i_skip + p_sys->i_packet_size] == 0x47 )
> + {
> + break;
> + }
> + i_skip++;
> + }
> + msg_Dbg( p_demux, "skipping %d bytes of garbage", i_skip );
> + stream_Read( p_demux->s, NULL, i_skip );
> +
> + if( i_skip < i_peek - p_sys->i_packet_size )
> + {
> + break;
> + }
> + }
> + if( !( p_pkt = stream_Block( p_demux->s, p_sys->i_packet_size ) ) )
> + {
> + msg_Dbg( p_demux, "eof ?" );
> + break;
> + }
> + }
You should factorize this code with the code from Demux() (I think they
are fully duplicated, no?)
> + const uint8_t *p = p_pkt->p_buffer;
> + if( ( p[3]&0x20 ) && /* adaptation */
> + ( p[5]&0x10 ) &&
> + ( p[4] >= 7 ) )
> + {
> + /* PCR is 33 bits */
> + const mtime_t i_pcr = ( (mtime_t)p[6] << 25 ) |
> + ( (mtime_t)p[7] << 17 ) |
> + ( (mtime_t)p[8] << 9 ) |
> + ( (mtime_t)p[9] << 1 ) |
> + ( (mtime_t)p[10] >> 7 );
> +
You should also factorize this code with the code from PCRHandle().
> + p_sys->i_last_pcr = i_pcr;
> + p_sys->i_last_streampos = stream_Tell( p_demux->s );
> + }
> + block_Release( p_pkt );
> +
> + if( stream_Tell( p_demux->s ) >= i_last_pos ) break;
> + }
> +
> + stream_Seek( p_demux->s, i_current_pos );
> + return;
Not needed.
> +}
> static void PCRHandle( demux_t *p_demux, ts_pid_t *pid, block_t *p_bk )
> {
> demux_sys_t *p_sys = p_demux->p_sys;
> @@ -1866,6 +1948,28 @@ static void PCRHandle( demux_t *p_demux, ts_pid_t *pid, block_t *p_bk )
> ( (mtime_t)p[9] << 1 ) |
> ( (mtime_t)p[10] >> 7 );
>
> + if( p_sys->i_first_pcr == 0 )
> + {
> + p_sys->i_first_pcr = i_pcr;
> + p_sys->i_first_streampos = stream_Tell( p_demux->s );
> +
> + GetLastPCR( p_demux );
> + if( p_sys->i_last_pcr - p_sys->i_first_pcr > 0 )
> + {
> + p_sys->i_mux_rate = (stream_Size( p_demux->s )) * INT64_C(1000000) / 50 / ((p_sys->i_last_pcr - p_sys->i_first_pcr) * 100 / 9);
I don't really understand the need for the 50 factor (I know it is already
present in the commented out code in Control() but...)
> + }
> + }
> + else
> + {
> + p_sys->i_current_pcr = i_pcr;
> + p_sys->i_current_streampos = stream_Tell( p_demux->s );
> +
> + if( p_sys->i_current_pcr > 0 && p_sys->i_first_pcr > 0 && p_sys->i_current_pcr > p_sys->i_first_pcr )
> + {
> + p_sys->i_time = (p_sys->i_current_pcr - p_sys->i_first_pcr) * 100 / 9;
> + }
> + }
> +
I think it would be better to look for the first and last PCR in a special
function called in the Open() one. This way the file length will be known
when preparsing.
Also, I have a concern about the way you look for the last/current PCR:
you must search for a PCR found for a same PID. In multiple program
streams, you can have multiple PIDs having PCR, and in this cases the
PCRs doesn't have the same origin.
Regards,
--
fenrir
More information about the vlc-devel
mailing list