Full Ableton Link support#10999
Conversation
|
Awesome! <3 |
|
Please finish merging this! I'm probably going to try bulding this branch tomorrow. I would use ableton link a lot. |
daschuer
left a comment
There was a problem hiding this comment.
Thank you for this nice PR
Here some comments:
67bb0c2 to
a2bf288
Compare
af87be3 to
958b49a
Compare
|
Any updates on this? Super excited by Link support in Mixxx |
|
Try the builds of this PR - it should work in general. |
|
Pretty cool, I've tested it and tempo sync seems to work pretty well! Two things:
Also I think this would be a prime candidate for bar detection if/when Mixxx finally gets it (#4489). |
7812f3e to
da63a02
Compare
|
I updated this PR to my local development state. |
|
Any Mac Builds I can test? This is soo cool! |
Check the GitHub Actions builds: https://github.com/mixxxdj/mixxx/actions/runs/4558513041 |
|
Is there an ETA on this ? |
|
You can download a build of this PR now. General availability depends much on testing on different platforms with different sound driver configurations. It's developed under Windows11 with ASIO sounddriver. I would be happy if users of other OSes would execute the Ableton Link test plan: https://github.com/Ableton/link/blob/master/TEST-PLAN.md |
292d3a5 to
ed68fc9
Compare
|
Just a friendly ping - all review requests are addressed and this is ready for merge! |
daschuer
left a comment
There was a problem hiding this comment.
some more comments. Review is in progress.
| &AbletonLink::slotControlSyncEnabled); | ||
| } | ||
|
|
||
| // Destroy control objects before releasing Link. |
There was a problem hiding this comment.
The comment describes the code we can see anyway. Why is this required?
There was a problem hiding this comment.
I would prefer to leave the three comments as they describe the required order of the three steps
There was a problem hiding this comment.
Can you extend it "..because ..."?
Co-authored-by: Daniel Schürmann <daschuer@mixxx.org>
1dd602d to
64ef6ef
Compare
3455e8c to
311fb44
Compare
| &AbletonLink::slotControlSyncEnabled); | ||
| } | ||
|
|
||
| // Destroy control objects before releasing Link. |
There was a problem hiding this comment.
Can you extend it "..because ..."?
| } | ||
|
|
||
| double AbletonLink::getBeatDistance() const { | ||
| const auto sessionState = m_pLink->captureAudioSessionState(); |
There was a problem hiding this comment.
getBeatDistance() is called more than once during an audio callback. However within a single audio callback it is assumed that all theses calls return the same distance. This is the case for all internal syncables.
The play position is advanced at a single point in the audio callback.
If we create a new session state at every call this assumption becomes void.
I am not sure how the Link thread is scheduled. I assume that it is scheduled asynchrony. To not have changing values we need to call captureAudioSessionState() only one a suitable point during the audio callback and use the sessionState as const during the callback. Probably along with onCallbackStart() when setting m_absTimeWhenPrevOutputBufferReachesDac.
A brief comment how the Link thread is scheduled would be helpfull at the new location of captureAudioSessionState()
There was a problem hiding this comment.
This must be captured every time again, as the world around is not stoping while the Mixxx engine is processing a buffer callback.
There was a problem hiding this comment.
I think we need first find a common understanding of the issue. So please correct me if I am wrong:
We have two independent threads firing asynchronously. The audio callback prepares the samples that are passed to the DAC ahead of time. The runtime of the Audio callback itself must not matter, if it is at least finished before the next audio callback is scheduled. This means a timing value gathered at the beginning of the callback for let's say deck 1 shall have the same timestamp in its calculation as one that is gathered later for let's say deck 4. Both produce samples that are played simultaneously and shall use the same beat distance to Link clients.
You're right that the Link timeline doesn't stop and we want to stay responsive to network updates. However, we need a reliable relation between the audio and the link thread. A kind of freeze, process, continue.
The core issue is this: Within a single audio callback, getBeatDistance() is called multiple times by different parts of the Mixxx engine. Each callback need to evaluates the session state at the freezed time, a fixed timestamp that is not effected by code runtime differences or CPU load.
But when getBeatDistance() is called again moments later in the same callback, it's asking Link the same question: "What will be the beat distance in the when the buffer I prepare now is processed in the DAC"
But if captureAudioSessionState() is called again between those two getBeatDistance() Link is advanced and we use a new session state no longer related to the first one used during the first part of the audio callback. This violates the a fundamental assumption: within one audio callback, all calculations referencing the same timestamp should be consistent and causes subtle timing glitches or phase discontinuities.
So I think we need to consider where the freeze is required and where we need a fresh timing. Here we need to consider the DAC timing. We need to compensate all CPU runtime differences and remove all time that comes from CPU processing from the calculation. We need to capture once each audio callback.
The key is ensuring all audio calculations within one callback see a consistent snapshot while still allowing the system to respond to Link updates at callback boundaries.
Does that make sense?
There was a problem hiding this comment.
I just tried to change this to make sessionState a memeber variable of the AbletonLink class. But it completly broke the functionality, as these member functions are also called outside of the engine callback.
The local capturing of the sessionState is clearly the correct implementation!
| timeSinceLastCbSecs < bufferSizeSec * 2) { | ||
| m_meanOutputLatency.insert(timeInfo->outputBufferDacTime - soundCardTimeNow); | ||
|
|
||
| m_absTimeWhenPrevOutputBufferReachesDac = filteredHostTimeNow + |
There was a problem hiding this comment.
I am not sure if this works. I have not fully understand the code but I assume it does not work.
Why not just follow the example? It use the same clock which is used in captureAudioSessionState() which is required:
This serves a probably well tested version that works OKisch.
However I think we can do a better alternative when the API provides the timing:
We have already solved the issue when syncing the wavefoms inupdateCallbackEntryToDacTime() This is pretty much the same task of predicting the reach of the DAC and sort out jitter noise and clock differences.
captureAudioSessionState() has a clock for reference and does a snapshot for "now".
All data is extracted by passing m_absTimeWhenPrevOutputBufferReachesDac to transpose the result to the actual point of interest and not "now". Inside the functions a jittery "now" is subtracted from a jittery m_absTimeWhenPrevOutputBufferReachesDac, so the jitter noise disappears.
This means we actually need the time form taking "now" to the to the point when reaching the DAC including jitter. This varies each callback and this variation should be part of the value to actually compensate such issues. I think we can even reuse "updateCallbackEntryToDacTime()" or extend it as desired.
There was a problem hiding this comment.
I am not sure if this works.
It 100% does, I spend months of testing until I reached this point. This is the essence of the whole PR!
Why not just follow the example?
I started with the example, and it did not work very well. Even the Ableton developers wrote somewhere that they never got the PortAudio implementation working compareable to the other backends.
We have already solved the issue when syncing the wavefoms in
updateCallbackEntryToDacTime()This is pretty much the same task of predicting the reach of the DAC and sort out jitter noise and clock differences.
This trial-and-error approach is completely unsuitable here, as it provides neither a steady increase nor jitter-free timing information. Any errors must be averaged out by the filter ignoring individual buffer callbacks is not an option here. And this delivers an absolute time with a jitter far lower than anywhere else in Mixxx.
The implemented algorithm is tested for very long time (there is a reason, why this PR took me 4 years), and I tested many variants starting from the official Ableton examples. Believe me that this is far better than what I started from!
There was a problem hiding this comment.
I am not sure if this works.
It 100% does, I spend months of testing until I reached this point. This is the essence of the whole PR!
Oh sorry. This shall not come along as general critic of this PR. It was meant as inline comment for the local scope.
I obviously do not understand the code and the reasons why it shall be like that. As a reviewer I like to understand it. This is required to verify all edge cases. Ideally it results in some nice code comments that also secures your implementation against future changes form other authors.
So let's try to get a common understanding:
I started with the example, and it did not work very well.
What was the issue?
Even the Ableton developers wrote somewhere that they never got the PortAudio implementation working compareable to the other backends.
Do you have a reference for this? This is probably an extremely important information for our GSoC project.
Lets not repeat such PortAudio issue.
I know that the timing information of PortAudio is broken when we uses the ALSA loop back pulse or pipewire devices. The ALSO timing of the hardware soundcards is very stable.
Which API do you use on windows? For my understanding native ASIO and WDM-KS also provide good timing even though PortAudio.
Any errors must be averaged out by the filter ignoring individual buffer callbacks is not an option here.
Can you explain the reasons?
There was a problem hiding this comment.
I added the requested explanations as comments!
|
Hey I just tested this today using Ableton Live 12 and Serato DJ 3 and it seems to be working as intended for the most part! There are some oddities I encountered with Mixxx not matching the behavior/UX I have come to expect while using Ableton Link with Serato, and while I know that Mixxx isn't always aiming to match other software exactly, I think some of those changes are worth while. I'm happy to detail those here or on Zulip, not exactly sure which one is preferred, and I'd be willing to help add those code changes potentially. |
Destroy m_pLink before control objects
| } | ||
|
|
||
| double AbletonLink::getBeatDistance() const { | ||
| const auto sessionState = m_pLink->captureAudioSessionState(); |
There was a problem hiding this comment.
I think we need first find a common understanding of the issue. So please correct me if I am wrong:
We have two independent threads firing asynchronously. The audio callback prepares the samples that are passed to the DAC ahead of time. The runtime of the Audio callback itself must not matter, if it is at least finished before the next audio callback is scheduled. This means a timing value gathered at the beginning of the callback for let's say deck 1 shall have the same timestamp in its calculation as one that is gathered later for let's say deck 4. Both produce samples that are played simultaneously and shall use the same beat distance to Link clients.
You're right that the Link timeline doesn't stop and we want to stay responsive to network updates. However, we need a reliable relation between the audio and the link thread. A kind of freeze, process, continue.
The core issue is this: Within a single audio callback, getBeatDistance() is called multiple times by different parts of the Mixxx engine. Each callback need to evaluates the session state at the freezed time, a fixed timestamp that is not effected by code runtime differences or CPU load.
But when getBeatDistance() is called again moments later in the same callback, it's asking Link the same question: "What will be the beat distance in the when the buffer I prepare now is processed in the DAC"
But if captureAudioSessionState() is called again between those two getBeatDistance() Link is advanced and we use a new session state no longer related to the first one used during the first part of the audio callback. This violates the a fundamental assumption: within one audio callback, all calculations referencing the same timestamp should be consistent and causes subtle timing glitches or phase discontinuities.
So I think we need to consider where the freeze is required and where we need a fresh timing. Here we need to consider the DAC timing. We need to compensate all CPU runtime differences and remove all time that comes from CPU processing from the calculation. We need to capture once each audio callback.
The key is ensuring all audio calculations within one callback see a consistent snapshot while still allowing the system to respond to Link updates at callback boundaries.
Does that make sense?
| timeSinceLastCbSecs < bufferSizeSec * 2) { | ||
| m_meanOutputLatency.insert(timeInfo->outputBufferDacTime - soundCardTimeNow); | ||
|
|
||
| m_absTimeWhenPrevOutputBufferReachesDac = filteredHostTimeNow + |
There was a problem hiding this comment.
I am not sure if this works.
It 100% does, I spend months of testing until I reached this point. This is the essence of the whole PR!
Oh sorry. This shall not come along as general critic of this PR. It was meant as inline comment for the local scope.
I obviously do not understand the code and the reasons why it shall be like that. As a reviewer I like to understand it. This is required to verify all edge cases. Ideally it results in some nice code comments that also secures your implementation against future changes form other authors.
So let's try to get a common understanding:
I started with the example, and it did not work very well.
What was the issue?
Even the Ableton developers wrote somewhere that they never got the PortAudio implementation working compareable to the other backends.
Do you have a reference for this? This is probably an extremely important information for our GSoC project.
Lets not repeat such PortAudio issue.
I know that the timing information of PortAudio is broken when we uses the ALSA loop back pulse or pipewire devices. The ALSO timing of the hardware soundcards is very stable.
Which API do you use on windows? For my understanding native ASIO and WDM-KS also provide good timing even though PortAudio.
Any errors must be averaged out by the filter ignoring individual buffer callbacks is not an option here.
Can you explain the reasons?
…alization (kValueUndefined = 0.0) instead of arbitrary 9999.9 value.
…ee time reference. Added comments about the weaknesses in timepoint and the latency reporting. Explained why the official AbletonLink example for PortAudio is not working well.
There was a problem hiding this comment.
Some more comments.
The filter for measuring the current buffer time works nice.
The surrounding code is redundant with the already existing timing code for waveform anti jitter. We need to join it to make the waveforms smooth on windows as well.
On Linux we can skip the filter because ALSA and Pipewire has its own reliable timing
I guess the the mentioned issues with Portaudio And ableton Link are widows only issues.
| // After local bpms are updated, trigger the rest of the post-processing | ||
| // which ensures that all channels are updating certain values at the | ||
| // same point in time. This prevents sync from failing depending on | ||
| // same point in time. This prevents sync from failing depending on |
| } | ||
|
|
||
| if (CmdlineArgs::Instance().getDeveloper()) { | ||
| qWarning() << "Pa_GetStreamTime: " |
There was a problem hiding this comment.
We must not print in the engine thread because it is a locking operation, xcept there is a strong reason for it. I suggest to maintain this as commted qDebug output. When we need it for debugging we can uncomment compile and enjoy.
|
|
||
| // With the cumulated buffer time an the linear regression in the host time filter we smooth | ||
| // and host time point for each soundcard callback - with a jitter of some 100us. | ||
| auto filteredHostTimeNow = m_hostTimeFilter.calcHostTime(m_cummulatedBufferTime); |
There was a problem hiding this comment.
The type can't be guessed and is important to understand the following code.
| auto filteredHostTimeNow = m_hostTimeFilter.calcHostTime(m_cummulatedBufferTime); | |
| std::chrono::microseconds filteredHostTimeNow = m_hostTimeFilter.calcHostTime(m_cummulatedBufferTime); |
It took me a while to understand. I think I got it now.
For my understanding the output is not filteredHostTimeNow because the input is not "time now". The input is the jitter free time DAC time. So the output is the DAC time transposed to Host time (+ the average call delay). Somehow the Host time for the currently received buffer. If we subtract the measured hostTime we see the different delays (jutter) that may happen due to CPU load or other high priority task.
Can you rename it to bufferTimeAsHostTime or such?
|
|
||
| // Calculates the host time based on the auxiliary time using linear regression. | ||
| // Returns kInvalidHostTime if there are not enough points or if the calculation isn't possible. | ||
| std::chrono::microseconds calcHostTime(double auxiliaryTime) const { |
There was a problem hiding this comment.
insertTimePoint() and calcHostTime() are always called after each other. You may create function the does the work of both but faster than the single functions.
| if (filteredHostTime != HostTimeFilter::kInvalidHostTime) { | ||
| m_absTimeWhenPrevOutputBufferReachesDac = filteredHostTime + outputLatency; | ||
| } else { | ||
| m_absTimeWhenPrevOutputBufferReachesDac = hostTime + outputLatency; |
There was a problem hiding this comment.
externalSyncLatencyUs is missing here. The user may also need a manual sync.
| // the time points. | ||
| m_cummulatedBufferTime += bufferSizeSec; | ||
| auto hostTime = std::chrono::duration_cast<std::chrono::microseconds>( | ||
| ClockT::now().time_since_epoch()); |
There was a problem hiding this comment.
Same here, you can reuse m_clkRefTimer here
| m_pStream); // There is a delay & jitter to timeInfo->currentTime | ||
|
|
||
| // The time points reported by PortAudio (timeInfo->currentTime or PaStreamCallbackTimeInfo) | ||
| // are not reliable (on Windows no backend reported satisfactory values - all had huge jitter). |
There was a problem hiding this comment.
On Linux ALSA and Pipewire the are reliable. So I think we can skip our own filter here.
You can see it by the smooth waveform. Linux smooth, Windows jittery.
There was a problem hiding this comment.
On the other hand the filter is fast, compared to all the other things we do in the engine. Mabe we can use it anyway ... ??
| PaTime soundCardTimeNow = Pa_GetStreamTime( | ||
| m_pStream); // There is a delay & jitter to timeInfo->currentTime |
There was a problem hiding this comment.
Use timeInfo->currentTime, it is the same, just measured at the very beginning of the callback.
| timeSinceLastCbSecs < bufferSizeSec * 2) { | ||
| m_meanOutputLatency.insert(timeInfo->outputBufferDacTime - soundCardTimeNow); | ||
|
|
||
| m_absTimeWhenPrevOutputBufferReachesDac = filteredHostTimeNow + |
There was a problem hiding this comment.
The sanity check for wrong API data below is not considered here. I think it was required of DirectSound on windows.
|
Takeaways from the graphs:
ALSA Pulse Pipewire ALSA stack
We can also see that the time constants from both filters do not match. We need to fix the issue that we subtract two values filters with different time constances. If we use the filtered value + the reported time, we overcompensate the jitter noise. |
|
Thanks for digging into the PipeWire/Pulse vs. ALSA timing — the two-filter time-constant mismatch is a nice catch. I took a small stab at exactly that part on a branch: The honest caveat: I could only confirm it builds and behaves on CoreAudio. I don't have a PipeWire/Pulse-over-ALSA rig with a Link peer and the scope-trace harness you used above, so I can't tell whether it actually helps the bumpy case. If someone with that setup could give it a spin, that'd be hugely helpful. Happy to turn it into a proper PR against this branch if it pans out. |



Based on the old PR #3783 from @spensbot, I created a new PR with Ableton Link support for Main.

The LINK-Button enables/disables the Link session. The number right to it shows the number of connected Link peers.
It syncronizes Tempo and Beat-Position (Phase).
In Ableton Link all peers are equal, there is no leader. Therefore syncronization is always bidirectional.
In Mixxx, it syncs the Sync-Leader (the deck with SYNC and Crown-Button active) with the other peers.
The most important part of this PR is the hosttime filter implemetation, which provides a jitter free timestamp for each audio buffer start - this timestamp is in host time (computer clock not soundcard clock) which is the needed input for the Ableton Link library.
What does not always work automatical, is to determine the latency of the Sound IO. This is because portaudio does not report reliable latency information for all sound apis. On my system it works well for ASIO API, but not so well for the others. Therefore I added a manual latency compensation setting in the preferences, to allow the user to correct the latency:

I tested this with the official LinkHut-App of the Ableton Link package and with the Android App "G-Stomper Rythm", which is a free Drummachine, but with advertizing. A very cool Ableton Link software is also the music visualisation and lighting software from spensbot: https://captivatesynth.com/
Test Plan
Any Ableton Link software, should pass the following test plan:
https://github.com/Ableton/link/blob/master/TEST-PLAN.md
To install the official Linkhut reference tools used in the test plan, you can use VCPKG:
./vcpkg install ableton[hut,hutsilent]Closes #8650