[vlc-devel] [PATCH 2/4] demux: mkv: truncate length on preskip, don't set negative pts

[Steve Lhomme]

On 2020-10-01 12:05, Francois Cartegnie wrote:
> Le 01/10/2020 à 09:10, Steve Lhomme a écrit :
>>>> +            if( track.i_codec_delay ) /* Handle pre-skip */
>>>> +            {
>>>> +                if( i_pts > track.i_codec_delay )
>>>> +                {
>>>> +                    p_block->i_length -= track.i_codec_delay;
>>>
>>> That looks suspicious. The delay should influence the PTS/DTS, not the
>>> duration of a block. If you wanted to skip a part at the beginning of
>>> a block you would also need to specify the offset to skip.
> 
> We have no per block metadata for that purpose, so that's how it is
> notified to decoder.
> 
> Preskip is the amount of data which is used to initialize the decoder,
> and does not output audio samples. The length of that block is minus prekip.

Still. You are not doing what the spec is specifically telling you to 
do: apply the CodecDelay on all PTS values. You cannot remove that part.

>>>> +                }
>>>> +                else
>>>> +                {
>>>> +                    vlc_tick_t skipped = i_pts - track.i_codec_delay;
>>>
>>> Here i_pts <= track.i_codec_delay. So this value is negative.
>>>
>>>> +                    if( p_block->i_length <= skipped )
>>>
>>> So this is always false.
>>
>> OK it's not negative if i_pts == track.i_codec_delay. I think in this
>> case we should not consider it as pre-roll. And CodecDelay is *not* a
>> preroll anyway.
> 
> The preskip data skip offsets time by 20ms, so that makes no difference.
> For opus and vorbis it is a delay from the pts.

Still, you're adding incorrect tests. The value can only be 0 at best, 
so you should test for that. Not whether it's bigger or smaller.

>>>
>>>> +                    {
>>>> +                        p_block->i_length = 0;
>>>> +                        p_block->i_flags |= BLOCK_FLAG_PREROLL;
>>>> +                    }
>>>> +                    else p_block->i_length -= skipped;
>>>
>>> The block length is always expanded.
>>>
> 
> 
> -- 
> Francois Cartegnie
> VideoLAN - VLC Developer
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>