enabling sync should enable the sync target for sync as well

Bug #1404739 reported by Daniel Schürmann
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mixxx
In Progress
Wishlist
Owen Williams

Bug Description

This is a copy of discussion at: https://github.com/mixxxdj/mixxx/pull/415

daschuer:

"
I have also just played with sync enabled and it works nice as long you enable sync_enable right from the start. Otherwise it is confusing for me, because we have a unexpected sync master shift when using a long press on sync_enable.
The rule "all synced decks always match bpm," is very nice and we should make it true for the two deck start as well.
Currently the track you start to sync is synced to the playing deck, which is a temporary master for this moment. Just after that it is relegated to nothing, because the synced deck steals the master nature and the old master is now entirely unrelated. That is quite unfair ;-).
So what about enable sync_enable on the ex master deck as well and let it be the master?
The user is able to remove it from the sync group manually, but for the moment a sync group member ship is reasonable.

The only renaming "problem" is if no deck of the sync group is playing and you play an other to the master. How can you match the sync group to the playing deck and make it a member of the sync group. Right click the sync button on a sync group member helps (because of broken pickSyncTarget()), but you cannot add the playing deck. Working solution: disable all sync_enable, enable sync_enable on the playing deck and add the others again. To be honest, I would be afraid to have a tempo shift on the playing deck when pressing the sync_enable, but it works ...
Any idea to improve this?
"

ywwg:
"
> So what about enable sync_enable on the ex master deck as well and let it be the master?

no, because users expect to tap sync_enable to match one deck to another without engaging full sync mode. This is not the place to discuss master sync right now. Go ahead and use pickSyncTarget()
"

Tags: sync
Revision history for this message
Daniel Schürmann (daschuer) wrote :

> no, because users expect to tap sync_enable to match one deck to another without engaging full sync mode. This is not the place to discuss master sync right now. Go ahead and use pickSyncTarget()

This is still true. I mean to add auto add the master track to the sync group only if full sync mode is started and it was thi inital master for the new sync group.

Revision history for this message
Owen Williams (ywwg) wrote :

There is no such thing as "full sync mode" or a "sync group".I don't understand.

Revision history for this message
Daniel Schürmann (daschuer) wrote :

sync group = all with sync_enable enabled.
> without engaging full sync mode ..
I was just reusing your term.

But let me explain it an other way round:
The user expect that a deck syncs to somewhat when short clicking the sync_enable without following a master, this is an important feature and we should keep in in any case. But when I holds the sync_enable button at least I do expect that it continuously follows a master. Since we have no master in this situation, it currently syncs to the playing deck, but than follows itself. This feels wrong.

I understand that this is correct from the programmatic point of view. And it see two possible solutions to make it feel correct.
* Enable the sync_enable for the playing deck as well (My favorite)
* Do not follow itself, and introduce a blink pattern to visualize this standby state.

Revision history for this message
Owen Williams (ywwg) wrote :

OK, so we agree that given the case where two decks are playing and neither has sync on, when the user enables sync on one deck, it picks up its bpm from the other deck. This is because people expect to push the button on the deck whose speed they want to change.

Now we are in a case where one sync has the sync light on and the other doesn't. The rule is, "all decks with sync on match bpm." A deck with sync off doesn't need to match anything. So if another deck has sync enabled, it will match the other deck with sync enabled. If you want all decks to match, you should enable sync on all decks. The deck with sync enabled is not "following itself" -- it is following internal sync. But the other rule is that you can change the speed slider of any sync deck, and all other decks will match. This is one of the great innovations for Mixxx master sync, that you don't need to think of which decks are "masters" and which decks are "followers". If the light is on, the deck is participating in master sync.

Another rule: "enabling sync on a deck should never change the speed of other decks" -- the other bug you filed would definitely violate this rule, so if I'm able to reproduce it I'll definitely fix it.

Revision history for this message
Owen Williams (ywwg) wrote :

One way to think of it is, the first time you enable sync when sync is not enabled, it has to get its BPM from somewhere. If there are no decks playing, it will take the bpm of the current deck. If another deck is playing, even if it doesn't have sync on, it will take the speed of that deck. If other decks are playing that do have sync on, it will match that rate.

The *only* time enabling sync changes the bpm based on a non-sync deck is if there are no sync decks, and there is at least one playing non-sync deck. This is necessary to move from the state of "sync is not on anywhere" to "sync is on somewhere" without causing unexpected changes in the speeds of playing decks.

Revision history for this message
Owen Williams (ywwg) wrote :

The one change I could see making is having sync_enabled not pick up the bpm from a non-sync deck, so that enabling sync when no other sync deck is active would initialize the internal clock from that deck. But then the question arises, how do we code the tap-to-sync behavior that previous users are used to? The current sync button is a latching button, so a brief press does sync_enabled=True and then sync_enabled=False, whereas the long press just does sync_enabled=True and holds it. How does mixxx know that the press is going to be short and match the bpm of the playing non-sync deck?

Revision history for this message
Daniel Schürmann (daschuer) wrote :

For me it would be a natural for the issue you describe it #6 to make the playing deck (the sync provider) sync_enabled=True.
It is in sync anyway, and the user gets a nice feedback to which deck he has just synced.
I can't think of a use case where this could be wrong.

If no deck has sync_enabled=True, the internal sync somehow follows the current playing deck, right?
Once you hit sync_enabled=True on any deck, the internal sync adopts a canst bpm value? Or do we have a floating master deck?

Revision history for this message
Owen Williams (ywwg) wrote :

The internal clock does float around when sync is off everywhere. As soon as one deck enables sync, the internal clock is initialized with that deck's information (bpm and beat distance).

I am open to the idea of enabling sync on both decks and will experiment with it. The problem with enabling sync on both decks is, again, the tap-to-sync problem. How does the other deck know to disable sync along with the first one? I want to avoid having any timers but I'm not sure it's possible.

Revision history for this message
RJ Skerry-Ryan (rryan) wrote :

Hm, good points from you both. I think this sounds like a nice behavior.

However, the new sync code is already very complicated. It's not clear how this could be implemented easily without adding a new sync state (FOLLOWER_MOMENTARY) for the reasons Owen mentions in comment 6.

Is there a dead-simple (like, 10 line) fix for this? If not I say we delay it to 2.1. We have so many integration bugs to nail down right now that we need to stop making changes unless we want a May release. (BTW I would like to shoot for a late January / mid-February release if we can get a beta out before the new year).

tags: added: sync
Changed in mixxx:
status: New → Confirmed
summary: - sync_enabled steals master nature of the playing deck
+ enabling sync should enable the sync target for sync as well
Revision history for this message
Owen Williams (ywwg) wrote :

Yeah I think the new feature will require some pretty careful planning and testing. The nice thing is it doesn't really change the basic workflow so users shouldn't have trouble adapting from the current scheme to the improved scheme.

Changed in mixxx:
assignee: nobody → Owen Williams (ywwg)
milestone: none → 2.3.0
status: Confirmed → In Progress
importance: Undecided → Wishlist
Be (be.ing)
Changed in mixxx:
milestone: 2.3.0 → none
Revision history for this message
Swiftb0y (swiftb0y) wrote :

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

lock status: Metadata changes locked and limited to project staff
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.