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

Beatjumping a Loop looses sync #11381

Closed
obsoleszenz opened this issue Mar 16, 2023 · 23 comments · Fixed by #11152
Closed

Beatjumping a Loop looses sync #11381

obsoleszenz opened this issue Mar 16, 2023 · 23 comments · Fixed by #11152

Comments

@obsoleszenz
Copy link

obsoleszenz commented Mar 16, 2023

Bug Description

Beatjumping / moving a loop often looses sync with other decks, sometimes being just slightly off sometimes more noticable. It also seems to increase the more often one jumps. Interestingly this doesn't happen if a loop is not enabled.
Also it doesn't happen if the loop just moves inside it's own range (moving a 8beat loop one beat back wont cause a problem). But it happens if one moves a 4 beat loop 8beat back/forth.

Reproduce

  1. Load two tracks with 4onthefloor beats to make things easy
  2. Enable quantization on both
  3. set loops on both decks
  4. beatmatch
  5. let one of the tracks jump back with a jump size > the activated loop size
  6. they are out of sync, if not try again

Version

2.3.4

OS

Linux

@daschuer
Copy link
Member

Is quantize enabled? Do you use const or non-const beat-grids? Can you also confirm the issue with 2.4-alpha?

@obsoleszenz
Copy link
Author

obsoleszenz commented Mar 16, 2023

Yes quantize is enabled. What is a non const beat grid? In case it's about variable tempo, no tempo is constant.

Are there appimages for the nightly/2.4 alpha builds? I'm on arch and would need to compile everything from source which takes quite some time on my machine.

@MarcPlace
Copy link

