Keylock: Option how key is unlocked/reset

[ronso0]

In Bug #1653631 I described the -at least for me- unexpected behaviour when releasing keylock
while rate is not 0 (pitch slider is off center), and got to know the reasons for the
current situation:
1 a track is running, keylock is ON, pitch/tempo != original
2 release keylock
3 key is reset to original > That 'jumping' key on right now is odd!

How I would love it:
2 release keylock
3 key is kept but now unlocked
4 any further rate change changes key accordingly

Reason is, with unlocked key it's much fun gently nudging the jogwheels to temporarily
vary the key to simulate an old turntable, or a worn out cassette.

I added another row in Preferences > Interface called "Keyunlock mode" where users can choose
wether to adapt key to current tempo (default, current behaviour) or to keep previously locked key
until next rate change. This way, the unlock behaviour wouldn't change, unless users choose so.

I will test if this works out with my changes so far.
The wording is just a quick shot.

What do you think?

[daschuer]

I will test if this works out with my changes so far.
The wording is just a quick shot.

What do you think?

Thank you for adopting this.

I think a preferences option is a good approach for testing this mode.

Does th key knob turn if you unlock with the new mode? Will a key knob get into "softtakover out of sync" state after toggling in the new mode?

How is the user able to reset to original key and rate after keylock was used?
Is there an easy way, or is it nearly impossible?

[ronso0]

Does th key knob turn if you unlock with the new mode? Will a key knob get into "softtakover out of sync" state after toggling in the new mode?

Who has a key knob (not rotary encoder)? Assuming you mean the tiny sliders in Shade, I will test all this now that Travis is happy.

How is the user able to reset to original key and rate after keylock was used?
Is there an easy way, or is it nearly impossible?

Any user without a physical key reset button or unable/unwilling to use GUI controls or keyboard is well served with the existing options. Current behaviour should stay default, no discussion about that.
But due to that I mapped a "Reset key" button on my controller. This is/was my way back to normal: quickly release and reset key when tempo is not original, then play with pitch.

What I propose is the possibility to play with pitch after keylock release, be it jog wheels, pitch fader or 'rate_temp_up/down' buttons, only without that sudden jump. I don't see another use for that.

Let's say a user consciously enables the new option (and then disables keylock) he probably knows that i.e. pitch fader or jog wheels affect key and also knows how to deal with it. Furthermore it matters which combination of lock & unlock options is used: if I keep key on keylock release, then play with it (beat offset guaranteed), I could quickly move pitch fader somewhere near center, then "lock original" and fade back to desired tempo without a huge key jump. "Reset key on track load" saves any further trouble.

That said, I can only see the mentioned use case as a reason to enable this non-default option:
use jogwheels, pitchfader, wahtever as a Toy. Play with pitch, use it as an effect in creative way.
For example I mapped shifted jog wheel in a way that I can adjust BPM in +-1 steps. This way I can slow down a track to let's say 20%, then throw some movie quotes on top and turn a set from hm...DnB to in a fun way.

Puh, that got long. Hope I also answered your question ;)

And I always use "lock original key" and -only rarely- match keys between decks. I imagine, someone how relies on matching keys for harmonic mixing probably matches and locks them on track load anyway.

[daschuer]

Who has a key knob (not rotary encoder)? Assuming you mean the tiny sliders in Shade, I will test all this now that Travis is happy.

I mean the Deere key knob. The user can map for example the fixed scale deck gain knob to key. We should at least have a clue what will happen than.

---

"Reset Key" should be the default and the leftmost radio button
I am not sure what, but there is something swapped, only the Lables?

There is an issue with the key display. I had expected the following with the "Keep Key" mode

• Load a track, Key: "Bb"
• Lock the key, Key: "Bb"
• Move the rate slider to the top, Key: "Bb"
• Unlock the Key, Key: "B" expected Key: "Bb" because we should Keep the Key.

[ronso0]

Regardless of my changes, I wonder what happens here if original key was locked but changed afterwards, manually or with "match key"

[daschuer]

In this case the Key knob is cranked, right? Original "Reset Key" reset to this cranked key at rate = 1. Is there a use case for this?

The user may use it like this:

• match key
• enable key-lock
• match beat
• Unlock key ... what will happen ?? The key jumps to rather random key now, by adding the offset introduced by the rate slider.

I think in the "Keep Key" case, the offset should be applied to the key knob by turning it.

[ronso0]

It's not working. Yet.
I don't see why the condition in keycontrols.cpp is not respected.

Also I learned that everything in DlgPrefControls::slotResetToDefaults() is only set when "Restore Defaults" button is clicked. I will either swap the two buttons or explicitly set it like it is done i.e. for "PositionDisplay". Same applies to Keylock mode.

[ronso0]

I'll add debug messages to keycontrol.cpp where the key lock/unlock is processed.

And I found possibly related stuff in engine/enginebuffer.cpp. Will read through this and see if has to do with keycontrol.

Not as easy as it appeared at first glance...

[daschuer]

The "Restore Defaults" works, but restores "Keep key" it should default to "Reset Key"

[daschuer]

I think you need to uncomment this code to make your feature work.

[ronso0]

Nice, I will try. Though I don't understand the related pitch stuff in enginebuffer, yet.

[ronso0]

Okay, it's working as intended:

• key knob won't turn
• defaults are set and restored correctly
• current lock/unlock works

Only problem: when pitch slider is moved after unlocking and key is not reset manually with lock/unlock original key or just 'reset_key', key won't be "reset on track load" even tough Prefs are set so.
Baseplayer only resets 'pitch_adjust' on track load, but not pitchTweakRatio.

I could a) include this reset here in basetrackplayer.cpp or b) just create something like slotResetPitchTweakRatio in keycontrol.cpp directly and connect it to EngineBuffer::trackLoaded like it's done for 'pitch_adjust'.
I prefer b but I'm not sure what's better. Any ideas?

Meanwhile I try to extend the keylock tests to also cover the new unlock mode.

[daschuer]

You can simply override the EngineControl::trackLoaded() function.

Looking at the surrounding code, all the forth and back between the enginebuffer and base track player is looking odd.

Normally it should be informed about the new track inside this
https://github.com/mixxxdj/mixxx/blob/master/src/engine/enginebuffer.cpp#L524
locking region.
This ensures that the old track is processed with the old pitch and the new track is processed with the new pitch. All the reset solutions in BasTrackPlayer, outside this lock, does not guarantee this.

But this is a topic of a separate PR.

[daschuer]

I think the key knob should jump if you unlock the key in keep mode.

Currently it suffers this:

• enable key lock
• increase the rate.
• unlock the key
• go back to normal rate
  -> track plays at a lower than the original key / even though all knobs are at unity.
• If I now touch the key knob, key jumps back to original + knob offset.
  This is unexpected, and could be solved by adopting the key offset when unlocking the key.

[ronso0]

Okay, I will try to fix that.

[ronso0]

Key offset (pitch_adjust) is now adopted when unlocking.

I tested with Deere: key jumps when unlocking, further key changes work correctly
and Shade: visual_key_distance displays correctly

key is not reset manually with lock/unlock original key or just 'reset_key', key won't be "reset on track load" even tough Prefs are set so.

It seems this issue is solved by last commit, as well.
Woohoo!

[daschuer]

Jippie :-) It works now like a charm. Thank you. LGTM.