mixxx
mixxx copied to clipboard
DEBUG ASSERT Beats::iteratorFrom
Reported by: daschuer Date: 2021-12-28T22:42:28Z Status: New Importance: Critical Launchpad Issue: lp1955929
I have just experienced Debug Assert violation in current main branch.
critical Engine DEBUG ASSERT: "it == cbegin() || it == cend() || *it - position < std::prev(it).beatLengthFrames()" in function mixxx::Beats::ConstIterator mixxx::Beats::iteratorFrom(mixxx::audio::FramePos) const at /home/daniel/workspace/2.3/src/track/beats.cpp:447
This happens after pressing play with a loaded Track, called from Beats::findPrevNextBeats
Commented by: daschuer Date: 2021-12-28T23:02:41Z
In the assertion case
*it = 32170 position = 15616 beatLengthFrames = 16554 at position 15616
Commented by: uklotzde Date: 2022-06-23T19:10:18Z
Just happened once while pre-listening some tracks. Probably right after loading and trying to play a track. Forget to inspect the actual values.
mixxxdj/mixxx#4916 0x00000000007a6394 in mixxx::Beats::iteratorFrom(mixxx::audio::FramePos) const (this=this@entry=0x262a6e0, position=...)
at /home/uk/volumes/Build/cpp/mixxx/src/track/beats.cpp:450
mixxxdj/mixxx#4917 0x00000000007a4463 in mixxx::Beats::findPrevNextBeats(mixxx::audio::FramePos, mixxx::audio::FramePos*, mixxx::audio::FramePos*, bool) const (this=0x262a6e0, position=..., prevBeatPosition=0x7fffaf7fc700, nextBeatPosition=0x2, snapToNearBeats=false)
at /home/uk/volumes/Build/cpp/mixxx/src/track/beats.cpp:382
mixxxdj/mixxx#4918 0x0000000000b0dc4f in BpmControl::getBeatContext(std::shared_ptr<mixxx::Beats const> const&, mixxx::audio::FramePos, mixxx::audio::FramePos*, mixxx::audio::FramePos*, double*, double*) (pBeats=warning: RTTI symbol not found for class 'std::_Sp_counted_ptr<mixxx::Beats*, (__gnu_cxx::_Lock_policy)2>'
warning: RTTI symbol not found for class 'std::_Sp_counted_ptr<mixxx::Beats*, (__gnu_cxx::_Lock_policy)2>'
std::shared_ptr<const mixxx::Beats> (use count 7, weak count 0) = {...}, position=..., pBeatLengthFrames=0x7fffaf7fc788, pBeatPercentage=0x7fffaf7fc768, pPrevBeatPosition=<optimized out>, pNextBeatPosition=<optimized out>)
at /home/uk/volumes/Build/cpp/mixxx/src/engine/controls/bpmcontrol.cpp:495
mixxxdj/mixxx#4919 BpmControl::getBeatMatchPosition(mixxx::audio::FramePos, bool, bool)
(this=0x13a8400, thisPosition=..., respectLoops=true, playing=true)
at /home/uk/volumes/Build/cpp/mixxx/src/engine/controls/bpmcontrol.cpp:816
mixxxdj/mixxx#4920 0x000000000092e0b7 in EngineBuffer::processSeek(bool) (this=this@entry=0x1fd6d10, paused=<optimized out>)
at /home/uk/volumes/Build/cpp/mixxx/src/engine/enginebuffer.cpp:1286
mixxxdj/mixxx#4921 0x000000000092cc14 in EngineBuffer::processTrackLocked(float*, int, mixxx::audio::SampleRate)
(this=this@entry=0x1fd6d10, pOutput=pOutput@entry=0x7fff87d8c040, iBufferSize=iBufferSize@entry=2048, sampleRate=...)
at /home/uk/volumes/Build/cpp/mixxx/src/engine/enginebuffer.cpp:847
mixxxdj/mixxx#4922 0x000000000092ec8d in EngineBuffer::process(float*, int) (this=0x1fd6d10, pOutput=0x7fff87d8c040, iBufferSize=2048)
Did this happen again? Can't reproduce with setting loop_in before existing loop (#11665)
I also tried reproducing this some time ago and I couldn't but judging from the initial posts the issue is sporadic by nature...
It has just happen again with the current 2.4 branch:
critical [Engine] DEBUG ASSERT: "it == cbegin() || it == cend() || (*it >= position && *std::prev(it) < position)" in function mixxx::Beats::ConstIterator mixxx::Beats::iteratorFrom(mixxx::audio::FramePos) const at /home/daniel/workspace/mixxx2/src/track/beats.cpp:455
Since the failing assert
it == cbegin() || it == cend() || (*it >= position && *std::prev(it) < position
is preceeded by this assert
it == cbegin() || it == cend() || *it >= position
we know that it >= position, so we could also write the failing assert as
it == cbegin() || it == cend() || *it > *std::prev(it)
I have been looking at the code to try to understand how it could ever occur that *it > *std::prev(it)
doesn't hold true, but I don't see it. We could add some more asserts in the ConstIterator += and -= operators, that might help to trace it down.
Now, I have never been able to reproduce this myself...
Also I wonder, is this really a "party stopper"? What happens if you continue despite this condition not holding true?
@daschuer do you have a way to reproduce this? Can you try with the additional asserts? In any case, those additional asserts make sense so I propose to merge the PR anyway.
I have not yet found a pattern to reproduce it. I can try.
Hopefully the added asserts will give us a bit more info. I find this code quite obscure, particularly the min int and max int trickery in the ConstIterator and the confusing use of an iterator inside the ConstIterator.
I just had the new assert on a 2.5 build on Windows11:
"DEBUG ASSERT: "it == cbegin() || it == cend() || *it >= position" in function class mixxx::Beats::ConstIterator __cdecl mixxx::Beats::iteratorFrom(class mixxx::audio::FramePos) const at C:\Users\Joerg..."
Qt6Core.dll!00007fffd65b5438() Unknown
Qt6Core.dll!00007fffd65b9563() Unknown
> mixxx.exe!mixxx::`anonymous namespace'::handleMessage(QtMsgType type=QtCriticalMsg, const QMessageLogContext & context={...}, const QString & input={...}) Line 358 C++
[External Code]
[Inline Frame] mixxx.exe!mixxx_debug_assert(const char *) Line 9 C++
mixxx.exe!mixxx::Beats::iteratorFrom(mixxx::audio::FramePos position) Line 465 C++
mixxx.exe!mixxx::Beats::findPrevNextBeats(mixxx::audio::FramePos position, mixxx::audio::FramePos * prevBeatPosition=0x000000f68f6fed60, mixxx::audio::FramePos * nextBeatPosition=0x000000f68f6fed70, bool snapToNearBeats=true) Line 394 C++
mixxx.exe!QuantizeControl::lookupBeatPositions(mixxx::audio::FramePos position) Line 97 C++
mixxx.exe!QuantizeControl::playPosChanged(mixxx::audio::FramePos position) Line 86 C++
mixxx.exe!QuantizeControl::setFrameInfo(mixxx::audio::FramePos currentPosition, mixxx::audio::FramePos trackEndPosition, mixxx::audio::SampleRate sampleRate={...}) Line 67 C++
mixxx.exe!EngineBuffer::processTrackLocked(float * pOutput=0x4135e540318c6319, const int iBufferSize=0x00000200, mixxx::audio::SampleRate sampleRate) Line 1100 C++
mixxx.exe!EngineBuffer::process(float * pOutput=0x000002bcbb2e4100, const int iBufferSize) Line 1161 C++
mixxx.exe!EngineDeck::process(float * pOut=0x000002bcbb2e4100, const int iBufferSize) Line 66 C++
mixxx.exe!EngineMixer::processChannels(int iBufferSize=0x00000200) Line 377 C++
mixxx.exe!EngineMixer::process(const int iBufferSize) Line 429 C++
mixxx.exe!SoundDevicePortAudio::callbackProcessClkRef(const __int64 framesPerBuffer=0x0000000000000100, float * out=0x000002bcd8307380, const float * in=0x0000000000000000, const PaStreamCallbackTimeInfo * timeInfo=0x000000f68f6ff8a8, unsigned long statusFlags=0x00000000) Line 993 C++
mixxx.exe!`anonymous namespace'::paV19CallbackClkRef(const void * inputBuffer, void * outputBuffer, unsigned long framesPerBuffer, const PaStreamCallbackTimeInfo * timeInfo, unsigned long statusFlags=0x00000000, void * soundDevice=0x000002bcbacb4f40) Line 74 C++
[External Code]
Is this a situation where we can add a fallback behavior to avoid the assert, just for 2.4 so we can unblock the release? I have not seen any discussion that this is causing problems with playback/performance in release mode
I assume this was a symptom https://github.com/mixxxdj/mixxx/issues/12071 at least I cannot reproduce it anymore. Pending is https://github.com/mixxxdj/mixxx/pull/12368, a final clean up PR
It seems the real fix is: https://github.com/mixxxdj/mixxx/pull/13150