VideoPlayer: check demux stream before dual layer test - #80
Merged
Portisch merged 1 commit intoAug 30, 2026
Merged
Conversation
OpenDefaultStreams dereferences the result of m_pDemuxer->GetStream() without a null check. On the DVD menu domain the demuxer holds no streams, while the selection streams come from libdvdnav with source STREAM_SOURCE_NAV and a nav index as id, so the lookup returns nullptr and Kodi segfaults before the first stream is opened. Only demuxer streams can carry the dual layer flags, so restrict the probe to STREAM_SOURCE_DEMUX and verify the stream exists and really is a video stream before casting. CDVDDemux::GetStream() ignores the demuxer id and looks up by stream id alone across all stream types, so the cast was unchecked for type as well. Fixes: 2a9a87e ("Videoplayer: open base layer as default on dual layer video")
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
CVideoPlayer::OpenDefaultStreams()dereferences whatm_pDemuxer->GetStream()returns without checking it:The guard checks
m_pDemuxer, but not the stream it hands back. That loop walksm_SelectionStreams, which also holds NAV, Bluray, text and external streams, and theiridis not a demuxeruniqueId—CSelectionStreams::Update()setss.id = stream->uniqueIdfor demuxer streams, but for DVD navigator streams it setss.id = i, a libdvdnav index. In the DVD menu domain the demuxer has no streams at all yet, so the lookup returnsnullptrand the next instruction faults.The cast is unchecked for type too, which I only noticed while fixing the first part.
CDVDDemux::GetStream(int64_t, int)throws the demuxer id away (return GetStream(iStreamId);) andCDVDDemuxFFmpeg::m_streamsis one map keyed byuniqueIdacross all stream types, so a NAV video stream withid = 1gets back whatever holds uniqueId 1 — on a DVD that is usually an audio stream. ReadingisDualStream/isELStreamoff aCDemuxStreamAudioreads past the end of the object and can quietly skip a valid video stream. Not fatal, so it would have survived a plain null check.So I gated on the source mask and validated the stream before casting, which is what
IsValidStream()already does a few hundred lines further down.CDVDDemuxFFmpeg::ReadInternal()guards this same cast the same way where it latches these two flags onto the packet — null check, thenstream->type == StreamType::VIDEO, then cast.Motivation and context
Every nightly since 2026-08-20 segfaults on Amlogic when a DVD is opened from its menu (
VIDEO_TS.IFOor ISO). PlayingVTS_*.VOBdirectly still works, which is why it first looked like certain discs were broken rather than DVD playback in general.It is a regression from 2a9a87e ("Videoplayer: open base layer as default on dual layer video"). What that commit is for still works after this change:
isDualStream/isELStreamonly ever get set inCDVDDemuxFFmpegon the Dolby Vision dual layer path behindaml_dolby_vision_enabled(), so they can only exist onSTREAM_SOURCE_DEMUXstreams. A stream the demuxer does not have cannot be an enhancement layer, so restricting the probe to demuxer streams cannot stop the EL being excluded where it actually matters. Bluray goes through theelse if (demuxer)branch inUpdate()and keeps sourceSTREAM_SOURCE_DEMUX, so DV P7 on Bluray is unaffected.How has this been tested?
I have not compile-tested this — I do not have a build tree set up. It touches one block, adds no includes, and only uses things the file already has (
STREAM_SOURCE_MASK(stream.source) == STREAM_SOURCE_DEMUXis the same expression as line 1230, andCDemuxStream*is used bare at 1459, 1468 and 1486), but that is me reading the code, not a build. It applies cleanly toaml-5.15.196-22.0at 9f5601b.What I did test is the diagnosis, on a Ugoos AM9 Pro (S905X5) on
22.0-Piers_nightly_20260828, Kodi0f9a78be:Five crashes, same backtrace every time —
CVideoPlayer::OpenDefaultStreams(bool)→Prepare()→Process(), always PC0x9dbf00. The kernel fault dump givesx0 : 0000000000000000in all five,ESR 0x92000006(data abort, level 2 translation fault, read), faulting address0x185. The mapping is00400000-01fd1000 r-xp 00000000, so the binary is not PIE and the PC is a link time address I could look up directly:0x185is 389, so the fault address isx0 + offsetof(isDualStream)withx0null. Nothing between the call and the load. GCC devirtualisingGetStream()down toGetStream(int)is also what makes the type confusion above reachable.To be sure it was this and not something nearby, I patched the
ldrbat0x9dbf00in the shippedkodi.bininto a branch past the check — the old control flow, before 2a9a87e — and bind mounted it over/usr/lib/kodi/kodi.bin. The discs that crashed five times now play through their menus, and the log showsOpening stream: 1 source: 512where it used to just stop. That confirms the cause and the code path, but it is not this patch's source, which is still uncompiled.For comparison,
nightly_20260730(Kodi726799f4, before the commit) logsOpening stream: 1 source: 512on the same discs at exactly the point where 0828 dies —source: 512beingSTREAM_SOURCE_NAV, id 1 the nav index. I hit it on three different discs, over both a CIFS mount andsmb://, so it is not disc or path specific.What is the effect on users?
DVD menus work again on Amlogic. Dolby Vision profile 7 is unchanged — the enhancement layer is still kept out of default video stream selection on the demuxer streams that can actually carry those flags.
Screenshots (if appropriate):
Types of change
Checklist: