muse-sequencer / muse

MusE is a digital audio workstation with support for both Audio and MIDI
https://muse-sequencer.github.io/
Other
644 stars 69 forks source link

Fix JACK synchronization while repositioning #1279

Closed theGreatWhiteShark closed 4 months ago

theGreatWhiteShark commented 4 months ago

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.

theGreatWhiteShark commented 4 months ago

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.

terminator356 commented 4 months ago

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 commented 4 months ago

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.