CueControl: Fix previewing regression from PR #2194

[Holzhaus]

No description provided.

[Holzhaus]

@daschuer Please verify if this fixes the issue reported in https://bugs.launchpad.net/mixxx/+bug/1903002.

[daschuer]

It is still there. I experienced now also the issue when jumping back ward.
Since it happens only rarely in 1 of 10 attempts or so, I am afraid it is a timing issue or race-condition.

[daschuer]

Are you able to reproduce it?

[Holzhaus]

Are you able to reproduce it?

Yes, sometimes the m_iCurrentlyPlayingHotcues semaphore is not updated properly:

warning [Main] CueControl::hotcueActivatePreview 1 0
warning [Main] CueControl::hotcueActivatePreview 0 0

[Holzhaus]

I'm pretty sure m_iCurrentlyPreviewingHotcues should be an atomic. It's also modified by updateIndicatorsAndModifyPlay which is called from the engine thread.

[daschuer]

Uh that is problematic, because sometimes the m_iCurrentlyPreviewingHotcues is test to 0 because of an earlier check of the same value. A Atomic allwn will not help. Too bad.

[daschuer]

We need to refactor isPlayingByPlayButton() to something like.
checkAndResetPreviewing() to allow atomic reset of m_iCurrentlyPreviewingHotcue

[Holzhaus]

Here's some log output:

Apparently the issue is in the 3 block where updateIndicatorsAndModifyPlay is called with first argument set to false instead of true.

debug [Main] keyboard press:  "C"
warning [Main] CueControl::hotcueActivatePreview 1 0
warning [Main] updateIndicatorsAndModifyPlay true true 1 false
warning [Engine] updateIndicatorsAndModifyPlay true true 1 false
warning [Main] CueControl::hotcueActivatePreview 0 1
warning [Main] m_iCurrentlyPreviewingHotcues 0
warning [Main] updateIndicatorsAndModifyPlay false true 0 false
warning [Engine] updateIndicatorsAndModifyPlay false true 0 false

debug [Main] keyboard press:  "Z"
warning [Main] CueControl::hotcueActivatePreview 1 0
warning [Main] updateIndicatorsAndModifyPlay true true 1 false
warning [Engine] updateIndicatorsAndModifyPlay true true 1 false
warning [Main] CueControl::hotcueActivatePreview 0 1
warning [Main] m_iCurrentlyPreviewingHotcues 0
warning [Main] updateIndicatorsAndModifyPlay false true 0 false
warning [Engine] updateIndicatorsAndModifyPlay false true 0 false

debug [Main] keyboard press:  "C"
warning [Main] CueControl::hotcueActivatePreview 1 0
warning [Main] updateIndicatorsAndModifyPlay true true 1 false
warning [Engine] updateIndicatorsAndModifyPlay false true 1 false
warning [Engine] m_iCurrentlyPreviewingHotcues = 0
warning [Main] CueControl::hotcueActivatePreview 0 0

[Holzhaus]

I have a hunch that this is caused by EngineBuffer::slotControlPlayRequest being triggered after the EngineBuffer::setNewPlayPos is set in the engine thread sometimes.

[Holzhaus]

Let's merge this, as it fixes an issue when pressing 2 hotcue buttons concurrently. The other issue is also present in the 2.3 branch and also happens with regular cues, so it was not caused by #2194.

[daschuer]

OK, LGTM

[daschuer]

I can confirm that 2.3 is also affected by the issue. When this happens the hotcue_X_enabled control also sticks with 1, which is somehow correct :-/

[daschuer]

I got it.

It it:

seekAbs(position);
m_pPlay->set(1.0);

It happens that the engine with higher priority processes the seek before the play is set.
In this case the pause state is verified and treated as a latching.
If we swap the commands, the engine may start playing at the old position which should be also avoided.

I will issue a PR.

[Holzhaus]

I did swap it locally but I could still reproduce the issue IIRC. Worth a try anyway.

[daschuer]

Swapping does not work:
This works for me: #3268