mixxx icon indicating copy to clipboard operation
mixxx copied to clipboard

[POC] add alternative Loop Cues recall mode: jump to and loop, toggle loop if in range

Open ronso0 opened this issue 2 years ago • 17 comments

This PR is supposed to bring the attention to this saved loops UX topic. It was touched in @Holzhaus's orginal PR https://github.com/mixxxdj/mixxx/pull/2194#issuecomment-520975024, and nPrevail filed lp:1966160.

new behaviour:

  • playposition anywhere outside loop: jump to cue and loop
  • inside loop: toggle

This is also my personal preference so I just opened this PR.

Question is: do we want to decide on a fixed implementation or make the behaviour a preferences switch?

  • fixed: loopcue and hotcue behaviour would be consistent (outside loop)
  • custom: users can adjust the behaviour (default is "goto and loop"?)

ronso0 avatar Jun 21 '22 07:06 ronso0

I actually like to think of the "loop cue" as a "hot cue" instead of an "active loop." I like to use hotcues to correct my beat matching, and I do that by triggering the hotcue to match the beat of the other deck. I typically keep triggering the hot cue until the first down beat matches the other deck.

I believe "toggling on/off a loop" when a deck is "inside the loop," and using the same hotcues button, will prevent one from correcting a beat match.

The benefit of the "loop" feature is to allow a "extended beat match," but I think hotcues become more important to "fix beat matching." I think it's easier to use a way to "activate/deactivate" toggle on a loop that's separate from hotcues.

I think what's great about a "saved hot cue loop" is that it'll instantly loop, stay looped, and act as a hot cue for fixing beat matching between more than one deck.

Just my thoughts!

nPrevail avatar Jun 21 '22 15:06 nPrevail

I deliberately implemented it this way because on a controller, you cannot see where the stored loop is without pressing the button. Hence, you cannot predict if this will cause an audible jump or not.

I mostly use saved loops for "catching" loops at the end of the track, so that I can take my time when doing a transition without having to worry that the track suddenly ends prematurely, so this change would basically make saved loops unusable for me, at least when you don't add a way to get the old behavior back:

My controller has a "saved loop mode" for the performance pads, where pressing a pad sets/toggles the loop and SHIFT+pad press enables + jumps to the loop (using hotcue_X_gotoandloop) (it works the same in Serato).

Holzhaus avatar Jun 21 '22 15:06 Holzhaus

I mostly use saved loops for "catching" loops at the end of the track, so that I can take my time when doing a transition without having to worry that the track suddenly ends prematurely,

I do the same thing you do as well. However, I also use saved loops to jump to the end of songs.

Sometimes I accidentally play a song that the crowd is not feeling, and it doesn't help that everyone has to hear the entire song. Instead of having to listen to the whole song and all 6 minutes, I play the saved loop that's near the very end/outro of a song, just to get to the basic percussion beat so I can quickly mix the next song and get things moving again. This helps me cut the track in 3 minutes.

I usually make this jump right after the chorus line, and before the next verse drops. This works a lot in R&B, Hip Hop, and sometimes soulful House (it can be a bit too "vocal" sometimes).

nPrevail avatar Jun 21 '22 18:06 nPrevail

My controller has a "saved loop mode" for the performance pads

I was using a Traktor s4 mk2 when I was testing out the saved hotcue loops.

nPrevail avatar Jun 21 '22 18:06 nPrevail

I do the same thing you do as well. However, I also use saved loops to jump to the end of songs.

Yes, I sometimes do that as well (although it's rare). You can already do this right now, by using the hotcue_X_gotoandloop CO.

My point is that right now, both use cases are possible. With this change, only second use case (jumping) will be possible, the other one will require some elaborate Javascript that switches between hotcue_X_setloop and hotcue_X_cueloop COs depending on context.

Holzhaus avatar Jun 21 '22 18:06 Holzhaus

I haven't used the Saved loops feature personally, but I do agree with @Holzhaus. The current usecase makes more sense. I have nothing against @ronso0's usecase, but I think the issue is controversial enough that we should make this behavior a user-configurable choice.

Swiftb0y avatar Jun 21 '22 19:06 Swiftb0y

btw: I tested some stuff in my script and hotcue_X_type of loop cues always returns 0 (on main). @Holzhaus Could you take a look?

Edit seems this was some weird cmake caching issue, even though I cleared the cache. After a restart it works as expected :shrug:

ronso0 avatar Jun 22 '22 01:06 ronso0

Thanks for your explanations!

After a bit of "real" testing I think this would allow both use cases:

  • add a preferences option to switch between current and alternative behaviour:
    • click activates loop (no jump) | Shift + click jumps and loops
    • click jumps and loops | Shift + click activates loop (no jump)
    • both: Shift + longpress clears cue
  • GUI button acts accordingly, and still responds to right-click (opens cue menu) and Shift + right-click (clears cue)

So pretty much what @ywwg suggested in https://github.com/mixxxdj/mixxx/pull/2194#issuecomment-520975024

From what I can tell, changing the GUI button behaviour (at run time) will be the hardest thing since the connections are configured when the skin is created. When changing the preference we can either a) require a skin reload (like when changing the overview type, which btw happens without confirmation), or b) add changeLeftConnection()/changeRightConnection() to wbasewidget.cpp to change the connections at run time I prefer a. Not just because I'm lazy, more because atm I don't overlook all implications of b Anyway, for the Shift+click we should add WBaseWidget::addLeftShiftConnection in order to 1) not cram the logic into WHotcue button (incl. controlproxy and what not) and 2) make Shift+click available for other GUI controls.

