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

Fix JACK synchronization while repositioning #1279

Conversation

theGreatWhiteShark
Copy link
Contributor

While using JACK Timebase support and being registered as Timebase master MusE appends the JACK's current transport position with bar, beat, tick, tempo, measure, etc (BBT) information via timebase_callback. Other clients can then trigger tempo changes, relocations etc. using these BBT infos.

But when repositioning to another location MusE did use the jack_transport_locate method of the JACK API, which only sends frame information. Thus, the JACK server will drop BBT support for a single process cycle and other clients need to figure out the new transport position by frame alone (which most probably will not work correctly when they did already handled tempo or measure changes). Instead, MusE does now use jack_transport_reposition in case it is registered as Timebase master. Using this method not just the new frame but all BBT information similar to the timebase_callback are provided.

While using JACK Timebase support and being registered as Timebase master MusE
appends the JACKs current transport position with bar, beat, tick, tempo,
measure, etc (BBT) information via `timebase_callback`. Other clients can then
trigger tempo changes, relocations etc. using these BBT infos.

But when repositioning to another location MusE did use the
`jack_transport_locate` method of the JACK API, which only sends frame
information. Thus, the JACK server will drop BBT support for a single process
cycle and other clients need to figure out the new transport position by frame
alone (which most probably will not work correctly when they did already handled
tempo or measure changes). Instead, MusE does now use
`jack_transport_reposition` in case it is registered as Timebase master. Using
this method not just the new frame but all BBT information similar to the
`timebase_callback` are provided.
Copy link
Member

@terminator356 terminator356 left a comment

Choose a reason for hiding this comment

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

Hello, thanks for the PR. Great detective work!
The concern I had with using jack_transport_reposition() was that
when other clients use jack_transport_locate(), it causes the BBT information
to be blanked. I noted this in the comments.
I found that it meant that using jack_transport_reposition() was
effectively useless - unless ALL Jack apps that we wish to connect with use
jack_transport_reposition() instead of jack_transport_locate().
Otherwise the BBT information is blanked.
These test were done around one decade ago.
So I may be wrong these days, maybe Jack has changed over the years.
However I see that for example QTractor still uses jack_transport_locate()
and does not use jack_transport_reposition().
So I am worried that the BBT information will be blanked.
Could you please possibly test operating the MusE transport with other Jack apps
running and verify that the BBT information is not blanked?
Try for example QTractor and Ardour.

Thanks.
Tim.

@theGreatWhiteShark
Copy link
Contributor Author

Could you please possibly test operating the MusE transport with other Jack apps
running and verify that the BBT information is not blanked?

I'm actually in the process of fixing BBT synchronization in Hydrogen. After writing and passing some internal unit and integration tests I do now use MusE as reference to test the implementation. I'm very glad you are bringing this up since I read your comment and I do interpret the (sparse) JACK API doc quite differently.

I think blanked out BBT information when relocation was triggered by a non-master client is no problem at all. On the contrary, I do not understand why the doc allows (or at least suggests) for non-master clients to trigger repositioning using BBT information.

The way I do understand the Timebase master concept is that one application broadcasts BBT position information along with tempo and measure to all other clients. If it features changes in tempo or measure only that very program can convert a frame into correct BBT information at any particular transport position. Say, non-master client B wants to locate to frame 123657. In order to find the corresponding bar, beat, and tick, it has to know about all previous changes in measure. And it has no way to know which tempo to pick from e.g. the Mastertrack in case MusE is registered as Timebase master. Blanking out BBT info would be better than sending an inconsistent one.

With blanked information I do a relocation that is off most of the time followed by a second one based on the BBT information from the timebase callback during the next process cycle. Using jack_transport_reposition when timebase master would avoid this glitch.

Copy link
Member

@terminator356 terminator356 left a comment

Choose a reason for hiding this comment

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

Sorry for the late reply.
I seem to vaguely recall that the frame member is not blanked,
even if the other members are.
If the frame is not blanked, then maybe jack_transport_reposition()
becomes just like jack_transport_locate().
Let's try it and see. We'll try to work out whatever happens.
But yeah, it is about time we switched to jack_transport_reposition().
Carry on... Thanks

@terminator356 terminator356 merged commit 7074cff into muse-sequencer:master May 17, 2024
@terminator356
Copy link
Member

I forgot to mention.
There are a few other places where we compose BBT information, especially sending to plugins.
Some plugins have beat syncing and so on and use the info to sync.

Ever since comprehensive MusE latency correction was added - years after that code and code you fixed,
we now correct the BBT and/or frame information for latency. My old commented-out incomplete skeleton
jack_transport_reposition() example code might have needed updating for latency correction.

It's possible this latency correction should be applied to the BBT/frame info that you composed in your fixed code.

See for example vst_native.cpp in VstNativeSynth::pluginHostCallback() the section audioMasterGetTime.

See also lv2host.cpp in LV2Synth::lv2audio_SendTransport(). And see the function's usage
in LV2SynthIF::getData() and LV2PluginWrapper::apply().

Hope that helps. Hopefully the two examples given above kinda show what's going on.
If I get time I could try to do it. Latency coding can be tricky in MusE.
Thanks.

@theGreatWhiteShark
Copy link
Contributor Author

I seem to vaguely recall that the frame member is not blanked,
even if the other members are.
If the frame is not blanked, then maybe jack_transport_reposition()
becomes just like jack_transport_locate().

You mean the JACK server itself blanks out BBT information in case the client providing them is not registered as timebase master? That would be a good approach. Although it is probably still more efficient to use jack_transport_locate() directly as we keep track of whether we are registered as Timebase master and know in advance whether the additional information will be used or discarded.

Ever since comprehensive MusE latency correction was added - years after that code and code you fixed,
we now correct the BBT and/or frame information for latency. My old commented-out incomplete skeleton
jack_transport_reposition() example code might have needed updating for latency correction.

It's possible this latency correction should be applied to the BBT/frame info that you composed in your fixed code.

Hmm. Good point. On first thought I think the BBT information sent to other clients should not be corrected. I have not looked into plugin libraries or handling but you probably have some sort of shared buffer for exchanging audio data and need to take care of keeping its write/read access in sync. In JACK, on the other hand, the server is responsible for latency issues, like XRuns, and provides each client a batch of audio data in each processing cycle. Also, it connects (potentially) large programs having their own latencies compared to a plugin which does a rather cheap calculation (well, if it does not do some sort of spectral convolution).

Maybe the correction is a use case for the BBT offset. It is an additional field and independent of the information already sent. But I doubt that there are sufficient applications supporting it to justify the work required to implement and test it.

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.

None yet

2 participants