Add local time sync ref (for AbletonLink etc.) 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

CMakeLists.txt

@@ -2484,6 +2484,7 @@ add_executable(
   src/test/signalpathtest.cpp
   src/test/skincontext_test.cpp
   src/test/softtakeover_test.cpp
+  src/test/soundmanager_test.cpp
   src/test/soundproxy_test.cpp
   src/test/soundsourceproviderregistrytest.cpp
   src/test/sqliteliketest.cpp

src/soundio/soundmanager.cpp

@@ -321,6 +321,40 @@ void SoundManager::queryDevicesMixxx() {
     m_devices.append(currentDevice);
 }
 
+SoundDevicePointer SoundManager::selectLocalTimeSyncRef(
+        const QHash<SoundDevicePointer, QList<AudioOutput>>& deviceOutputs,
+        const QList<SoundDevicePointer>& devices) {
+    const std::array<AudioPathType, 5> priorityOrder = {
+            AudioPathType::Main,
+            AudioPathType::Deck,
+            AudioPathType::Bus,
+            AudioPathType::Headphones,
+            AudioPathType::Booth};
+
+    SoundDevicePointer pNewLocalTimeSyncRef = nullptr;
+    for (const auto& pDevice : std::as_const(devices)) {
+        if (pDevice->getDeviceId().name == kNetworkDeviceInternalName) {
+            continue;
+        }
+        QList<AudioOutput> outputs = deviceOutputs[pDevice];
+        for (const auto& type : priorityOrder) {
+            auto it = std::find_if(outputs.begin(),
+                    outputs.end(),
+                    [type](const AudioOutput& out) {
+                        return out.getType() == type;
+                    });
+            if (it != outputs.end()) {
+                pNewLocalTimeSyncRef = pDevice;
+                break;
+            }
+        }
+        if (pNewLocalTimeSyncRef) {
+            break;
+        }
+    }
+    return pNewLocalTimeSyncRef;
+}
+
 SoundDeviceStatus SoundManager::setupDevices() {
     // NOTE(rryan): Big warning: This function is concurrent with calls to
     // pushBuffer and onDeviceOutputCallback until closeDevices() below.
@@ -351,10 +385,6 @@ SoundDeviceStatus SoundManager::setupDevices() {
     // callback from the logic in SoundDevicePortAudio. They should communicate
     // via message passing over a request/response FIFO.
 
-    // Instead of clearing m_pClkRefDevice and then assigning it directly,
-    // compute the new one then atomically hand off below.
-    SoundDevicePointer pNewMainClockRef;
-
     m_audioLatencyOverloadCount.set(0);
 
     // load with all configured devices.
@@ -363,7 +393,77 @@ SoundDeviceStatus SoundManager::setupDevices() {
 
     // pair is isInput, isOutput
     QVector<DeviceMode> toOpen;
-    bool haveOutput = false;
+
+    // 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());
+        // Statically connect the Network Device to the Sidechain
+        if (pDevice->getDeviceId().name == kNetworkDeviceInternalName) {
+            AudioOutput out(AudioPathType::RecordBroadcast,
+                    0,
+                    mixxx::audio::ChannelCount::stereo(),
+                    0);
+            deviceOutputs[pDevice].append(out);
+        }
+    }
+
+    // Select pNewLocalTimeSyncRef
+    // 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:
+    // 1.) VSync of the waveforms
+    // 2.) Sync of external audio device (e.g. drum machine) with feedback
+    //     to an auxiliary input of the mixer in a external mixing setup
+    // 3.) Sync of lighting (DMX) or video (VJ)
+    SoundDevicePointer pNewLocalTimeSyncRef = selectLocalTimeSyncRef(deviceOutputs, m_devices);
+
+    // Select pNewMainClockRef
+    // The main clock reference is the device that is used for the
+    // audio processing in the engine. It can be either a local
+    // PortAudio device or the Network clock in case of broadcasting.
+    // There are four use cases:
+    // 1.) No broadcasting->Always Soundcard Clock
+    // 2.) JACK API used->Always Soundcard Clock
+    // 3.) Broadcasting of internal mixed Main signal->Always Network Clock
+    // 4.) Broadcasting of Record/Broadcast input SoundDevice->Always Soundcard Clock
+    SoundDevicePointer pNewMainClockRef = nullptr;
+    if (m_config.getForceNetworkClock() && !jackApiUsed()) {
+        for (const auto& pDevice : std::as_const(m_devices)) {
+            if (pDevice->getDeviceId().name == kNetworkDeviceInternalName) {
+                pNewMainClockRef = pDevice;
+                break;
+            }
+        }
+    } else {
+        pNewMainClockRef = pNewLocalTimeSyncRef;
+    }
+
+    // Fallback to keep waveforms running if no local SoundDevice is configured
+    // If pNewLocalTimeSyncRef or pNewMainClockRef is still nullptr,
+    // set it to the first network clock device.
+    if (!pNewLocalTimeSyncRef || !pNewMainClockRef) {
+        for (const auto& pDevice : std::as_const(m_devices)) {
+            if (pDevice->getDeviceId().name == kNetworkDeviceInternalName) {
+                if (!pNewLocalTimeSyncRef) {
+                    qWarning() << "No local sound device configured, local "
+                                  "sync reference not set! Using"
+                               << pDevice->getDisplayName();
+                    pNewLocalTimeSyncRef = pDevice;
+                }
+                if (!pNewMainClockRef) {
+                    qWarning() << "Output sound device clock reference not set! Using"
+                               << pDevice->getDisplayName();
+                    pNewMainClockRef = pDevice;
+                }
+                if (pNewLocalTimeSyncRef && pNewMainClockRef) {
+                    break;
+                }
+            }
+        }
+    }
+
     // loop over all available devices
     for (const auto& pDevice : std::as_const(m_devices)) {
         DeviceMode mode = {pDevice, false, false};
@@ -393,26 +493,10 @@ SoundDeviceStatus SoundManager::setupDevices() {
                 m_pEngineMixer->onInputConnected(in);
             }
         }
-        QList<AudioOutput> outputs =
-                m_config.getOutputs().values(pDevice->getDeviceId());
 
-        // Statically connect the Network Device to the Sidechain
-        if (pDevice->getDeviceId().name == kNetworkDeviceInternalName) {
-            AudioOutput out(AudioPathType::RecordBroadcast,
-                    0,
-                    mixxx::audio::ChannelCount::stereo(),
-                    0);
-            outputs.append(out);
-            if (m_config.getForceNetworkClock() && !jackApiUsed()) {
-                pNewMainClockRef = pDevice;
-            }
-        }
-
-        for (const auto& out : std::as_const(outputs)) {
+        // Iterate over all outputs for the current device
+        for (const auto& out : std::as_const(deviceOutputs[pDevice])) {
             mode.isOutput = true;
-            if (pDevice->getDeviceId().name != kNetworkDeviceInternalName) {
-                haveOutput = true;
-            }
             // following keeps us from asking for a channel buffer EngineMixer
             // doesn't have -- bkgood
             const CSAMPLE* pBuffer = m_registeredSources.value(out)->buffer(out).data();
@@ -427,16 +511,6 @@ SoundDeviceStatus SoundManager::setupDevices() {
                 goto closeAndError;
             }
 
-            if (!m_config.getForceNetworkClock() || jackApiUsed()) {
-                if (out.getType() == AudioPathType::Main) {
-                    pNewMainClockRef = pDevice;
-                } else if ((out.getType() == AudioPathType::Deck ||
-                                   out.getType() == AudioPathType::Bus) &&
-                        !pNewMainClockRef) {
-                    pNewMainClockRef = pDevice;
-                }
-            }
-
             // Check if any AudioSource is registered for this AudioOutput and
             // call the onOutputConnected method.
             for (auto it = m_registeredSources.find(out);
@@ -453,19 +527,10 @@ SoundDeviceStatus SoundManager::setupDevices() {
         }
     }
 
-    for (const auto& mode: toOpen) {
+    for (const auto& mode : toOpen) {
         SoundDevicePointer pDevice = mode.pDevice;
         m_pErrorDevice = pDevice;
 
-        // If we have not yet set a clock source then we use the first
-        // output pDevice
-        if (pNewMainClockRef.isNull() &&
-                (!haveOutput || mode.isOutput)) {
-            pNewMainClockRef = pDevice;
-            qWarning() << "Output sound device clock reference not set! Using"
-                       << pDevice->getDisplayName();
-        }
-
         int syncBuffers = m_config.getSyncBuffers();
         // If we are in safe mode and using experimental polling support, use
         // the default of 2 sync buffers instead.
@@ -485,11 +550,14 @@ SoundDeviceStatus SoundManager::setupDevices() {
         }
     }
 
-    if (pNewMainClockRef) {
+    VERIFY_OR_DEBUG_ASSERT(pNewMainClockRef) {
+        // This should never happen, because the user can't delete the last
+        // broadcast device in the preferences.
+        qWarning() << "No output devices opened, no clock reference device set";
+    }
+    else {
         qDebug() << "Using" << pNewMainClockRef->getDisplayName()
                  << "as output sound device clock reference";
-    } else {
-        qWarning() << "No output devices opened, no clock reference device set";
     }
 
     qDebug() << outputDevicesOpened << "output sound devices opened";
@@ -499,8 +567,8 @@ SoundDeviceStatus SoundManager::setupDevices() {
     }
 
     m_pControlObjectSoundStatusCO->set(
-            outputDevicesOpened > 0 ?
-                    SOUNDMANAGER_CONNECTED : SOUNDMANAGER_DISCONNECTED);
+              outputDevicesOpened > 0 ?
+                      SOUNDMANAGER_CONNECTED : SOUNDMANAGER_DISCONNECTED);
 
     // returns OK if we were able to open all the devices the user wanted
     if (devicesNotFound.isEmpty()) {

src/soundio/soundmanager.h

@@ -52,6 +52,10 @@ class SoundManager : public QObject {
     void queryDevicesPortaudio();
     void queryDevicesMixxx();
 
+    static SoundDevicePointer selectLocalTimeSyncRef(
+            const QHash<SoundDevicePointer, QList<AudioOutput>>& deviceOutputs,
+            const QList<SoundDevicePointer>& devices);
+
     // Opens all the devices chosen by the user in the preferences dialog, and
     // establishes the proper connections between them and the mixing engine.
     SoundDeviceStatus setupDevices();

src/test/soundmanager_test.cpp

@@ -0,0 +1,151 @@
+#include "soundio/soundmanager.h"
+
+#include <gtest/gtest.h>
+
+#include <QHash>
+#include <QList>
+
+#include "soundio/sounddevice.h"
+
+class MockSoundDevice : public SoundDevice {
+  public:
+    MockSoundDevice(const QString& name)
+            : SoundDevice(nullptr, nullptr) {
+        m_deviceId.name = name;
+    }
+
+    SoundDeviceStatus open(bool, int) override {
+        return SoundDeviceStatus::Ok;
+    }
+
+    bool isOpen() const override {
+        return true;
+    }
+
+    SoundDeviceStatus close() override {
+        return SoundDeviceStatus::Ok;
+    }
+
+    void readProcess(SINT) override {
+    }
+
+    void writeProcess(SINT) override {
+    }
+
+    QString getError() const override {
+        return QString();
+    }
+
+    mixxx::audio::SampleRate getDefaultSampleRate() const override {
+        return mixxx::audio::SampleRate(44100);
+    }
+};
+
+TEST(SoundManagerTest, SelectLocalTimeSyncRefNoSoundDevice) {
+    QHash<SoundDevicePointer, QList<AudioOutput>> deviceOutputs;
+    QList<SoundDevicePointer> devices;
+
+    // No sound devices defined
+    // deviceOutputs and devices are empty
+
+    // Test case: Select local time sync reference with no sound devices
+    SoundDevicePointer result = SoundManager::selectLocalTimeSyncRef(deviceOutputs, devices);
+    EXPECT_EQ(result, nullptr);
+}
+
+TEST(SoundManagerTest, SelectLocalTimeSyncRefNoDeviceOutput) {
+    QHash<SoundDevicePointer, QList<AudioOutput>> deviceOutputs;
+    QList<SoundDevicePointer> devices;
+
+    auto portAudioDevice1 = SoundDevicePointer(new MockSoundDevice("PortAudioDevice1"));
+    auto portAudioDevice2 = SoundDevicePointer(new MockSoundDevice("PortAudioDevice2"));
+
+    // No device outputs defined
+    deviceOutputs[portAudioDevice1] = {};
+    deviceOutputs[portAudioDevice2] = {};
+
+    devices.append(portAudioDevice1);
+    devices.append(portAudioDevice2);
+
+    // Test case: Select local time sync reference with no device outputs
+    SoundDevicePointer result = SoundManager::selectLocalTimeSyncRef(deviceOutputs, devices);
+    EXPECT_EQ(result, nullptr);
+}
+
+TEST(SoundManagerTest, SelectLocalTimeSyncRefOneDevice) {
+    QHash<SoundDevicePointer, QList<AudioOutput>> deviceOutputs;
+    QList<SoundDevicePointer> devices;
+
+    auto portAudioDevice1 = SoundDevicePointer(new MockSoundDevice("PortAudioDevice1"));
+    auto portAudioDevice2 = SoundDevicePointer(new MockSoundDevice("PortAudioDevice2"));
+
+    deviceOutputs[portAudioDevice1] = {
+            AudioOutput(AudioPathType::Main,
+                    0,
+                    mixxx::audio::ChannelCount::stereo(),
+                    0),
+            AudioOutput(AudioPathType::Headphones,
+                    0,
+                    mixxx::audio::ChannelCount::stereo(),
+                    0)};
+    deviceOutputs[portAudioDevice2] = {};
+
+    devices.append(portAudioDevice1);
+    devices.append(portAudioDevice2);
+
+    // Test case: Select local time sync reference
+    SoundDevicePointer result = SoundManager::selectLocalTimeSyncRef(deviceOutputs, devices);
+    EXPECT_EQ(result, portAudioDevice1);
+}
+
+TEST(SoundManagerTest, SelectLocalTimeSyncRefTwoDevices) {
+    QHash<SoundDevicePointer, QList<AudioOutput>> deviceOutputs;
+    QList<SoundDevicePointer> devices;
+
+    auto portAudioDevice1 = SoundDevicePointer(new MockSoundDevice("PortAudioDevice1"));
+    auto portAudioDevice2 = SoundDevicePointer(new MockSoundDevice("PortAudioDevice2"));
+
+    deviceOutputs[portAudioDevice1] = {AudioOutput(AudioPathType::Headphones,
+            0,
+            mixxx::audio::ChannelCount::stereo(),
+            0)};
+    deviceOutputs[portAudioDevice2] = {AudioOutput(
+            AudioPathType::Main, 0, mixxx::audio::ChannelCount::stereo(), 0)};
+
+    devices.append(portAudioDevice1);
+    devices.append(portAudioDevice2);
+
+    // Test case: Select local time sync reference
+    SoundDevicePointer result = SoundManager::selectLocalTimeSyncRef(deviceOutputs, devices);
+    EXPECT_EQ(result, portAudioDevice1);
+}
+
+TEST(SoundManagerTest, SelectLocalTimeSyncRefWithNetworkDevice) {
+    QHash<SoundDevicePointer, QList<AudioOutput>> deviceOutputs;
+    QList<SoundDevicePointer> devices;
+
+    auto portAudioDevice1 = SoundDevicePointer(new MockSoundDevice("PortAudioDevice1"));
+    auto portAudioDevice2 = SoundDevicePointer(new MockSoundDevice("PortAudioDevice2"));
+    auto networkDevice = SoundDevicePointer(new MockSoundDevice(kNetworkDeviceInternalName));
+
+    deviceOutputs[portAudioDevice1] = {};
+    deviceOutputs[portAudioDevice2] = {
+            AudioOutput(AudioPathType::Main,
+                    0,
+                    mixxx::audio::ChannelCount::stereo(),
+                    0),
+            AudioOutput(AudioPathType::Headphones,
+                    0,
+                    mixxx::audio::ChannelCount::stereo(),
+                    0)};
+    deviceOutputs[networkDevice] = {AudioOutput(
+            AudioPathType::Main, 0, mixxx::audio::ChannelCount::stereo(), 0)};
+
+    devices.append(portAudioDevice1);
+    devices.append(networkDevice);
+    devices.append(portAudioDevice2);
+
+    // Test case: Select local time sync reference
+    SoundDevicePointer result = SoundManager::selectLocalTimeSyncRef(deviceOutputs, devices);
+    EXPECT_EQ(result->getDeviceId(), portAudioDevice2->getDeviceId());
+}