Skip to content

enable beatloop activation directly after activating a hotcue#2213

Merged
daschuer merged 2 commits intomixxxdj:masterfrom
goddisignz:cue_loop
Aug 1, 2019
Merged

enable beatloop activation directly after activating a hotcue#2213
daschuer merged 2 commits intomixxxdj:masterfrom
goddisignz:cue_loop

Conversation

@goddisignz
Copy link
Copy Markdown
Contributor

Fixes https://bugs.launchpad.net/mixxx/+bug/1464003 .
Basically the loopcontroller has to set the loop start position to the queued seek position before activating the loop.
Furthermore, when a hotcue that is at the end of an active loop is activated, also disable the loop instead of jumping back to the loop entry.
Replaces #2190 together with #2209

Comment thread src/engine/enginebuffer.cpp Outdated
@Holzhaus Holzhaus mentioned this pull request Jul 31, 2019
5 tasks
@daschuer
Copy link
Copy Markdown
Member

I am unsure about the last commit. It combines the final missing change to merge this with a new topic, of fixing https://bugs.launchpad.net/mixxx/+bug/1752133.
Would you mind to force push only the fix and we can merge this?

The change of the quantize behavior can go into a new PR. Currently I am used to the old behavior. So it probably needs a bit testing to accept that.

@goddisignz
Copy link
Copy Markdown
Contributor Author

I didn't know, there was already a discussion about that quantize loop matter.
When the previous beat is chosen is instead of the closest, the loop may end up one beat before the hotcue when the seek is already processed before the beatloop slot is called.
This happens often (or most of the time) when one uses the keyboard to press a hotcue and enable a beatloop directly afterwards. But it still might happen using the controller script when someone has a really fast CPU (mine does not seem to be able to cause this case).

I would need to do more calculation to prevent this, if requested.
I thought the previous beat was chosen to avoid causing a seek and therewith disabling the loop by accident, which would not have happended anymore due to the new inside active loop positioning.

@daschuer
Copy link
Copy Markdown
Member

daschuer commented Aug 1, 2019

Thank you. LGTM, waiting for CI.

@daschuer
Copy link
Copy Markdown
Member

daschuer commented Aug 1, 2019

I didn't get the fast CPU argument here. I think this should be solved after this is merged.

I think the real issue is a not exact quantized Cue point. This can happen due beatgrid adjustments or the transition of Mixxx from a interger to a floating point transport.

Let's discuss it at Launchpad.

@daschuer
Copy link
Copy Markdown
Member

daschuer commented Aug 1, 2019

The Travis failure on Mac os is unterelated.
Thank you for the fix.

@daschuer daschuer merged commit ef2548e into mixxxdj:master Aug 1, 2019
@daschuer
Copy link
Copy Markdown
Member

daschuer commented Aug 1, 2019

The issue from bug https://bugs.launchpad.net/mixxx/+bug/1752133
Is now clear for me.
I think a low impact solution would be to define a small snap region
Around the beat. The solution of using the nearest beat is also interesting.
I am curious to read other opinions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants