[vlc-devel] [PATCH] demux: mkv: fix seek

[Steve Lhomme]

On 2018-06-29 17:56, Zhao Zhili wrote:
>
>> On Jun 29, 2018, at 10:36 PM, Steve Lhomme <robux4 at ycbcr.xyz> wrote:
>>
>> Hi
>>
>>
>> On 2018-06-26 11:48, Zhao Zhili wrote:
>>> This fixes a regression from 6b10c2e6. Seek failed due to empty
>>> _tracks_seekpoints[video_track].
>>> ---
>>>   modules/demux/mkv/matroska_segment.cpp | 9 +++++++++
>>>   1 file changed, 9 insertions(+)
>>>
>>> diff --git a/modules/demux/mkv/matroska_segment.cpp b/modules/demux/mkv/matroska_segment.cpp
>>> index 148e66e..844558c 100644
>>> --- a/modules/demux/mkv/matroska_segment.cpp
>>> +++ b/modules/demux/mkv/matroska_segment.cpp
>>> @@ -635,6 +635,15 @@ bool matroska_segment_c::Preload( )
>>>                   cluster = kc_ptr;
>>> +
>>> +            // add first cluster as trusted seekpoint for all tracks
>> Not exactly trusted, it just kickstarts the seeking with a known position.
>>
>>> +            for( tracks_map_t::const_iterator it = tracks.begin();
>>> +                 it != tracks.end(); ++it )
>>> +            {
>>> +                _seeker.add_seekpoint( it->first,
>>> +                    SegmentSeeker::Seekpoint( cluster->GetElementPosition(), 0 ) );
>> It marks the Seekpoint as TRUSTED, ie sure to be a seek point for that track because we encountered a keyframe. Thie may not be the case at all.
>> SegmentSeeker::Seekpoint::TrustLevel::QUESTIONABLE would be better here.
> Good point.
>
>> Also a pts of 0 may not be correct. A Segment can start with a high PTS if it's the continuation of a previous Segment. So using -1 here is better (technically it should be VLC_TS_INVALID but that's not what the code expects).
> Yes, -1 is better. Technically it should be VLC_TS_LOWER_BOUND but we don't have that. INVALID doesn't mean it's small, although let VLC_TS_INVALID equal to INT64_MAX will break a lot of code I guess.

We're in C++. We shouldn't even have to provide a fake value when we 
don't have one.

As for VLC_TS_INVALID the goal is to set it to INT64_MIN. It should 
never be tested as superior or inferior but equal or different.
Yes some code will break and we'll fix it.

>
>> After these 2 changes the fix still works but it's cleaner.
>>
>>> +            }
>>> +
>>>               /* stop pre-parsing the stream */
>>>               break;
>>>           }
>>> -- 
>>> 2.9.5
>>>
>>> _______________________________________________
>>> vlc-devel mailing list
>>> To unsubscribe or modify your subscription options:
>>> https://mailman.videolan.org/listinfo/vlc-devel
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel