Fix QML crash on stem track load: null WaveformWidgetFactory - #17015
Open
nomiapps wants to merge 1 commit into
Open
Fix QML crash on stem track load: null WaveformWidgetFactory#17015nomiapps wants to merge 1 commit into
nomiapps wants to merge 1 commit into
Conversation
WaveformWidgetFactory::createInstance() has exactly one call site, mixxxmainwindow.cpp, so under --qml / --new-ui the singleton is never created. The allshader beat and slip-mode renderers dereference it in preprocessInner() to read isStemSplitTracks(), but only behind an isStemTrack && short-circuit -- so plain tracks were fine and loading any stem file crashed on the scene graph render thread. Resolve the factory once at construction and null-check it at the use site. Ask isCreated() rather than calling instance() directly: Singleton::instance() trips a VERIFY_OR_DEBUG_ASSERT of its own when the singleton was never created, which terminates builds configured with MIXXX_DEBUG_ASSERTIONS_FATAL and otherwise logs a warning for every renderer. With the factory absent, splitStemTracks is false and the renderers draw the unsplit beat grid, which is what the QML UI showed before stem support existed. Reported in mixxxdj#17011 as an assert at singleton.h:22 reached from allshader_sg::WaveformRenderBeat::preprocessInner on a debug build. Verified on Windows against main: mixxx --new-ui loading a .stem.mp4 into a deck now decodes the stems and runs with no assert and no "Singleton class has not been created yet" warnings, where the same build without this change logged one per affected renderer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
The error/bug report was not about --new-ui but about the QML Latenight, Did you regenerate the error first before trying to fix it? |
Contributor
|
I already solved the issue in 38f421e. Sorry for late communication. |
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.
Fixes #17011.
What happens
Loading any stem file in the QML UI (
--qml/--new-ui) kills Mixxx. On a build withMIXXX_DEBUG_ASSERTIONS_FATALit terminates on the assert @Eve00000 captured in #17011:with the stack, on the scene graph render thread:
On a release build there is no assert, so
instance()returnsnullptr, the very nextdereference runs, and it is an access violation in the same function instead.
Why
WaveformWidgetFactory::createInstance()has exactly one call site in the tree,src/mixxxmainwindow.cpp, which is the QWidget main window. The QML UI never constructsit, so the singleton is null for the whole session.
WaveformRenderBeat::preprocessInner()andWaveformRendererSlipMode::preprocessInner()both do:
The dereference sits behind the
isStemTrack &&short-circuit, which is why the QML UIlooked healthy until the first stem track — plain tracks never evaluate the right-hand
side.
The fix
Resolve the factory once at construction, and null-check it at the use site.
The construction-time lookup asks
isCreated()instead of callinginstance()directly.That matters:
Singleton::instance()runs aVERIFY_OR_DEBUG_ASSERT(m_instance)of itsown, so merely asking for the pointer is fatal under
MIXXX_DEBUG_ASSERTIONS_FATALandlogs
Singleton class has not been created yet, returning nullptrotherwise — once perrenderer, per session.
dlgpreferences.cppalready guards the same singleton this way.With the factory absent,
splitStemTracksis false and the renderers draw the unsplitbeat grid — the behaviour the QML UI had before stem support existed. Nothing changes for
the QWidget UI, where the factory always exists.
Testing
Windows,
RelWithDebInfo,-DQML=ON,mixxx --new-ui --developerwith a.stem.mp4passed on the command line so it loads straight into deck 1.
Singleton class has not been created yet, returning nullptrwarnings onthe render thread; on a fatal-assert build this is QML Latenight crashes on loading a stemfile (Win & Linux) #17011's crash.
SoundSourceSTEM/Track - Importing stem(s) info), waveformrenderers come up for both decks, zero singleton warnings, no assert, still running
after 40s.
Note for reviewers
In the QML UI this value is computed and then never used --
splitStemTracksonly feeds theslip beat renderer, which QML never builds (its beat renderer is fixed at
PositionSource::Play). That is why the crash surfaced on a plain non-slip renderer with nointerest in the answer.
An alternative would be to compute
splitStemTracksinside them_isSlipRendererbranch thatconsumes it, removing the factory access by construction rather than by null check. Happy to
respin it that way if you prefer; as it stands the guard is the smaller, more backportable
change.
🤖 Generated with Claude Code