change keylock on/off affects original key

[Leo Combes]

Sorry if the title is not very descriptive, but it is difficult to describe the situation in few words.

Basically it is an error when enable or disable the keylock when the pitch control is in certain position.
It is best to test to check.

How to repeat:

situation one
1- set the pitch range at 70% or 90%
2- load a song
3- set pitch control to full low speed -70% (or -90%)
4- enable keylock (works)
5- disable keylock (fail)
6- set pitch control back to 0%, with or without keylock enabled. The song sounds accelerated, and can't be corrected. Deck is lost and Mixxx restart is needed.

situation two
1- set the pitch range at 70% or 90%
2- load a song
3- set pitch control to full high speed +70% (or +90%)
4- enable keylock (fail)
5- set pitch control back to 0% with keylock activated. Song sounds with the original key but slower.

System:
Linux Mint 64bit kernel 3.11.0-12 generic
soundcard: maya 44 and HDA Intel
Mixxx 1.12.0-alpha build r4536+

[Owen Williams (ywwg)]

This is on purpose, although it's usually used in the opposite order: adjust the pitch without keylock until the track is the right pitch, then engage keylock, then adjust the pitch slider until the BPM matches. This is because we actually have two adjustments, speed and pitch, but most controllers and all skins only show one of them. Ejecting the track and reloading the track with keylock off fixes it, as does resetting both pitch and speed COs with a script or something.

Maybe we should make it so a right click of the pitch slider, with keylock off, resets both speed and pitch?

[Owen Williams (ywwg)]

(To be clear, this is a feature because it allows DJs to match both actual pitch and BPM of tracks with only one slider)

[Leo Combes (combesl)]

Thanks for review and clarify this issue.
I do not fully understand what you said, or how it is used as a positive feature. At first I thought it was a bug. But if is a feature, is not a problem.
However, it is not good what happens in the described cases, the deck stays in a useless state and you need to reboot mixxx.
If you implement right click to reset speed and pitch, please add a function (control) to reset from the controller.

[Owen Williams (ywwg)]

Oh, I'm sorry, we did change this after the alpha release. Now, when you unset keylock, it also resets the pitch value. If you can build mixxx from source you should hear the difference.

[Leo Combes (combesl)]

Hi Owen,
I just compile from source and dont notice any change.
I am now with build r4577+

I'm doing something wrong?

[Daniel Schürmann (daschuer)]

 > Now, when you unset keylock, it also resets the pitch value.

I think we do not and I think this would be wrong.

But also I think the bug report is valid, at least because Leo did not expect this feature.

Currently if you wish to play the track at original pitch, you need to press key-lock BEFORE touching the pitch slider.

There is a problem when one is using "Sync" and noticed that it sounds bad and wish to jump back to the original key.
Now, you need to right click the pitch slider, enable key-lock and press sync again.

Original it was planned to have a key knob, to adjust the key, but this is unlikely a part of a controller and is only implemented in the developers Deere skin

Conclusion:
We have a kind of regression, since the Mixxx 1.11 "Keylock" button was
"Set key to original track key and lock it"
in Mixxx 1.12 it is just
"Keylock"

We have also a unused Control "sync_key"

What about adding a control "reset_key"

How should the both above be controlled (Controller/GUI) ?

[Leo Combes (combesl)]

The way how you use it can be good or can be a feature or not (or bug... or not). Or maybe I don't understand how it works and it's my problem.
But it is a problem when the deck is in the state that I described at the end and I have to reboot to use mixxx. That's not right.
Some type of "reset" could be implemented.

[Owen Williams (ywwg)]

Leo I don't get that behavior with latest trunk. After I load a new track everything resets. Which keylock scaler are you using? (soundtouch or rubberband? ie faster / better?)

[Owen Williams (ywwg)]

Daniel, I mean currently when keylock is unset, the musical pitch is reset to natural. The speed is kept where it is. That way the pitch can't get "stuck" in a weird mode.

I think the current behavior is intuitive (pressing keylock keeps musical pitch, unpressing keylock resets musical pitch) unless there's a bug, which is what Leo seems to be experiencing.

[Leo Combes (combesl)]

I don't know which keyscale use, you mean "beat analyzer"? I have "queen mary" and "soundtouch", I don't know which differences are in each one.
I load a new track and deck continues in wrong key as in "situation one" described in first post.

What version you have?

[Owen Williams (ywwg)]

Under Sound Hardware, what do you have selected for "Keylock/Pitch-bending engine"?

[Leo Combes (combesl)]

Keylock/Pitch-Bending Engine => Rubberband

[Daniel Schürmann (daschuer)]

I have just tested latest trunk and everything works as desired at default pitch range.
Also the keylock unset behavior is just fine.
I am able to reset the track to original pitch an speed by unsettling key lock and right clicking the pitch slider (tested with Shade skin)

But I can actually create the bug described in situation one, finally the pitch is not at the original key and some octaves off.
Loading a new track does not help.

Situation two works as desired for me.

[Owen Williams (ywwg)]

Oh I see. This is why I hate our giant pitch ranges -- it's triggering our "isScratching" behavior because the keylock scalers can't deal with such giant pitch changes. I'll see if I can fix it.

[Owen Williams (ywwg)]

Hm not quite. The problem is that at extremely slow speeds, rubberband can crash (enginebufferscalerubberband.cpp:71), so we adjust the pitch instead of the speed to compensate. This pitch change gets locked in and can only be unset if the pitch knob is reset.

Fix #1 is to reset the pitch knob when a new track is loaded. That's easy enough, but is that ok? I think it is.

Fix #2 is harder. The pitch knob is getting programatically tweaked, and the current code says "if the pitch value has been changed, use the pitch scaling engine." So when keylock is disabled, the pitch knob is still tweaked so the code uses the keylock engine. My plan is to make it so that disabling keylock disables the keylock engine. Users who have a separate pitch knob (people that don't exist), can still use it. If they happen to enable keylock and then disable it, the pitch change will get reset. I don't know how to avoid this.

Other than reducing the max pitch scale to 50% :))))

[Owen Williams (ywwg)]

https://github.com/mixxxdj/mixxx/pull/324

[RJ Skerry-Ryan (rryan)]

Marking fixed since the associated PR is merged.

[Swiftb0y (swiftb0y)]

Mixxx now uses GitHub for bug tracking. This bug has been migrated to:
https://github.com/mixxxdj/mixxx/issues/7563