Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reset pitch when loading new track #6776

Closed
mixxxbot opened this issue Aug 22, 2022 · 20 comments
Closed

Reset pitch when loading new track #6776

mixxxbot opened this issue Aug 22, 2022 · 20 comments
Labels
Milestone

Comments

@mixxxbot
Copy link
Collaborator

Reported by: MelGrubb
Date: 2012-12-14T20:52:15Z
Status: Fix Released
Importance: Wishlist
Launchpad Issue: lp1090546
Attachments: 0078-Bug-Reset-pitch-when-loading-new-track.patch


I would like the option to reset the pitch slider when a new track is loaded into a deck. This doesn't have to be mandatory, but I would like the option as it fits my style better. I have, on more than one occasion, synced a track to the one currently playing, and then forgotten to reset it when that track is done playing. Usually I catch it during preview, but not always. It seems to me that it would be highly unlikely for the next track I load to need exactly the same speed tweak that the current one does. It could happen, but I don't think it's the majority use case.

@mixxxbot
Copy link
Collaborator Author

Commented by: rryan
Date: 2012-12-14T21:10:32Z


Not a bad idea. I don't see how it would be useful to keep the pitch slider where it was when you load a new track unless you load a track that had exactly the same BPM as the previous track it won't be in sync rate-wise with any other tracks.

@mixxxbot
Copy link
Collaborator Author

Commented by: raulbehl
Date: 2014-11-24T08:35:31Z


Should we have this as a default or preference enabled ?

@mixxxbot
Copy link
Collaborator Author

Commented by: daschuer
Date: 2014-11-24T10:15:14Z


The only reason for not resetting is that a Hardware controller will get out of sync.
But even this might be better than playing a song accidentally with the same pitch of the previous song.

So +1 for default = reset on track load.

@mixxxbot
Copy link
Collaborator Author

Commented by: MelGrubb
Date: 2014-11-24T13:01:16Z


It would probably be safest to default this to off, but make it available. Unless there are statistics available to show that the majority of users have controllers that would not be bothered by this, the safest route is to maintain the current behavior. I myself use a controller with rotary knobs for pitch, so there's no sync problem for me. Someone with fader-style controls would find this a problem, though.

@mixxxbot
Copy link
Collaborator Author

Commented by: ywwg
Date: 2014-11-24T14:47:25Z


+1 to default to not resetting the pitch control.

@mixxxbot
Copy link
Collaborator Author

Commented by: raulbehl
Date: 2014-11-25T03:44:35Z


I think it is better to have it as preference enabled. We can reset pitch along with the gain knobs (given that Reset Equalizers on Track Load check box is checked). We can alter the name of the check box to something like
Reset Equalizer and Pitch on Track Load

Any suggestions?

@mixxxbot
Copy link
Collaborator Author

Commented by: ywwg
Date: 2014-11-25T03:55:13Z


In general our preference defaults have a traditional DJ setup with hardware in mind, so that's why rate and eq don't get reset on track load.

The preferences should not be linked.

@mixxxbot
Copy link
Collaborator Author

Commented by: rryan
Date: 2014-11-25T04:51:06Z


FYI
https://github.com/mixxxdj/mixxx/blob/master/src/engine/enginebuffer.cpp#L533

On Mon, Nov 24, 2014 at 10:55 PM, Owen Williams wrote:

In general our preference defaults have a traditional DJ setup with
hardware in mind, so that's why rate and eq don't get reset on track
load.

The preferences should not be linked.

--
You received this bug notification because you are a member of Mixxx
Development Team, which is subscribed to Mixxx.
https://bugs.launchpad.net/bugs/1090546

Title:
Reset pitch when loading new track

To manage notifications about this bug go to:
https://bugs.launchpad.net/mixxx/+bug/1090546/+subscriptions

@mixxxbot
Copy link
Collaborator Author

Commented by: raulbehl
Date: 2014-11-25T13:50:56Z


This is already happening by default as per enginebuffer.cpp
But as I see "pitch" is not declared as a valid mixxxcontrol thus, we do not see this happening currently.
Changing "pitch" to "rate" (at line 261 in enginebuffer.cpp) resets the pitch every time a new song is loaded.

How should I go about it?

