[vlc-devel] [PATCH 1/4] mkv/demux: fix 17567: null-pointer dereference in EnsureDuration

[Steve Lhomme]

On Wed, Nov 2, 2016 at 10:03 AM, Filip Roséen <filip at atch.se> wrote:
> Hi,
>
> I think we can agree on the fact that we do not agree about the semantics of
> how to treat unread clusters of unknown size.
>
> My primary objective was to fix the crash, if you want to change the
> semantics of EnsureDuration (which you approved in the previous patch-set) I
> will not be overly excited, but as long as the crash is fixed.. it’s better
> than nothing.
>
> Thanks,
> Filip Roséen
>
> On 2016-11-02 09:50, Steve Lhomme wrote:
>
>  If the source is not being update/appended while reading (a finite
>  size file) then you know where the end is and you can fully parse the
>  file.
>
> Of course, when the stream is fully exhausted we can seek back to the
> beginning and play the file; but my reasoning is precisely not to exhaust a
> stream where an non-finite cluster is present.
>
>  That has nothing to do with the fact the muxer didn't know the end
>  when it wrote the data.
>
> The fact that the muxer did not know, or was not clever enough to fill in
> that piece of data after the cluster was created, should not cause us to
> potentially run into issues where playback does not start within a
> reasonable amount of time, or not at all, because we are waiting for
> EnsureDuration to finish.
>
> I rather eliminate all false-positives and not exhaust clusters where the
> size is inherently unknown, than to start running through the stream (when
> we have no idea where we will stop).
>
>  So far we rely on STREAM_CAN_FASTSEEK in EnsureDuration to tell in
>  which case we are. It may not be perfect but that's closer to not
>  parsing the duration for a fully written file.
>
> EnsureDuration is a hack to get a duration for files that does not include
> such in its metadata, and with that I don’t think we should invoke usage of
> this hack in places where we cannot prove (to 100%) that the stream actually
> is finite (if the size of a cluster is not there, nothing states that we
> will get to the end).

To summarize the issue, is STREAM_CAN_FASTSEEK_FORWARD needed ?

As you say, it's a way to know the duration even if it's not written
specifically. So that's already a fringe case. If the file is
particularly badly muxed, then too bad it's normal that it doesn't
play quickly. But we shouldn't remove the feature of knowing the
duration of a fully written file just to avoid to read bad files too
much. The same applies for a file with no duration and no Cues. All
Clusters and the whole last Cluster will need to be parsed. And that
has nothing to do with the finite size. It has everything to do with
how fast we can seek through all these data.

So yes, f2756634ed3601fa7c00e974533f68fcbbb12d06 is wrong IMO too. I
overlooked it.

> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>