Skip to content

Fix some race conditions around loops and beatloops.#937

Merged
rryan merged 3 commits into
mixxxdj:masterfrom
daschuer:loop_race
Jun 5, 2016
Merged

Fix some race conditions around loops and beatloops.#937
rryan merged 3 commits into
mixxxdj:masterfrom
daschuer:loop_race

Conversation

@daschuer
Copy link
Copy Markdown
Member

@daschuer daschuer commented May 1, 2016

This fixes the underlying issue of
https://bugs.launchpad.net/mixxx/+bug/1576819

This fixes also some, but not all race conditions of
https://bugs.launchpad.net/mixxx/+bug/1212952

To fix ll race conditions, we need to rework all code.

@rryan
Copy link
Copy Markdown
Member

rryan commented May 6, 2016

Tests seem to segfault:

�[0;32m[ RUN      ] �[mLoopingControlTest.LoopResizeSeek
Loading resources from  "/home/travis/build/mixxxdj/mixxx/res/" 
ConfigObject: Could not read "/home/travis/build/mixxxdj/mixxx/src/test/test_data/test.cfg" 
Compressor attack per frame:  0.000408163 decay per frame:  4.08163e-05 
ControlDoublePrivate::getControl returning NULL for ( "[Master]" , "guiTickTime" ) 
ControlDoublePrivate::getControl returning NULL for ( "[Master]" , "guiTick50ms" ) 
ControlDoublePrivate::getControl returning NULL for ( "[Master]" , "guiTickTime" ) 
ControlDoublePrivate::getControl returning NULL for ( "[Master]" , "guiTick50ms" ) 
ControlDoublePrivate::getControl returning NULL for ( "[Master]" , "guiTickTime" ) 
ControlDoublePrivate::getControl returning NULL for ( "[Master]" , "guiTick50ms" ) 
ControlDoublePrivate::getControl returning NULL for ( "[Master]" , "guiTickTime" ) 
ControlDoublePrivate::getControl returning NULL for ( "[Master]" , "guiTick50ms" ) 
ControlDoublePrivate::getControl returning NULL for ( "[Master]" , "guiTickTime" ) 
ControlDoublePrivate::getControl returning NULL for ( "[Master]" , "guiTick50ms" ) 
ControlDoublePrivate::getControl returning NULL for ( "[Master]" , "guiTickTime" ) 
ControlDoublePrivate::getControl returning NULL for ( "[Master]" , "guiTick50ms" ) 
ControlObject "[PreviewDeck1]" "input_configured" already created 
ControlObject "[PreviewDeck1]" "passthrough" already created 
ControlObject "[PreviewDeck1]" "pregain" already created 
ControlObject "[PreviewDeck1]" "replaygain" already created 
ControlObject "[PreviewDeck1]" "play" already created 
ControlObject "[PreviewDeck1]" "loop_start_position" already created 
ControlObject "[PreviewDeck1]" "loop_end_position" already created 
ControlObject "[PreviewDeck1]" "vinylcontrol_status" already created 
ControlObject "[PreviewDeck1]" "vinylcontrol_enabled" already created 
ControlObject "[PreviewDeck1]" "file_bpm" already created 
ControlObject "[PreviewDeck1]" "file_key" already created 
ControlDoublePrivate::getControl returning NULL for ( "[Master]" , "guiTickTime" ) 
ControlDoublePrivate::getControl returning NULL for ( "[Master]" , "guiTick50ms" ) 
ControlDoublePrivate::getControl returning NULL for ( "[Master]" , "guiTickTime" ) 
ControlDoublePrivate::getControl returning NULL for ( "[Master]" , "guiTick50ms" ) 
/home/travis/build.sh: line 45:  9955 Segmentation fault      (core dumped) ./mixxx-test

@daschuer
Copy link
Copy Markdown
Member Author

daschuer commented May 7, 2016

Program received signal SIGSEGV, Segmentation fault.
clearActiveBeatLoop (this=0x3282ca0) at src/engine/loopingcontrol.cpp:710
710 m_pActiveBeatLoop->deactivate();
(gdb) bt
#0 clearActiveBeatLoop (this=0x3282ca0) at src/engine/loopingcontrol.cpp:710
#1 LoopingControl::slotLoopEndPos (this=this@entry=0x3282ca0, pos=600)
at src/engine/loopingcontrol.cpp:613
#2 0x00000000008090f2 in LoopingControl::qt_static_metacall (_o=0x3282ca0,
_c=, _id=5, _a=0x7fffffffdb00)
at lin64_build/engine/moc_loopingcontrol.cc:85
#3 0x00007ffff50e887a in QMetaObject::activate (sender=0x3727ce0,
m=m@entry=0x1019580 ControlObject::staticMetaObject,
local_signal_index=local_signal_index@entry=0,
argv=argv@entry=0x7fffffffdb00) at kernel/qobject.cpp:3539
#4 0x000000000050dffe in ControlObject::valueChanged (this=,

@daschuer
Copy link
Copy Markdown
Member Author

daschuer commented May 7, 2016

Segfault fixed

rryan
rryan reviewed May 23, 2016
View reviewed changes
Comment thread src/engine/loopingcontrol.cpp
if (loopSamples.end != kNoTrigger &&
(loopSamples.end - pos) < MINIMUM_AUDIBLE_LOOP_SIZE) {
if (closestBeat != -1 && m_pBeats) {
pos = static_cast<int>(floor(m_pBeats->findNthBeat(closestBeat, -2)));
Copy link
Copy Markdown
Member

@rryan rryan May 23, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any particular reason for the change in logic here? (looking backward -2 instead of forward by 2)

Would this work with a beatmap positioned at the start of the song?

Copy link
Copy Markdown
Member

@rryan rryan May 23, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also -- same question here regarding no longer using s_dBeatSizes[0].

Copy link
Copy Markdown
Member Author

@daschuer daschuer May 23, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same here, 1/8 beats is only a guess and in most cases wrong.

Copy link
Copy Markdown
Member Author

@daschuer daschuer May 23, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any particular reason for the change in logic here? (looking backward -2 instead of forward by 2)

The old code was wrong for beat maps since it looks forward for the next beat but moves the loop in point back by
the amount of frames. Now we really hit the beat before.

Would this work with a beatmap positioned at the start of the song?

No, you cannot set a quantization loop in the region outside the beatmap. In this case we hit the condition below and get a minimum loop.

Copy link
Copy Markdown
Member

@rryan rryan Jun 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense -- thanks.

@rryan
Copy link
Copy Markdown
Member

rryan commented May 23, 2016

To fix all race conditions, we need to rework all code.

Ain't that the truth! Thanks for working on this.

@rryan
Copy link
Copy Markdown
Member

rryan commented Jun 5, 2016

LGTM -- thanks for the fixes!

@rryan rryan merged commit 48ebeca into mixxxdj:master Jun 5, 2016
@rryan rryan changed the title Loop race Fix some race conditions around loops and beatloops. Jun 5, 2016
@daschuer daschuer deleted the loop_race branch December 17, 2018 17:59
@mixxxbot mixxxbot mentioned this pull request Aug 22, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@daschuer @rryan