'Reset key' doesn't work if a track doesn't have a key entered

[mixxxbot]

Reported by: beenisss
Date: 2018-02-07T20:03:11Z
Status: Fix Released
Importance: Medium
Launchpad Issue: lp1748001
Attachments: [Screen Shot 2018-07-14 at 21.59.46.jpg](https://bugs.launchpad.net/bugs/1748001/+attachment/5163832/+files/Screen Shot 2018-07-14 at 21.59.46.jpg)

---

As above really. (Almost) none of the tracks in my library have any key information. For those that don't, pitch reset doesn't work, meaning I'm stuck if I've adjusted the pitch by an unknown fraction of a semitone (for example) and want to revert it.

In addition, the downward micro-adjustment for pitch doesn't work with or without a key set, but upward does.

This is in 2.1, r6510 currently.

[mixxxbot]

Commented by: beenisss
Date: 2018-03-01T16:16:01Z

---

Looks like there are two separate issues here.

Regarding the small pitch adjustment buttons, if I run Mixxx in developer mode and hover over the buttons, the Up button has these characteristics:

LeftConnection:[Channel1],pitch_up Parameter: 0 Value: 0 Direction: FROM_AND_TO_WIDGET Emit: PRESS_AND_RELEASE
RightConnection:[Channel1],pitch_up Parameter: 0 Value: 0 Direction: FROM_WIDGET Emit: PRESS_AND_RELEASE
DisplayConnection:[Channel1],pitch_up Parameter: 0 Value: 0 Direction: FROM_AND_TO_WIDGET Emit: PRESS_AND_RELEASE

Whereas the Down button only has:
LeftConnection:[Channel1],pitch_down Parameter: 0 Value: 0 Direction: FROM_AND_TO_WIDGET Emit: PRESS_AND_RELEASE
DisplayConnection:[Channel1],pitch_down Parameter: 0 Value: 0 Direction: FROM_AND_TO_WIDGET Emit: PRESS_AND_RELEASE

Will update on the other issue once I get my head around Eclipse.

[mixxxbot]

Commented by: beenisss
Date: 2018-03-01T17:14:20Z

---

The issue with the pitch_down button is specific to the Tango skin.

The reset key issue is not.

[mixxxbot]

Commented by: beenisss
Date: 2018-03-01T17:35:10Z

---

Gonna log a separate bug for the pitch_down button issue as it actually affects both the key and the rate buttons.

[mixxxbot]

Commented by: beenisss
Date: 2018-07-14T21:43:14Z

---

I've been playing around with the Shade skin and it has changed my understanding of this issue a little.

Obviously all the skins have push buttons for changing the key independent of tempo. The Shade skin also has fine- and coarse-adjust sliders, and right-clicking on coarse adjust resets the key of the track. This works without the track having a key set.

If the track does have a key set, then this right-click action does the same thing as right-clicking the Sync Key button.

[mixxxbot]

Commented by: daschuer
Date: 2018-07-14T22:42:15Z

---

So do we still have an issue?
Can you describe what you do, what happens and what should happen?

[mixxxbot]

Commented by: beenisss
Date: 2018-07-15T13:23:40Z
Attachments: [Screen Shot 2018-07-14 at 21.59.46.jpg](https://bugs.launchpad.net/mixxx/+bug/1748001/+attachment/5163832/+files/Screen Shot 2018-07-14 at 21.59.46.jpg)

---

It kinda depends what was in mind when the skins/key control code was written.

Basically, the key reset action resets the pitchRatio value to 1.0. On the Shade skin you can do this by right-clicking the coarse-adjust key slider, or by right-clicking Sync Key to reset it. However (and this is true for all skins) reset using the Sync Key button only works if the track has a key set. I don't know if this is intentional or not - I can sort of see the justification for it working this way, but given that you can reset the pitchRatio value without a track key being present (and the effect is identical with or without), it's kind of a pain that the button action only works for tracks with a key, when none of the other skins have a key slider.

The reason I originally raised this issue is that almost none of my tracks have key information and this makes it difficult to use reset. I actually want the reset behaviour to do something different (reset pitchTweakRatio to 1.0 rather than resetting pitchRatio) but I figured this issue (if it's considered valid) should be dealt with first.

There's also been some discussion on this in the key manipulation stream on Zulip:
https://mixxx.zulipchat.com/#narrow/stream/109122-general/topic/Key.20manipulation

As a side note, the response areas for manipulating the key sliders on Shade aren't aligned properly - in the attached screenshot the white boxes indicate the area where the skin responds to a click and drag action. Obviously this should be a few pixels lower in each case (which matters as the sliders are tiny.) I will have a look at this at some point if somebody else doesn't get to it first.

[mixxxbot]

Commented by: daschuer
Date: 2018-07-15T18:53:29Z

---

Do you wish to fix the bug yourselves, as it is assigned to you?

mixxx/src/engine/keycontrol.cpp

Line 371 in 201987e

slotSetEngineKey(m_pFileKey->get());

Is the effected line. Here we need something more intelligent, considdering the current rate and temp settings.

[mixxxbot]

Commented by: beenisss
Date: 2018-07-16T16:25:14Z

---

Yes, I've been looking at it and was planning to fix it.

[mixxxbot]

Commented by: beenisss
Date: 2018-07-21T13:02:18Z

---

By far the simplest way to resolve this seems to be just removing these lines from the setEngineKey code:

```
    if (thisFileKey == mixxx::track::io::key::INVALID ||
        newKey == mixxx::track::io::key::INVALID) {
        return;
    }
```

Removing it fixes the issue and doesn't appear to break anything else. I can't see any ill effects of running setEngineKey when the file or destination key are zero.

Have commented in more depth on Zulip: https://mixxx.zulipchat.com/#narrow/stream/109171-development/subject/Reset.20key.20button.2Ftracks.20with.20no.20key/near/130053817

[mixxxbot]

Issue closed with status Fix Released.