I've noticed a similar issue when I try to decrease beats on a running loop e.g. from 8 to 4 beats.
For example deck 1 and 2 are completely in sync (let's say both running on 120 BPM, beat is matched). Now I'm setting a loop on deck 1 with 8 beats while deck 2 is still playing => both are still in perfect sync. Now I decrease the loop on deck 1 from 8 to 4 beats => deck 1 now gets out of sync of deck 2. The more I'm decreasing beats, the more it drifts away (e.g. when I'm setting a loop with 32 beats, and then I'm decreasing it to 16 => 8 => 4 while running).
Oddly enough, increasing a running loop seems to work fine, haven't seen any problems with it so far.
I'm using 2.4 alpha (recent commit 0e7601c) on Linux.

@ronso0
Copy link
Member

ronso0 commented Mar 26, 2023

Do you halve the loop while you're in the first halve of it? Like, no potential conflict of playposition and loop_out marker?

@MarcPlace
Copy link

Oh, good question... wasn't aware of. So I did some further investigations and yes, it seems that it happens (only?) in the second half of the loop e.g. if the loop decreases from 8 beats width to 4 beats while it plays already the 5th or 6th beat within the loop.
But I'm pretty sure I did this also already quite often in previous versions with no problems at all. Unfortunately I don't know if my issue has an impact or something to do with @obsoleszenz failure while he was moving a loop forth and back. But yes, maybe it could be.

@ronso0
Copy link
Member

ronso0 commented Mar 26, 2023

yes, it seems that it happens (only?) in the second half of the loop e.g. if the loop decreases from 8 beats width to 4 beats while it plays already the 5th or 6th beat within the loop.

Hmkay, the code to calculate the new position is here, but it looks like there haven't been relevant changes for years, so the source must be somewhere else IMO.

if (adjusted_position > new_loop_out) {
// In case play head has already passed the new out position, seek in whole
// loop size steps back, as if playback has been looped within the boundaries
double adjust_steps = ceil((adjusted_position - new_loop_out) / new_loop_size);
adjusted_position -= adjust_steps * new_loop_size;
DEBUG_ASSERT(adjusted_position <= new_loop_out);
VERIFY_OR_DEBUG_ASSERT(adjusted_position > new_loop_in) {
// I'm not even sure this is possible. The new loop would have to be bigger than the
// old loop, and the playhead was somehow outside the old loop.
qWarning() << "SHOULDN'T HAPPEN: seekInsideAdjustedLoop couldn't find a new position --"
<< " seeking to in point";
adjusted_position = new_loop_in;
}

Can you confirm the shift is not a result of the engine being overstrained, i.e. no buffer underruns occur? (see buffer underrun counter at the bottom of Pref >-Sound Hardware).

Could you please verify this shift was introduced by 2.3.4, like test again with 2.3.3 (or earlier version if you find 2.3.3 is also affected).

@obsoleszenz
Copy link
Author

obsoleszenz commented Mar 27, 2023

Can you confirm the shift is not a result of the engine being overstrained, i.e. no buffer underruns occur? (see buffer underrun counter at the bottom of Pref >-Sound Hardware).

Seems not to be the case, counter did not increase

Could you please verify this shift was introduced by 2.3.4, like test again with 2.3.3 (or earlier version if you find 2.3.3 is also affected).

As there are no appimages i can't test the 2.3.3 again without compiling from source :/ But actually I'm quite sure this was a problem also with 2.3.3 and before.

But it's really easy to test:

  1. Load two tracks with 4onthefloor beats to make things easy
  2. Enable quantization on both
  3. set loops on both decks
  4. beatmatch
  5. let one of the tracks jump back with a jump size > the activated loop size
  6. they are out of sync

@MarcPlace
Copy link

MarcPlace commented Mar 27, 2023

Can you confirm the shift is not a result of the engine being overstrained, i.e. no buffer underruns occur? (see buffer underrun counter at the bottom of Pref >-Sound Hardware).

Yes, no underruns here, the counter still shows 0 when this issue occurs.

Could you please verify this shift was introduced by 2.3.4, like test again with 2.3.3 (or earlier version if you find 2.3.3 is also affected).

Went back to 2.3.3 release version just to be sure and yes, it already happens in this version too. Very odd, never noticed it before. Btw I use the buttons on my Denon MC7000 controller to change the width of the loop. But I've counter checked this using the loop buttons on the skin... it also happens there.
And after all I come to this conclusion that it really happens only in the second half of the loop. If I decrease the loop in the first half (e.g. set an 8 beats loop, and reducing it to 4 beats within the first 4 beats) it seems to work quite well.
HTH
Edit: Just tried 2.3.2 unfortunately with the same result. Seems to be broken for a long time...

@obsoleszenz
Copy link
Author

obsoleszenz commented Mar 28, 2023

yes, it seems that it happens (only?) in the second half of the loop e.g. if the loop decreases from 8 beats width to 4 beats while it plays already the 5th or 6th beat within the loop.

Hmkay, the code to calculate the new position is here, but it looks like there haven't been relevant changes for years, so the source must be somewhere else IMO.

if (adjusted_position > new_loop_out) {
// In case play head has already passed the new out position, seek in whole
// loop size steps back, as if playback has been looped within the boundaries
double adjust_steps = ceil((adjusted_position - new_loop_out) / new_loop_size);
adjusted_position -= adjust_steps * new_loop_size;
DEBUG_ASSERT(adjusted_position <= new_loop_out);
VERIFY_OR_DEBUG_ASSERT(adjusted_position > new_loop_in) {
// I'm not even sure this is possible. The new loop would have to be bigger than the
// old loop, and the playhead was somehow outside the old loop.
qWarning() << "SHOULDN'T HAPPEN: seekInsideAdjustedLoop couldn't find a new position --"
<< " seeking to in point";
adjusted_position = new_loop_in;
}

Can you confirm the shift is not a result of the engine being overstrained, i.e. no buffer underruns occur? (see buffer underrun counter at the bottom of Pref >-Sound Hardware).

Could you please verify this shift was introduced by 2.3.4, like test again with 2.3.3 (or earlier version if you find 2.3.3 is also affected).

I'm currently debugging this and looks like the pointed out code is not the problem. Is there anything afterwards that is still manipulating the position? I just figured out that the misplacement of the playhead also happens if the loop is moved forwards.

@ronso0
Copy link
Member

ronso0 commented Mar 28, 2023

  • Load two tracks with 4onthefloor beats to make things easy
  • Enable quantization on both
  • set loops on both decks
  • beatmatch
  • let one of the tracks jump back with a jump size > the activated loop size
  • they are out of sync

I can not reproduce the issue with Mixxx 2.3.4. I did

  • deck 1: load a track with a perfectly set beatgrid (const tempo) to deck 1
  • deck 1 & 2: enable quantize, center pitch fader on both decks
  • deck 1: hit play, set a 8-beat loop,
  • drag the track over to the other deck 2 = deck clone = both decks play perfectly in sync (with loops: forever)
  • on one deck halve loop, move it, double it, move it (beatjump size > loop size), no matter what I do ...
  • beatgrids on deck 1 & 2 are always in sync

@obsoleszenz Could it be you confuse 'quantize' with beatsync? Like, the tempo of tracks is not the same and they are not synced 100% and jumping around makes this more obvious over time?

FWIW I could also not reproduce it with two different tracks (130.2 vs 160) when using Sync lock. No beat grid shift noticable.

@obsoleszenz
Copy link
Author

  • Load two tracks with 4onthefloor beats to make things easy
  • Enable quantization on both
  • set loops on both decks
  • beatmatch
  • let one of the tracks jump back with a jump size > the activated loop size
  • they are out of sync

I can not reproduce the issue with Mixxx 2.3.4. I did

* **deck 1**: load a track with a perfectly set beatgrid (const tempo) to deck 1

* **deck 1 & 2**: enable quantize, center pitch fader on both decks

* **deck 1**: hit play, set a 8-beat loop,

* drag the track over to the other **deck 2** = deck clone = both decks play perfectly in sync (with loops: forever)

* on one deck halve loop, move it, double it, move it (beatjump size > loop size), no matter what I do ...

* beatgrids on deck 1 & 2 are always in sync

@obsoleszenz Could it be you confuse 'quantize' with beatsync? Like, the tempo of tracks is not the same and they are not synced 100% and jumping around makes this more obvious over time?

FWIW I could also not reproduce it with two different tracks (130.2 vs 160) when using Sync lock. No beat grid shift noticable.

I figured out it's easier to reproduce with quantize being disabled, as quantize will resync the tracks by pitching up/down one track, so it's slightly off but mixxx will try to fix it again.

A better way to reproduce it is this:

  • load two track with a perfectly set beatgrid (const tempo) to deck 1 & 2
  • play them and beatmatch
  • disable quantization on both
  • now jump 16 beat backwards on deck1
  • everything should be still beatmatched
  • now jump 16 beat forward on deck1
  • everything should be still beatmatched
  • now set a 4 beat loop on deck1
  • now jump 16 beat backward on deck1
  • not beatmatched anymore

@ronso0
Copy link
Member

ronso0 commented Mar 29, 2023

Still can't reproduce if I use deck cloning after starting deck1. No matter how I resize or move the loop, beatgrids are aligned.

Can you reproduce with a cloned deck?
Can you reproduce with diff. tracks after you hit Sync to have parallel beatgrids?

@obsoleszenz
Copy link
Author

Still can't reproduce if I use deck cloning after starting deck1. No matter how I resize or move the loop, beatgrids are aligned.

Can you reproduce with a cloned deck? Can you reproduce with diff. tracks after you hit Sync to have parallel beatgrids?

Yup also happens with a cloned deck. But might be more noticable without syncing.

@obsoleszenz
Copy link
Author

I recorded a small video showing the problem:
https://transfer.sh/5QYzcN/output.webm

@obsoleszenz
Copy link
Author

Here a longer video with syncing & quantization turned one, one can hear that mixxx somehow fixes the syncing but directly after the jump it's out of sync:
https://transfer.sh/Wx2uzI/output.mkv

@obsoleszenz
Copy link
Author

@ronso0 can you make sure that you have key sync disabled? It doesn't happen for me when key sync is enabled (my current theory is that the bug is somewhere in the enginebufferscale?)

@MarcPlace
Copy link

MarcPlace commented Apr 5, 2023

I have observed the following when I tried to reproduce @obsoleszenz issue and mine (these are two different cases, but I think the source of our issues could be the same):
I've cloned the same track from deck 1 to deck 2 while playing, setting loops on both decks, and then beatjump a loop on one deck (doesn't matter which one). Everything is in sync, I can jump the loop around with no issues at all.
Now take a 125 BPM track and change pitch to let's say 126 BPM, and repeat above steps once again. That was the point where I could also reproduce @obsoleszenz issue.
Maybe I can manage to produce a quick video the next days to show my issue too.

@ronso0
Copy link
Member

ronso0 commented Apr 6, 2023

Now take a 125 BPM track and change pitch to let's say 126 BPM, and repeat above steps once again.

That way I can reproduce the issue (with cloned decks).

@ronso0
Copy link
Member

ronso0 commented Apr 6, 2023

To summarize:

  • with rate slider centered seeks are perfect
  • with quantize enabled seeks are perfect
  • amount of rate offset doesn't matter as long as it's > 0
  • with rate offset, both forward and backward loopmoves cause a shift
  • seek offset depends on the buffer size: the bigger the buffer, the bigger the offset

Maybe the issue is somewhere in

double LoopingControl::seekInsideAdjustedLoop(
or
void EngineBuffer::processTrackLocked(

PS contrary to my first test I can now reproduce it even with jumps smaller than the loop size (so that the playhead doesn't seek), with rate centered 🤷‍♂️

@ronso0
Copy link
Member

ronso0 commented Apr 8, 2023

btw https://mixxx.zulipchat.com/#narrow/stream/247620-development-help/topic/LoopingControl.3A.3AnextTrigger.28.29

Did some debugging, and LoopingControl::seekInsideAdjustedLoop called by LoopingControl::nextTrigger, and EngineBuffer::processTrackLocked themselves seem okay.
The offset we see might be introduced by ReadAheadManager::getNextSamples (see reachedTrigger and overshoot).

@ronso0
Copy link
Member

ronso0 commented Apr 10, 2023

The offset seems to be exactly the buffer length, so the jump obviously happens one process call to early

@ronso0
Copy link
Member

ronso0 commented May 16, 2023

Removing the flushing code paths (in all buffer scalers) touched in #11152 fixes the loopmove offset 🎉
Though, I'm not familiar with the scale, there's probably a scenario where flushing is required to avoid other failures/offsets.

@ronso0 ronso0 added this to the 2.4.0 milestone May 16, 2023
@ronso0
Copy link
Member

ronso0 commented Jun 6, 2023

Fixed by #11152

@ronso0 ronso0 closed this as completed Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants