[vlc-devel] [PATCH 02/14] mkv: make sure indexes are in order

[Steve Lhomme]

On Wed, Mar 9, 2016 at 2:21 PM, Denis Charmet <typx at dinauz.org> wrote:
> Hi,
>
> While I agree that nothing in the spec prevents, it is very unlikely since
> the cues are generated as the file is being muxed and there are few chances
> that the muxer will start indexing bipredicitive frames which would be the
> only ones "out of order" of course you could have corruptions.

For the record, file written with libmatroska are guaranteed to have
sorted Cues thanks to this line:
https://github.com/Matroska-Org/libmatroska/blob/master/matroska/KaxCues.h#L67

Since we're using libmatroska for reading, rather than recreating a
sort, you might use the same Sort() call before actually using the
data in the KaxCues that you just read.

> In any case you should apply that to IndexAppendCluster too since it also
> adds indexes while the file is being read. It was my poor attempt at somehow
> improving the seek without cues for a backward case.
>
> We'll discuss that more extensively when I come at videolabs. But if you
> want to start fixing the Seek and Cthulhu knows it *does* need fixes...
> indexes shouldn't be a vector but a key ordered map with timecode as the key
> so we can start adding indexes of key frames and maybe each track that would
> need some kind of preroll could have its own map... so we could even
> reinject correctly the seekpoint subtitles when seeking.
>
> On 2016-03-09 13:12, Filip Roséen wrote:
>>
>> To add a little bit of information regarding the mental process
>> leading up this patch:
>>
>> I could not find anything proving that the Cues are guaranteed to
>> appear in order, and since we (when looking for the index) are doing a
>> sequential search for the _first_ Cue (index) that is bigger than the
>> desired seekpoint I was worried that a file containing Cues that are
>> not in ascending order would make us beginning seeking at the wrong
>> place.
>>
>> Let’s say we receive the Cues in the following order:
>>
>>         * Cue #1: i_mk_time = 0
>>         * Cue #2: i_mk_time = 100
>>         * Cue #3: i_mk_time = 90
>>
>> The code responsible for seeking used a for-loop iterating through
>> each Cue (index) (in order), breaking when i_mk_date <
>> mkv_index.i_mk_time + mkv_index.i_mk_time_offset (where i_mk_date is
>> the desired seeking position, and mkv_index is an object of type
>> mkv_index_t).
>>
>> It then takes one step back (since it expects the index before our
>> find to be smaller than i_mk_date).
>>
>> If we then imagine i_mk_date = 95, we would first end up at _Cue #2_,
>> and then begin to seek from _Cue #1_ (even though _Cue #3_ would be a
>> far better place).
>>
>> We would find _Cue #3_ if the cue’s appeared in sorted order inside
>> our container of indexes.
>>
>> Please correct me if I understood things incorrectly, I am still very
>> new to the matroska fileformat and libebml; thanks!
>> -------------------------
>>
>> On 16/03/09 13:00, Jean-Baptiste Kempf wrote:
>>
>>> On 09/03/2016 12:49, Filip Roséen wrote:
>>>
>>>> This patch will protect us from seeking too far if a mkv-files contains
>>>> Cues that are not in ascending order.
>>>
>>>
>>> TypX, robux?
>>>
>>> – Jean-Baptiste Kempf http://www.jbkempf.com/ - +33 672 704 734 Sent from
>>> my Electronic Device _______________________________________________
>>> 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
>
>
> Regards,
> --
> Denis Charmet - TypX
> Le mauvais esprit est un art de vivre
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel