Rework master sync internal clock behavior

[ywwg]

See https://bugs.launchpad.net/bugs/1851985

This change reworks how master sync hands off between decks and the internal clock.

Before: Internal clock always active, so even if only one sync deck is playing, it's locked to the internal clock bpm. For beatmap (non-const beatgrid) tracks, this would cause the track bpm to be locked at the initial bpm. This causes weird effects as the track's bpm wants to change but isn't allowed to.

After: If only one sync deck is playing, it is Master. This allows its rate to remain constant and let the bpm float up and down. If two decks are playing, then Internal Clock takes over and both tracks play at the same rate. Once a single track is playing, the rate follows that deck.

I had to rework a lot of code and a lot of tests. This is a large change, sorry! Master Sync code has always been a little spaghetti-ish.

Note: This includes my change to remove half/double bpm adjustment. I'll restore that code for this PR soon.

[ywwg]

bpm display not updated on track load -- will fix

[ywwg]

bpm display not updated on track load -- will fix

huh, now I can't reproduce this. I'll keep an eye out for it though.

[daschuer]

Thank you for picking this up.
I have left some minor comments.
The code looks good. I think we need sorrowly manual testing before merge.
I think it makes sense to do after the half/double remove is reverted.

[daschuer]

Please apply this to make CMake work:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 2873b5b..651df9f 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -949,7 +949,6 @@ add_executable(mixxx-test
   src/test/soundproxy_test.cpp
   src/test/soundsourceproviderregistrytest.cpp
   src/test/sqliteliketest.cpp
-  src/test/synccontroltest.cpp
   src/test/tableview_test.cpp
   src/test/taglibtest.cpp
   src/test/trackdao_test.cpp

[daschuer]

This works nice.
Unfortunately I have managed that auto DJ stops unintended and I cannot reproduce it.
I am sure this is not an issue here but an edge case with the new auto DJ implementation.

[ywwg]

Please apply this to make CMake work:

I reintroduced this file in an update so this should work again

[ywwg]

This is a very complicated PR, but if other people could test master sync mode (push and hold sync on more than one deck) and play around with it, including auto dj, I'd appreciate it! This goes double for tracks that are beatmapped (old disco, etc) rather than gridded.

[ywwg]

yeah there's a lot going on. And it looks like basesyncablelistener is probably not needed at all since only EngineSync inherits from it

[Holzhaus]

Maybe we need it later when we add midi sync?

[ywwg]

Maybe we need it later when we add midi sync?

midi sync would just be a Syncable like the decks are -- just a source of clock that can be Explicit Master

[Holzhaus]

OK, we should probably merge the classes then.

[ywwg]

Can that please wait for a followup PR?

[ywwg]

I believe all notes are addressed

[daschuer]

Did some brief test at the track end and everything works as expected.
LGTM .

@uklotzde Merge? You have an open review here.

[ywwg]

notes addressed, thanks!

[ywwg]

all approved, who wants to push the button?

[ywwg]

woo thanks!

[Holzhaus]

By the way, I'd appreciate if you wrote a blog post about master sync and also give some usage examples for explicit master. I bet most users have no idea that this exists. If you agree, can you add yourself to the list in mixxxdj/website#77?

[Holzhaus]

Any Ideas how to expose explicit master via GUI? We could make the sync button in a different color or with a highlighted border. Right clicking the sync button could toggle master sync.

[ywwg]

Let's take that discussion to zulip