mixxx icon indicating copy to clipboard operation
mixxx copied to clipboard

DEBUG ASSERT Beats::iteratorFrom

Open mixxxbot opened this issue 2 years ago • 2 comments

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 avatar Aug 23 '22 02:08 mixxxbot

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


In the assertion case

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

mixxxbot avatar Aug 23 '22 02:08 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)

mixxxbot avatar Aug 23 '22 02:08 mixxxbot

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

ronso0 avatar Aug 10 '23 22:08 ronso0

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

Swiftb0y avatar Aug 10 '23 23:08 Swiftb0y

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

daschuer avatar Aug 12 '23 16:08 daschuer

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 avatar Sep 24 '23 11:09 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.

m0dB avatar Sep 24 '23 12:09 m0dB

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

daschuer avatar Sep 24 '23 17:09 daschuer

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.

m0dB avatar Sep 25 '23 15:09 m0dB

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]	

grafik

grafik

grafik

grafik

grafik

JoergAtGithub avatar Nov 02 '23 19:11 JoergAtGithub

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

ywwg avatar Nov 22 '23 20:11 ywwg

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

daschuer avatar Dec 06 '23 16:12 daschuer

It seems the real fix is: https://github.com/mixxxdj/mixxx/pull/13150

JoergAtGithub avatar Apr 21 '24 13:04 JoergAtGithub