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

9897 - fix ticker.cc loop range end wrap #966

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jsundqvist
Copy link

@x42
Copy link
Member

x42 commented Mar 10, 2025

Nice find!

now makes me wonder if end_sample < start_sample is actually an issue further up and should not happen in the first place.

@jsundqvist
Copy link
Author

jsundqvist commented Mar 11, 2025

@x42 here's another one https://github.com/Ardour/ardour/pull/967/files

EDIT: never mind. that one doesn't work with hardware, probably due to _mclk_out_latency.

@x42
Copy link
Member

x42 commented Mar 24, 2025

So it this pull-request ready to go?

I lent my only hardware device that can be sync'ed to MIDI Clock, so I cannot test myself.

@jsundqvist
Copy link
Author

No, it's accumulating a tiny bit of lag in the loop and getting out of sync, I'll convert to draft.

Other related issues:

  • Notes are cut short at the start of the loop (maybe NoteOffs from the previous lap, unrelated to MIDI clock)
  • Playback Start/Continue delayed by one beat (probably _mclk_out_latency + midi_clock_beat_at_or_after, unrelated to loops)
  • Position not reset when Stopping and Continuing.

Let's see if I can make separate changes or if they need to be here. I'm running a Roland TD-27 drum module where MIDI clock is driving the modules built-in Click track, so pretty handy testbed.

@jsundqvist jsundqvist marked this pull request as draft March 25, 2025 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants