Guard the missing mdia box in the MP4 track picker - #13229
Merged
Conversation
PickMp4TrackViewModel.Initialize dereferenced track.Mdia unconditionally
for four members and then guarded it with ?. for the fifth. The compiler
carried that maybe-null state back over the loop edge, so the first
unguarded access warned:
PickMp4TrackViewModel.cs(66,31): warning CS8602: Dereference of a
possibly null reference.
Mp4Parser.GetSubtitleTracks() drops mdia-less traks, so no shipping call
path can hit it today - but Initialize takes any List<Trak>, and the
mixed style left it ambiguous which way the code meant to lean.
The track's mdia is now read once into a local and skipped when absent,
which settles the question and drops the UI project to zero warnings.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
Clears the one warning left in the UI project, noticed while building #13226.
Why it warned
Initialize(List<Trak>, string)builds each display row from fivetrack.Mdiamembers - four dereferenced unconditionally, andDurationguarded withtrack.Mdia?.Minf?.... That?.putstrack.Mdiainto maybe-null state, and the state is carried back over theforeachedge, so the first access in the next iteration (line 66) is the one that warns.libseis nullable-oblivious, so nothing outside this loop contributes.Is it reachable
Not today. Every call site goes through
Mp4Parser.GetSubtitleTracks(), which already filters ontrak.Mdia != null && trak.Mdia.Minf?.Stbl != null:Mp4FragmentedSubtitleTrackand never touchesMdiaBut
Initializeis public and accepts anyList<Trak>-Moov.Tracksunfiltered would NRE - and the mixed style left it ambiguous which way the method meant to lean. So this guards rather than suppresses.Change
The track's
mdiais read once into a local and the row is skipped when it is absent; the remainingMinf?.Stbl?chain stays, since those really can be missing on a track that has anmdia.Left alone
Export()has the same unguarded chain -track.Mdia.IsVobSubSubtitleat line 141, thentrack.Mdia.Minf.Stblthree times at 149-151. It does not warn (no?.onMdiain that method to seed the state) and it is equally unreachable, but it is the same latent NRE. Out of scope here - happy to fold it in if you want the file consistent.Verification
dotnet build src/ui/UI.csproj -c Release- 0 warnings, 0 errors (was 1 warning)dotnet test tests/UI/UITests.csproj -c Release- 1360 passed, 0 failed🤖 Generated with Claude Code