fix assertion in Lp1839669 #2235

daschuer · 2019-08-11T13:02:14Z

This should fix a https://bugs.launchpad.net/mixxx/+bug/1839669.
Unfortunately I can't reproduce the bug by loading a first track only.

…ffer::updateIndicators. Fixes lp1839669.

ywwg

Fix looks good, just a few comment nits for typos. Thanks!

ywwg · 2019-08-12T13:49:13Z

src/engine/enginebuffer.cpp

+    double ratio = m_tempo_ratio_old;  
+    if (ratio == 0.0) {
+	// In case the track is slowed done to zero we will have INF relaining seconds
+        // We jump back to a rate of 1.0 to show a useful time. 


inconsistent tabbing in this block

fix typo: "In case the track is slowed done to zero we will have INF remaining seconds.
// We jump back to a rate of 1.0 to show a useful time."

@daschuer Ping

ywwg · 2019-08-12T13:50:35Z

src/engine/enginebuffer.cpp

-        // no track loaded, function not called in this case
-        return;
+    if (!m_trackSampleRateOld) {
+       // this happens if Deck Passthrough is active but no track is loaded


punctuation: "This happens if Deck Passthrough is active but no track is loaded. We skip indicator updates."

@daschuer Ping

This should also be fixed.

ywwg · 2019-08-12T13:51:08Z

src/util/duration.cpp

-    if (dSeconds < 0.0) {
-        // negative durations are not supported
+    if (dSeconds < 0.0 || !isfinite(dSeconds)) {
+        // negative durations and isnane high values are not supported


isnane -> isNaN

@daschuer typo still not fixed

I cannot find this anymore.

daschuer · 2019-08-12T23:30:46Z

Done, thank you for review.

src/util/duration.cpp

uklotzde

Test fails because the special case "?" is not handled.

uklotzde · 2019-09-09T22:27:16Z

@daschuer Ping

uklotzde · 2019-10-20T08:30:51Z

@daschuer Ping again

daschuer · 2019-10-21T22:00:04Z

Thank you for the remainder. Done!

uklotzde · 2019-10-22T21:11:13Z

Please merge master and push again. I still see test failures in the AppVeyor Windows build.

uklotzde · 2019-10-27T06:59:44Z

@daschuer Ping

uklotzde · 2019-10-27T11:23:27Z

I'm experiencing another debug assertion. Not sure it is a follow-up error that will be fixed by this PR:

Critical [CachingReaderWorker 2]: DEBUG ASSERT: "even(static_cast<int>(currentClosestBeat))" in function void QuantizeControl::updateClosestBeat(double) at /tmp/mixxx/src/engine/controls/quantizecontrol.cpp:122
Critical [Engine]: DEBUG ASSERT: "m_trackSampleRateOld && m_tempo_ratio_old" in function void EngineBuffer::updateIndicators(double, int) at /tmp/mixxx/src/engine/enginebuffer.cpp:1205

#0  0x00007ffff63e8af5 in raise () at /lib64/libpthread.so.0
#1  0x000000000065a519 in mixxx::(anonymous namespace)::MessageHandler(QtMsgType, QMessageLogContext const&, QString const&) (type=<optimized out>, input=...)
    at /tmp/mixxx/src/util/logging.cpp:133
#2  0x00007ffff41e6321 in qt_message_print(QtMsgType, QMessageLogContext const&, QString const&) () at /lib64/libQt5Core.so.5
#3  0x00007ffff41e6439 in qt_message(QtMsgType, QMessageLogContext const&, char const*, __va_list_tag*) () at /lib64/libQt5Core.so.5
#4  0x00007ffff41b8712 in QMessageLogger::critical(char const*, ...) const () at /lib64/libQt5Core.so.5
#5  0x00000000004e59da in mixxx_debug_assert(char const*, char const*, int, char const*)
    (function=0x109d348 "void QuantizeControl::updateClosestBeat(double)", line=122, file=0x109d310 "/tmp/mixxx/src/engine/controls/quantizecontrol.cpp", assertion=0x109d378 "even(static_cast<int>(currentClosestBeat))") at /usr/include/qt5/QtCore/qlogging.h:91
#6  0x00000000004e59da in mixxx_maybe_debug_assert_return_true(char const*, char const*, int, char const*)
    (function=0x109d348 "void QuantizeControl::updateClosestBeat(double)", line=122, file=0x109d310 "/tmp/mixxx/src/engine/controls/quantizecontrol.cpp", assertion=0x109d378 "even(static_cast<int>(currentClosestBeat))") at /tmp/mixxx/src/util/assert.h:16
#7  0x00000000004e59da in QuantizeControl::updateClosestBeat(double) (this=0x2b424e0, dCurrentSample=<optimized out>) at /tmp/mixxx/src/engine/controls/quantizecontrol.cpp:122
#8  0x00000000010b72d2 in  ()
#9  0x00007fff31bcc0f0 in  ()
#10 0x000000000096aeef in QuantizeControl::trackLoaded(std::shared_ptr<Track>) (this=0x2b424e0, pNewTrack=...) at /tmp/mixxx/src/engine/controls/quantizecontrol.cpp:49
#11 0x000000000097ec3b in EngineBuffer::notifyTrackLoaded(std::shared_ptr<Track>, std::shared_ptr<Track>) (this=0x21b2320, pNewTrack=
    std::shared_ptr<Track> (use count 29, weak count 1) = {...}, pOldTrack=std::shared_ptr<Track> (empty) = {...}) at /usr/include/c++/9/ext/atomicity.h:96
#12 0x000000000097f181 in EngineBuffer::slotTrackLoaded(std::shared_ptr<Track>, int, int) (this=0x21b2320, pTrack=..., iTrackSampleRate=44100, iTrackNumSamples=15213218)

The actual values upon loading a new track:

Debug [CachingReaderWorker 2]: dCurrentSample 0 prevBeat -8703.79 nextBeat 26112 closestBeat -1 currentClosestBeat -8703.79

Looks like uninitialized or bogus values.

uklotzde · 2019-10-27T11:26:10Z

I didn't notice those issues recently, because debug assertions were unintentionally disabled.

uklotzde · 2019-10-27T11:28:04Z

It happens for various files, different formats, at different positions. I will revert back from a CMake build to an SCons build and test again. Update: Issues are independent of the build system.

uklotzde · 2019-10-27T12:33:30Z

Caused by #2196

daschuer · 2019-11-15T08:37:17Z

Fix was merged in #2334
Merge after CI has passed?

uklotzde · 2019-11-15T11:52:06Z

Still test failures

daschuer · 2019-11-18T23:57:18Z

On my system the test runs without any issues.

Strange the first works the second not.
EXPECT_EQ(false, isfinite(std::numeric_limits::infinity()));
EXPECT_EQ(false, __builtin_isfinite(std::numeric_limits::infinity()));

Anyway I need to guard for qint64 max es well. Hopefully the test go though then.

daschuer · 2019-11-19T21:15:10Z

Finally all tests succeed @ywwg @uklotzde anything else?

Holzhaus · 2019-11-21T14:48:38Z

Builds, all tests pass and the debug assertion no longer occurs when trying to load a track into a deck with passthrough enabled. LGTM.

A bit OT: We should really try to improve the passthough UX. I had this issue a while back when the "CH 1 INPUT" hardware switch on my controller was accidently set to "LINE" (which enables passthrough for that deck). I didn't realize that at first and wondered for ~15 minutes why I can load a track into that deck but Mixxx refuses to play it. Can we disable the deck completely (no track loading, grey out all play/pause/cue buttons, replace waveform with a) a live waveform from the input stream or b) a overlay that says "Passthrough")?

uklotzde · 2019-11-21T15:25:58Z

LGTM

Ping @ywwg

daschuer

This should be ready to merged, or did I miss a comment?

uklotzde · 2019-12-09T18:48:58Z

No veto on ping, so let's merge this now!

daschuer added 2 commits August 11, 2019 14:27

Replace assertrion with a commented handling of edge case in EngineBu…

bc479b7

…ffer::updateIndicators. Fixes lp1839669.

Guard formatTime() against INF and added a test.

6f11f9d

ywwg requested changes Aug 12, 2019

View reviewed changes

Improve comments

f4dc231

uklotzde reviewed Aug 15, 2019

View reviewed changes

src/util/duration.cpp Outdated Show resolved Hide resolved

uklotzde added the engine label Sep 1, 2019

uklotzde added this to the 2.3.0 milestone Sep 1, 2019

uklotzde added the minor bug label Sep 1, 2019

uklotzde suggested changes Sep 1, 2019

View reviewed changes

Be-ing mentioned this pull request Sep 30, 2019

Fix race conditions between caching reader and worker (again) #2308

Closed

Considder special case of invalid time expressed as "?"

d86b175

Merge remote-tracking branch 'upstream/master' into lp1839669

93a125b

Merge remote-tracking branch 'upstream/master' into lp1839669

a026399

Guard also against qint64 overflow

99033ef

daschuer force-pushed the lp1839669 branch from 7467e4f to 99033ef Compare November 18, 2019 23:59

uklotzde approved these changes Nov 21, 2019

View reviewed changes

daschuer commented Nov 29, 2019

View reviewed changes

uklotzde merged commit 80ec75a into mixxxdj:master Dec 9, 2019

daschuer deleted the lp1839669 branch September 26, 2021 17:41

mixxxbot mentioned this pull request Aug 23, 2022

Debug assertion when loading 1st track #9710

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix assertion in Lp1839669 #2235

fix assertion in Lp1839669 #2235

daschuer commented Aug 11, 2019

ywwg left a comment

ywwg Aug 12, 2019

ywwg Aug 12, 2019

uklotzde Nov 29, 2019

daschuer Nov 29, 2019

ywwg Aug 12, 2019

uklotzde Nov 29, 2019

daschuer Nov 29, 2019

ywwg Aug 12, 2019

uklotzde Nov 29, 2019

daschuer Nov 29, 2019

daschuer commented Aug 12, 2019

uklotzde left a comment

uklotzde commented Sep 9, 2019

uklotzde commented Oct 20, 2019

daschuer commented Oct 21, 2019

uklotzde commented Oct 22, 2019

uklotzde commented Oct 27, 2019

uklotzde commented Oct 27, 2019

uklotzde commented Oct 27, 2019

uklotzde commented Oct 27, 2019 •

edited

Loading

uklotzde commented Oct 27, 2019

daschuer commented Nov 15, 2019

uklotzde commented Nov 15, 2019

daschuer commented Nov 18, 2019

daschuer commented Nov 19, 2019

Holzhaus commented Nov 21, 2019

uklotzde commented Nov 21, 2019

daschuer left a comment

uklotzde commented Dec 9, 2019

fix assertion in Lp1839669 #2235

fix assertion in Lp1839669 #2235

Conversation

daschuer commented Aug 11, 2019

ywwg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daschuer commented Aug 12, 2019

uklotzde left a comment

Choose a reason for hiding this comment

uklotzde commented Sep 9, 2019

uklotzde commented Oct 20, 2019

daschuer commented Oct 21, 2019

uklotzde commented Oct 22, 2019

uklotzde commented Oct 27, 2019

uklotzde commented Oct 27, 2019

uklotzde commented Oct 27, 2019

uklotzde commented Oct 27, 2019 • edited Loading

uklotzde commented Oct 27, 2019

daschuer commented Nov 15, 2019

uklotzde commented Nov 15, 2019

daschuer commented Nov 18, 2019

daschuer commented Nov 19, 2019

Holzhaus commented Nov 21, 2019

uklotzde commented Nov 21, 2019

daschuer left a comment

Choose a reason for hiding this comment

uklotzde commented Dec 9, 2019

uklotzde commented Oct 27, 2019 •

edited

Loading