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

[Steve Lhomme]

On Wed, Nov 2, 2016 at 11:55 AM, Filip Roséen <filip at atch.se> wrote:
> Hi Steve,
>
> On 2016-11-02 11:39, Steve Lhomme wrote:
>
>  As I said, the already merge change is wrong. Non-finite streams
>  should be treated as finite streams when it comes to the duration, so
>  long as we know the file end (I suppose STREAM_CAN_FASTSEEK implies
>  that).
>
> I do not agree to this, which you know.
>
>  But why is it OK with a file with missing/bogus Cues then ? This is
>  the exact same code that will try to find the duration.
>
>  Now I'm beginning to see a difference though. If one (and probably
>  all) Cluster has no known size we need to parse all elements/Blocks of
>  that Cluster. Whereas for a finite size Cluster we just skip over that
>  Cluster until we find no Cluster. That's a lot less skipping and
>  header reading.
>
>  So I agree that for non-finite Clusters we should not try too hard.
>  But that decision should be made in EnsureDuration(). Because if the
>  Cues is present we don't have to do that huge data parsing.
>
> Precisely my point from the beginning (though not where to have the decision
> take place - that is something I (now) agree with), there is a huge
> difference between knowing the cluster size (meaning that we can jump to the
> next one directly), and walking through the entire thing (scanning for the
> end).
>
>  It makes sense than a live Matroska recorder would keep track of where
>  it wrote each Cluster and write a Cues at the end, making the file a
>  lot easier to seek.
>
>  How about the attached patch ?
>
> If one really want to make sure that things behave as I have been trying to
> explain, one should add a check for non-finite cluster in the
> while-predicate, inside the if-block modified by your patch, and abort
> computation if such is discovered.
>
> The branch to locate the last cluster should be taken even if we have cues
> that have populated _seeker._cluster_positions (because there is nothing
> saying that the last entry in the _seeker corresponds to the last cluster in
> the file (broken or missing cues)).

Done & Done https://patches.videolan.org/patch/14898/

> Other than the above, LGTM.
>
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>