And the tests need to be adjusted/extended, of course.

What do you think?

ronso0 avatar Jun 22 '22 12:06 ronso0

Why not add COs (hotcue_X_activate, hotcue_X_activate_secondary) which work differently depending on the user setting so you don't have to reconnect anything?

The advantage would be that the pad behavior would work the same on controllers. The disadvantage would be that we have more COs and that it becomes harder to explain how the controller mapping works in the manual, because it depends on user config.

Holzhaus avatar Jun 22 '22 14:06 Holzhaus

Why not add COs (hotcue_X_activate, hotcue_X_activate_secondary) which work differently depending on the user setting so you don't have to reconnect anything?

Sure :roll_eyes: I was so deep in the woods when writing this I didn't see the trees. Nevertheless we'd need a leftShiftConnection to allow the secondary action also in the GUI.

(until then consider this branch my personal implementation to "keep things rolling" and getting more used to saved loops)

ronso0 avatar Jun 22 '22 19:06 ronso0

This is now a somewhat working POC:

  • hotcue_X_activate:
    • if playing and not previewing
      • new: jumps to loop cue and loops, or toggles the loop if the playposition is inside the loop range
      • old: like reloop, activate loop without jumping, jump if loop is behind playpos
  • new hotcue_X_activate_secondary triggers the alternative of the selected behaviour
  • switch the new/old behaviour in Preferences > Decks > "Loop Cue Activation Mode"

~~I included the (unpolished) shiftLeftConncetion commit so this can be tested with official skins:~~ Hotcue button behaviour:

  • left-click a (set) loop cue button: triggers the selected behaviour
  • Shift + left-click: triggers the alternative

Let me know what you think!

ronso0 avatar Jun 24 '22 10:06 ronso0

The diff is broken, messed up rebase?

Holzhaus avatar Jun 24 '22 14:06 Holzhaus

What do you consider 'broken'?

To clarify: I'd like to get some feedback on the UX first. These commits are rather quick and dirty and there's definitely room for improvement, especially in the prefs. Also, I didn't double-check the wbasewidget/wpushbutton changes are free of regressions / work 100% with all types of pushbutton modes.

ronso0 avatar Jun 24 '22 20:06 ronso0

@Holzhaus maybe it was the DlgPrefDeck commit that held unrelated formatting that looked broken. I split the commits to be easier to digest.

ronso0 avatar Jun 24 '22 21:06 ronso0

What do you consider 'broken'?

For example, this PR downgrades some GitHub Actions dependencies and the CMake changes to the windows code signing seem unrelated as well.

Holzhaus avatar Jun 24 '22 22:06 Holzhaus

I am not sure what's wrong? I built (and just rebased) cleanly onto main image

ronso0 avatar Jun 24 '22 22:06 ronso0

Maybe a bug in the GitHub app. This is how it looks on mobile:

Screenshot_20220625_012542_com.github.android.jpg

Sorry for the noise then.

Holzhaus avatar Jun 24 '22 23:06 Holzhaus

This PR is marked as stale because it has been open 90 days with no activity.

github-actions[bot] avatar Oct 10 '22 00:10 github-actions[bot]

I believe "toggling on/off a loop" when a deck is "inside the loop," and using the same hotcues button, will prevent one from correcting a beat match.

@nPrevail If the deck is not playing (play button latched) the new mode now behaves exactly as hotcue_N_activate. This also works when keeping the button pressed (cue preview), as long as you don't reach the loop out marker (and then it shouldn't hurt, presuming the loop length is n beats).

ronso0 avatar Feb 12 '23 22:02 ronso0

I've rebased this onto main, and removed the WHotcueButton commit (which doesn't work as desired anyway).

Considering another request https://mixxx.discourse.group/t/help-with-the-hotcues-loop-behavior/29079/3, I think we should indeed make the alternative mode always jump to the cue (loop in) and leave loop toggling to the dedicated loop controls.

ronso0 avatar Feb 25 '24 16:02 ronso0

I think we should indeed make the alternative mode always jump to the cue (loop in) and leave loop toggling to the dedicated loop controls.

Hmm, downside of this that we can't activate a saved loop without jumping with the default GUI and mapping controls anymore:

  • playpos is inside a saved loop (inactive)
  • some other loop was active recently
  • = hotcue_N_activate would jump to loop_in
  • = reloop would activate the previous loop

