Skip to content

(fix) avoid automatic key offset after track load with keylock mode 'Keep current key'#16460

Open
ronso0 wants to merge 3 commits into
mixxxdj:2.6from
ronso0:keyoffset-fix
Open

(fix) avoid automatic key offset after track load with keylock mode 'Keep current key'#16460
ronso0 wants to merge 3 commits into
mixxxdj:2.6from
ronso0:keyoffset-fix

Conversation

@ronso0
Copy link
Copy Markdown
Member

@ronso0 ronso0 commented May 14, 2026

Final fix for #12027

The issue is that track load triggers a cascade of calls in KeyControl which lead to unintended und annoying key offsets on track load with

  • keylock ON, rate off center
  • keylock mode "Keep current key"
  • Reset Pitch on track load option enabled

Reproduce issue / confirm fix:

  1. rate centered, pitch = 0, pitch_adjust = 0, keylock disabled
  2. load a track
  3. move rate slider off center -> key is off
  4. load a new track
  5. enable keylock
  6. center rate slider
  7. reset key with reset_key control -> key is reset, pitch = 0
  8. load another track
    -> previously: pitch != 0
    -> now: pitch = 0

The issue stems from the fact that in #1222 we decided to not make the Key knob (pitch_adjust) jump when locking so that users can easily turn back to original key with it.
The consequence is that pitch_adjust should stay steady on track load.
The culprit is that BaseTrackPlayerImpl::slotTrackLoaded() did always reset pitch_adjust to 0 on track load.

The fix is to reset pitch (not pitch_adjust) to 0 in this case (key locked with "Keep current key").

Works beautifully and key tests pass 🤷‍♂️


FWIW actually we should refactor KeyControl as it seems too complicated with all its interleaved, potentially infinitly looping valueChanged() connections, as well as the interaction with BaseTrackPlayer and EngineBuffer, but I'm certainly not gonna refactor that, so consider this a quick but solid fix.

@ronso0 ronso0 changed the base branch from main to 2.6 May 14, 2026 22:20
@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented May 15, 2026

Yes, this definitly fixes it for me. No key issues anymore in a 5h set 🎉

Will fixup the unused param issues and add a regression test soon.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant