[vlc-devel] [PATCH] TS demux: get time and length

[Naohiro KORIYAMA]

Hi Laurent,

Thank you for the review.

2011/9/16 Laurent Aimar <fenrir at elivagar.org>:
>  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.

I put the test at  DEMUX_GET_TIME and DEMUX_GET_LENGTH.

>  You should first test for STREAM_CAN_SEEK capability and abort if not.

I put the test at Open() and if the test is passed, call GetFirstPCR()
and GetLastPCR(),
also calculate i_mux_rate.

>> +    stream_Seek( p_demux->s, i_pos );
>  You should check for the return value.
>> +    block_t     *p_pkt;
>  You can move it down.

OK.

>  You should factorize this code with the code from Demux() (I think they
> are fully duplicated, no?)

As you say, I copied this part from Demux(). I refactor it to ReadTSPacket().

>  You should also factorize this code with the code from PCRHandle().

Done.

>> +    return;
>  Not needed.

OK.

>> +                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...)

I think it needless too. So, I deleted the 50 factor.

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

I moved GetLastPCR() to Open() and also put the new function
GetFirstPCR() there .

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

I prepared the variable p_sys->i_pid_first_pcr to remember  the PID of
the first PCR.
And I looked for the last/current PCR which have the same PID of the first PCR.

Regards,
-- 
KORIYAMA, Naohiro
nkoriyama at gmail.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-TS-demux-2985-Enhance-TS-demuxer-to-support-duration.patch
Type: application/octet-stream
Size: 13196 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20110916/a4c3f69d/attachment.obj>