There is hotcue_N_cueloop for this purpose but very likely no one has that mapped.

We could change reloop_toggle for this purpose, but that would break the original reloop use case. Hmm, would it? Actually, the documentation only says

Toggles the current loop on or off.

but we don't specify what 'current loop' means. I think it's unclear because it was written when we had only one loop.

Another potential source of confusion: which loop to choose if the current temp loop and the cue loop overlap at the current playposition? Which should be prioritized?

What about three modes:

  1. activate, jump if loop out is behind playpos, always toggle if in loop range (current)
  2. = 1. but also jump if loop is ahead
  3. always jump, always activate, use reloop_toggle to deactivate and to activate without jump if in loop range

ronso0 avatar Feb 25 '24 23:02 ronso0

So, 2. Is useful when we have no a reloop_toggle mapped, right? My idea was to use shift in that case. The GUI has always reloop_toggle For 3. also shift is required to not jump. GUI users can't activate the saved loop without jump, because the right click is already used for the context menu.

The question around which loop is used with reloop_toggle. I have no good idea.

How about this: 4. Activate loop. Jump if already activated. Use reloop_toggle for disabling.

I am personally happy with 1 but during testing, I noticed that jumping forward would be quite useful during preparation. In this case the controller with shift is out of reach.

daschuer avatar Feb 26 '24 06:02 daschuer

So, 2. Is useful when we have no a reloop_toggle mapped, right?

Both 1. and 2. work if reloop_toggle is not mapped, which was the intention in the first place I guess.

I think we should not rely on Shift, that would require re-mapping hotcue buttons or mix controller and keyboard/mouse. IMO it would be best to find a way to work with the default hotcue_N_activate mapping, but we can't have everything, only offer compromises, that's why I proposed 1., 2. and 3. to allow users to pick the mode that fits their mappings best

  1. activate, jump if loop out is behind playpos, always toggle if in loop range (current)
  2. = 1. but also jump if loop is ahead
  3. always jump, always activate, use reloop_toggle to deactivate and to activate without jump if in loop range

ronso0 avatar Feb 26 '24 13:02 ronso0

I pushed to last state just to have a working implementation

  • Activate loop, jump to if loop is behind
  • Jump to and loop, toggle if inside loop range

I'll take another look soonish how to get the best out of it. IMHO 4 modes is too much.

ronso0 avatar Feb 26 '24 13:02 ronso0

Two modes would be nice.

  1. Is a good established mode.

Problems:

  • Cue-ing inside the loop is not possible,
  • Jump forward is not possible.

Both are double assigned to "disable loop"

I can't imagine that 2. without the catching loop works good for many users. On the other hand, at least all GUI users have a reloop_toggle knob.

How about fix all at once with s single second mode:

  • like 1. but Jump to loop in when loop is enabled.

Disabling can be done with shift or reloop_toggle

daschuer avatar Feb 26 '24 14:02 daschuer

How about fix all at once with s single second mode:

* like 1. but Jump to loop in when loop is enabled.

Hmm, I thought we want to have a jump mode that behaves exactly like hotcues (always jump on first press).

ronso0 avatar Feb 26 '24 14:02 ronso0

Hmm, I thought we want to have a jump mode that behaves exactly like hotcues (always jump on first press).

Fair point. I was trying to find a compromise with two modes that still allows a catching loop. A related issue is, that we can't yet use reloop_toggle to enable a saved loop.

I try to summarize:

@ywwg said:

As for the dilemma of hotcue loop modes: default press should definitely jump-to-and-loop. I like shift-press as "load loop" without jump

@nPrevail said:

I like to use hotcues to correct my beat matching ...

@ronso0 said:

reloop_toggle may not mapped

I think we should not rely on Shift.

@daschuer said

I like to conditionally jump forward to a loop without a controller. (during preparation)

daschuer avatar Feb 26 '24 15:02 daschuer

  1. Is good for no reloop_toggle mapped, comes at the cost that jump is on shift

  2. has currently the issue that @nPrevail's re-cueing as is not possible.

So we may optimize the second mode for reloop_toggle mapped and met @nPrevail and @ywwg expectations, mine is more like nice to have.

  1. Always jump to loop-In, use shift or reloop_toggle for disabling.

daschuer avatar Feb 26 '24 15:02 daschuer

Yup, thanks for the summary!

jump is on shift

Not sure what you mean. In case you're referring to the GUI: there is no Shift anymore, I reverted the non-working WHotcueButton commit. I may take another stab at that at some point. = currently there is only the new hotcue_N_activate_secondary control.

ronso0 avatar Feb 26 '24 15:02 ronso0

jump is on shift

That just follows the assumption that hotcue_N_activate_secondary is mapped on shift on controllers only. The good think is that we have always a Reloop button in the GUI

My double click idea fails in live situation when you like to get creative like you can do with normal hot cues. So let's scratch that.

daschuer avatar Feb 26 '24 15:02 daschuer