DEBUG ASSERT Beats::iteratorFrom

[mixxxbot]

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

[mixxxbot]

Commented by: daschuer
Date: 2021-12-28T23:02:41Z

---

In the assertion case

*it = 32170
position = 15616
beatLengthFrames = 16554 at position 15616

[mixxxbot]

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)

[ronso0]

Did this happen again?
Can't reproduce with setting loop_in before existing loop (#11665)

[Swiftb0y]

I also tried reproducing this some time ago and I couldn't but judging from the initial posts the issue is sporadic by nature...

[daschuer]

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

[m0dB]

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?

[m0dB]

@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.

[daschuer]

I have not yet found a pattern to reproduce it. I can try.

[m0dB]

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.

[JoergAtGithub]

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]	

[ywwg]

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

[daschuer]

I assume this was a symptom #12071 at least I cannot reproduce it anymore.
Pending is #12368, a final clean up PR

[JoergAtGithub]

It seems the real fix is: #13150