SyncLock: Don't recalc half/double multiplier on every callback

[ywwg]

This PR builds on #3698. It contains some refactoring to make the functions clearer.

[ywwg]

This patch is ready for review except that it builds on #3698 and there's no way to tell github to show the diff to that patch instead of main

[ywwg]

rebased / updated etc! ready for review

[daschuer]

Did you forget to push? I assume you want to make a single commit on top of main.

[daschuer]

Which code line contains the fix? Maybe it is also to have a single commit containing the fix only.

[ywwg]

this is the fix -- we just update the beat distance and bpm, NOT basebpm. Updating basebpm triggers all of the half/double code, and we should not do that on every callback.

[ywwg]

rebased and squashed. Sorry to mix the refactor with the fix.

[daschuer]

Is this still [WIP] according to the title?

[daschuer]

I think there is a remaining issue: When I sync to a non const tempo track at master in a slow start region with 100 BPM a cont track is synced to it it follows the track lets say to 150 BPM.
When I change now the explicite master to the const track, the original master haves the tempo.
That was at least surprising. I think we should also in this case not recalc the multiplier.

[ywwg]

I am finding major half/double issues in the code in general :(. I will try to fix those first.

[ywwg]

This now is a followon to #3899

[ywwg]

I'll split out the fix

[ywwg]

ok should be good now

[daschuer]

This has a failing test now.

[daschuer]

I can confirm that the original issue is fixed, now random pitch change when a track is playing.

I have only discovered this weird case:

When I have a playing track in deck A and it has a tempo change in this case form 107 to 100 and the follower is cued to take over at 160 it sits at 107 than suddenly changes to 200.
This a constructed edge case, but the result is surprising.

I think we mus t accept this case to make the feature work at a whole.

Of cause this does not happen when the internal clock takes over.
[BPM]", "sync_lock_algorithm" = REFER_LOCK_BPM
But that is currently not accessible for ordinary users.

[ywwg]

fixed broken test (force push)

[daschuer]

Don't rebase PRs under review. I can now review and test the whole PR again, because I cannot see what has changed and fixed the test.

Do you have the original commits you .git folder?
Maybe you can re-create them or at least point me to your changes.

[daschuer]

Fortunately I had the original testing branch in my .git folder.
The fix was just:

index cfe50230b1..fc51115364 100644
--- a/src/test/enginesynctest.cpp
+++ b/src/test/enginesynctest.cpp
@@ -1839,7 +1839,7 @@ TEST_F(EngineSyncTest, HalfDoubleThenPlay) {
     ProcessBuffer();
     pButtonSyncEnabled2->slotSet(1.0);
     pButtonSyncEnabled1->slotSet(1.0);
-    EXPECT_DOUBLE_EQ(0.5,
+    EXPECT_DOUBLE_EQ(1.0,
             m_pChannel1->getEngineBuffer()
                     ->m_pSyncControl->m_masterBpmAdjustFactor);
     EXPECT_DOUBLE_EQ(1.0,

Next time it would be much less hassle if you just commit that fix. Easy to review and now business logic touched.