Skip to content

Conversation

@JoergAtGithub
Copy link
Member

This PR implements the algorithm for determining a synchronization sound-device, that is explicitly selected for time synchronization with functions outside of audio processing in the engine. The synchronization itself, will be added in a follow-up PR.
Everything the DJ hears and sees must be in sync with the local PortAudio sound device, taking into account the full output latency (including the latency contributed by the particular sound API). Even in cases where we use the network clock as reference for the interval of the audio buffer in the engine.

This PR:

  • Adds local time sync ref selection to SoundManager
  • Moved pNewMainClockRef selection out of main loop and added proper handling of setups where only headphones or booth are configured.
  • Simplified fallback mechanisms and removed redundant code.
  • Added unit tests for new function selectLocalTimeSyncRef

@daschuer
Copy link
Member

Everything the DJ hears and sees must be in sync with the local PortAudio sound device, taking into account the full output latency (including the latency contributed by the particular sound API). Even in cases where we use the network clock as reference for the interval of the audio buffer in the engine.

I have probably not yet understand the issue. All of thy syncing is only amatter win which stream padding samples are added. Ony the Master is "clean" and does not need padding samples.

The case that could be problematic is the case if the network clock is used and we have passing samples in the Master output. What does AbletonLink do in that case? Which clock is used for syncing AbletonLink devices?

// The local time sync reference is the device that is used for the
// synchronization of processes outside the audio processing engine
// to the DAC timing of the local sounddevice the DJ hears.
// This sync reference shall be used for:
Copy link
Member

Choose a reason for hiding this comment

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

This comment is missleading. Because for my undertsanding we only have one clock everything is synced too.
pNewLocalTimeSyncRef is only a temporary, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

This PR is about selecting the sounddevice that is to be used as reference for external syncronisation. This is the sounddevice with the DAC that generates the sound the DJ hears.
Time syncronization has nothing to do with the clock and this comment section does not contain this term therefore.
Output Latency is specific for each sounddevice and contains various buffers inside and outside of Mixxx. All DAC channels of the same sound-device have the same output-latency, but different sound-devices may have different output-latency. Therefore we need to determine the sound-device that is the reference, before we can gather the correct output-latency.
Logically, the DJ always hears the sound via one of the local PortAudio devices and never via the stream broadcasted over the network. Accordingly, a soundDeviceNetwork can never be useful as a reference for synchronization.

Copy link
Member

Choose a reason for hiding this comment

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

The Mixxx engine picks one DAC crystal for synchronization or the network clock.
Let's imagine we have a buffer size of 1024 frames and the network clock is 0,1% faster than the DAC (unlikely, but good for this example.

For synchronization, the DAC is feed with only 1023 frames. Broadcasting and recording still receive all 1024 frames.
This means that from DAC perspective a 120 BPM track is played with 119,9 BPM, however Mixxx reports 120 BPM.
This means if the DAC becomes now the pNewLocalTimeSyncRef it will be off compared to the Mixxx data.

Moved pNewMainClockRef selection out of main loop and added proper handling of setups where only headphones or booth are configured.
Simplified fallback mechanisms and removed redundant code.
Added unit tests for new function selectLocalTimeSyncRef
@JoergAtGithub JoergAtGithub force-pushed the localSyncRefSoundDeviceSelection branch from ac2dae6 to f3c85af Compare February 9, 2025 12:21
@JoergAtGithub JoergAtGithub added this to the 2.6-beta milestone Feb 22, 2025
@github-project-automation github-project-automation bot moved this to In progress in Releases Mar 25, 2025
// Get all outputs for each device
QHash<SoundDevicePointer, QList<AudioOutput>> deviceOutputs;
for (const auto& pDevice : std::as_const(m_devices)) {
deviceOutputs[pDevice] = m_config.getOutputs().values(pDevice->getDeviceId());
Copy link
Member

Choose a reason for hiding this comment

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

This avoid the hash lookup before writing:

Suggested change
deviceOutputs[pDevice] = m_config.getOutputs().values(pDevice->getDeviceId());
deviceOutputs.insert(pDevice, m_config.getOutputs().values(pDevice->getDeviceId()));

// The local time sync reference is the device that is used for the
// synchronization of processes outside the audio processing engine
// to the DAC timing of the local sounddevice the DJ hears.
// This sync reference shall be used for:
Copy link
Member

Choose a reason for hiding this comment

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

The Mixxx engine picks one DAC crystal for synchronization or the network clock.
Let's imagine we have a buffer size of 1024 frames and the network clock is 0,1% faster than the DAC (unlikely, but good for this example.

For synchronization, the DAC is feed with only 1023 frames. Broadcasting and recording still receive all 1024 frames.
This means that from DAC perspective a 120 BPM track is played with 119,9 BPM, however Mixxx reports 120 BPM.
This means if the DAC becomes now the pNewLocalTimeSyncRef it will be off compared to the Mixxx data.

@daschuer daschuer modified the milestones: 2.6-beta, 2.7-beta May 12, 2025
@daschuer daschuer mentioned this pull request Jan 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: In progress

Development

Successfully merging this pull request may close these issues.

2 participants