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

Don't allow to set loop in/out at the silence padding after the track. #11558

Merged
merged 10 commits into from
May 30, 2023

Conversation

daschuer
Copy link
Member

This fixes #11557

Copy link
Member

@ronso0 ronso0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.

@daschuer
Copy link
Member Author

Done.

@ronso0
Copy link
Member

ronso0 commented May 16, 2023

Thanks, but unfortunately it's still not clamping to track end.
I tried with the same track many times, and often it failed when dragging the loop_out, but it definitely fails when I let the track play to the end then hit Loop_out.

Finally I figured the issue is that QuantizeControl::lookupBeatPositions uses the current position (which may already be / likely IS after the end), so --with a bit of luck-- even m_pPreviousBeat is after the end, too :
With that in mind, maybe it makes sense to go back to square one here, instead of incremental fixes, idk.

Btw, if you decide to keep the current commits, could you please reword the last commit?
"Make sure the loop in and loop out is not placed on an extended beat behind the track"

@daschuer
Copy link
Member Author

I have fixed the issue along with some corner cases with holding the loop in or out button. Instead of stopping the track or take an inconsistent state it now loops to loop in instead and keeps the track running.
I consider this much better than the silence of a stopped track.

@ronso0
Copy link
Member

ronso0 commented May 24, 2023

While that last change isn't expected behaviour IMO it may still be a welcome helper.
And it seems to work around a new (yet unknown) issue in the engine buffer (or ReadAheadManager or LoopingControl::nextTrigger, idk):

  • I reverted that last commit
  • set the loop_out to the latest possible position
  • set something like an 8-beat loop, play
  • click and hold loop_in
  • when it comes very close to loop_out, the smallest loop size possible is used
  • that small loop plays a few times but suddenly the deck stops (seems the play pos crossed the track end)

Maybe that is fixed with #11532 or #11152 (with my proposed changes included)

So, this LGTM, I'll do some more testing soonish.

@ronso0 ronso0 linked an issue May 30, 2023 that may be closed by this pull request
@ronso0 ronso0 added the looping label May 30, 2023
@ronso0
Copy link
Member

ronso0 commented May 30, 2023

LGTM, this is much beter than before!

However, there still seems to be a chance to stop the deck unexpectedly.
I set loop_out at track end, let the loop run, and click loop_in repeatedly.
At some pont this should produce the shortest possible loop. Though I managed to stop the deck with that (once, didn't try more often, and I don't see how that could happen 🤷‍♂️ ).

Another issue I spotted (unrelated to this PR) is that the waveform play pos definitely goes beyond the track end with that very short loop.

@ronso0 ronso0 merged commit e29caf8 into mixxxdj:2.3 May 30, 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 this pull request may close these issues.

Loop out at track end is not reachable
2 participants