@mixxxbot
Copy link
Collaborator Author

Commented by: ywwg
Date: 2014-11-25T14:12:44Z


RJ, That's the melodic pitch slider, which has to be reset because of the weirdness in rubberband that can set the value of the melodic pitch by itself. I thought this bug was talking about the traditional vinyl-style rate slider. (thus the mention of "speed" in the original bug)

@mixxxbot
Copy link
Collaborator Author

Commented by: rryan
Date: 2014-11-25T15:07:36Z


Oh -- I totally misunderstood this bug. Carry on :).

On Tue, Nov 25, 2014, 9:20 AM Owen Williams wrote:

RJ, That's the melodic pitch slider, which has to be reset because of
the weirdness in rubberband that can set the value of the melodic pitch
by itself. I thought this bug was talking about the traditional vinyl-
style rate slider. (thus the mention of "speed" in the original bug)

--
You received this bug notification because you are a member of Mixxx
Development Team, which is subscribed to Mixxx.
https://bugs.launchpad.net/bugs/1090546

Title:
Reset pitch when loading new track

To manage notifications about this bug go to:
https://bugs.launchpad.net/mixxx/+bug/1090546/+subscriptions

@mixxxbot
Copy link
Collaborator Author

Commented by: raulbehl
Date: 2014-11-25T16:10:02Z


If I am getting this right, we do not want this to happen by default. So we should probably be enabling it through a check box inside preferences>equalizers ?

Correct me if I am taking it the other way.

@mixxxbot
Copy link
Collaborator Author

Commented by: ywwg
Date: 2014-11-25T16:23:51Z


I wouldn't put it in the equalizers page, probably the controls page. But yes, it can be an option if someone wants to write it. I'm going to target 1.13 because it's not really necessary.

@mixxxbot
Copy link
Collaborator Author

Commented by: MelGrubb
Date: 2014-11-25T16:27:21Z


I can see a case where the preference might not be as simple as a checkbox. Now that the concept of a master tempo is on the way, I think the choice for resetting pitch/speed on reload should have three choices.

  1. Never reset speed
  2. Always reset speed
  3. Reset speed only when no master tempo is in effect.

I haven't played with the master tempo stuff, so I don't know the mechanics of it yet, but if there are any preferences having to do with master tempo, then I would think this setting would belong next to those.

@mixxxbot
Copy link
Collaborator Author

Commented by: daschuer
Date: 2014-11-25T16:54:28Z


  1. might be "Keep Tempo" or "Auto Sync Tempo"
    This feature is also strong connected with the "sync" button
    It should react sensible for all three options.

Please consider Bug #⁠1258617 as well.

@mixxxbot
Copy link
Collaborator Author

Commented by: ywwg
Date: 2014-11-25T17:03:44Z


If master sync is on and "reset speed" is selected, the speed will still match master sync.

I have a strict NO PREFERENCES policy with master sync. It is master sync, and it rules your world. All others bow before master sync.

Checkbox will be fine.

@mixxxbot
Copy link
Collaborator Author

Commented by: ywwg
Date: 2014-11-27T15:34:39Z


Thanks for working on this Rahul, but please note that since we're targetting 2.1 instead of 1.12, this work may have to sit around for a while before it gets merged. If you're ok with that, please continue. But if you're looking for something that will get merged sooner please focus on bugs marked for 1.12

@mixxxbot
Copy link
Collaborator Author

Commented by: raulbehl
Date: 2014-11-27T18:29:47Z
Attachments: 0078-Bug-Reset-pitch-when-loading-new-track.patch


Thanks for the advice Owen. Well I have already implemented it. I'll upload the patch here and when it is the right time, we can always come back to it.
I'll search for some bugs marked for 1.12. :-)

@mixxxbot
Copy link
Collaborator Author

Commented by: daschuer
Date: 2014-11-27T21:10:49Z


Hi Rahul,

thank you for the patch. Would you mind to do a Github pull request as well?

@mixxxbot
Copy link
Collaborator Author

Issue closed with status Fix Released.

@mixxxbot mixxxbot transferred this issue from another repository Aug 24, 2022
@mixxxbot mixxxbot added this to the 2.0.0 milestone Aug 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant