Merge branch '2.3' of git@github.com:mixxxdj/mixxx.git into main

7 files changed, 183 insertions, 87 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8a0ba8d109..686f9ea4a3 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -85,6 +85,7 @@
 * Fix loss of precision when dealing with floating-point sample positions while setting loop out position and seeking using vinyl control [#3126](https://github.com/mixxxdj/mixxx/pull/3126) [#3127](https://github.com/mixxxdj/mixxx/pull/3127)
 * Prevent moving a loop beyond track end [#3117](https://github.com/mixxxdj/mixxx/pull/3117) [lp:1799574](https://bugs.launchpad.net/mixxx/+bug/1799574)
 * Use 6 instead of only 4 compatible musical keys (major/minor) [#3205](https://github.com/mixxxdj/mixxx/pull/3205)
+* Fix possible memory corruption using JACK on Linux [#3160](https://github.com/mixxxdj/mixxx/pull/3160)
 
 ## [2.2.4](https://launchpad.net/mixxx/+milestone/2.2.4) (2020-06-27)
 
diff --git a/src/engine/controls/bpmcontrol.cpp b/src/engine/controls/bpmcontrol.cpp
index 48ab0453cc..049a9e791b 100644
--- a/src/engine/controls/bpmcontrol.cpp
+++ b/src/engine/controls/bpmcontrol.cpp
@@ -436,6 +436,7 @@ double BpmControl::calcSyncAdjustment(bool userTweakingSync) {
         kLogger.trace() << m_group << "****************";
         kLogger.trace() << "master beat distance:" << syncTargetBeatDistance;
         kLogger.trace() << "my     beat distance:" << thisBeatDistance;
+        kLogger.trace() << "user offset distance:" << m_dUserOffset.getValue();
         kLogger.trace() << "error               :"
                         << (shortest_distance - m_dUserOffset.getValue());
         kLogger.trace() << "user offset         :" << m_dUserOffset.getValue();
@@ -1011,6 +1012,9 @@ double BpmControl::updateBeatDistance() {
 }
 
 void BpmControl::setTargetBeatDistance(double beatDistance) {
+    if (kLogger.traceEnabled()) {
+        qDebug() << getGroup() << "BpmControl::setTargetBeatDistance:" << beatDistance;
+    }
     m_dSyncTargetBeatDistance.setValue(beatDistance);
 }
 
diff --git a/src/engine/sync/internalclock.cpp b/src/engine/sync/internalclock.cpp
index 0f598743c0..9b1b1a819d 100644
--- a/src/engine/sync/internalclock.cpp
+++ b/src/engine/sync/internalclock.cpp
@@ -20,7 +20,6 @@ InternalClock::InternalClock(const QString& group, SyncableListener* pEngineSync
           m_iOldSampleRate(44100),
           m_dOldBpm(124.0),
           m_dBaseBpm(124.0),
-          m_bClockUpdated(false),
           m_dBeatLength(m_iOldSampleRate * 60.0 / m_dOldBpm),
           m_dClockPosition(0) {
     // Pick a wide range (1 to 200) and allow out of bounds sets. This lets you
@@ -101,7 +100,6 @@ void InternalClock::setMasterBeatDistance(double beatDistance) {
     if (kLogger.traceEnabled()) {
         kLogger.trace() << "InternalClock::setMasterBeatDistance" << beatDistance;
     }
-    m_bClockUpdated.fetchAndStoreAcquire(true);
     m_dClockPosition = beatDistance * m_dBeatLength;
     m_pClockBeatDistance->set(beatDistance);
     // Make sure followers have an up-to-date beat distance.
@@ -202,19 +200,12 @@ void InternalClock::updateBeatLength(int sampleRate, double bpm) {
 void InternalClock::onCallbackStart(int sampleRate, int bufferSize) {
     Q_UNUSED(sampleRate)
     Q_UNUSED(bufferSize)
-    m_bClockUpdated.fetchAndStoreAcquire(false);
     m_pEngineSync->notifyInstantaneousBpmChanged(this, getBpm());
 }
 
 void InternalClock::onCallbackEnd(int sampleRate, int bufferSize) {
     updateBeatLength(sampleRate, m_pClockBpm->get());
 
-    // If the clock was updated mid-call due to seeks or other updates from other decks, skip the
-    // incrementing of the clock.
-    if (m_bClockUpdated.fetchAndStoreRelaxed(false)) {
-        return;
-    }
-
     // stereo samples, so divide by 2
     m_dClockPosition += bufferSize / 2;
 
diff --git a/src/engine/sync/internalclock.h b/src/engine/sync/internalclock.h
index c5093e32c2..aa2e6b989c 100644
--- a/src/engine/sync/internalclock.h
+++ b/src/engine/sync/internalclock.h
@@ -75,7 +75,6 @@ class InternalClock : public QObject, public Clock, public Syncable {
     int m_iOldSampleRate;
     double m_dOldBpm;
     double m_dBaseBpm;
-    QAtomicInteger<bool> m_bClockUpdated;
 
     // The internal clock rate is stored in terms of samples per beat.
     // Fractional values are allowed.
diff --git a/src/soundio/sounddeviceportaudio.cpp b/src/soundio/sounddeviceportaudio.cpp
index 2af44a3b79..53d7370452 100644
--- a/src/soundio/sounddeviceportaudio.cpp
+++ b/src/soundio/sounddeviceportaudio.cpp
@@ -1,32 +1,10 @@
-/***************************************************************************
-                          sounddeviceportaudio.cpp
-                             -------------------
-    begin                : Sun Aug 15, 2007 (Stardate -315378.5417935057)
-    copyright            : (C) 2007 Albert Santoni
-    email                : gamegod \a\t users.sf.net
-***************************************************************************/
-
-/***************************************************************************
-*                                                                         *
-*   This program is free software; you can redistribute it and/or modify  *
-*   it under the terms of the GNU General Public License as published by  *
-*   the Free Software Foundation; either version 2 of the License, or     *
-*   (at your option) any later version.                                   *
-*                                                                         *
-***************************************************************************/
-
 #include "soundio/sounddeviceportaudio.h"
 
-#include <portaudio.h>
 #include <float.h>
 
-#include <QtDebug>
 #include <QRegularExpression>
 #include <QThread>
-
-#ifdef __LINUX__
-#include <QLibrary>
-#endif
+#include <QtDebug>
 
 #include "control/controlobject.h"
 #include "control/controlproxy.h"
@@ -34,21 +12,28 @@
 #include "soundio/soundmanager.h"
 #include "soundio/soundmanagerutil.h"
 #include "util/denormalsarezero.h"
+#include "util/fifo.h"
+#include "util/math.h"
 #include "util/sample.h"
 #include "util/timer.h"
 #include "util/trace.h"
-#include "util/math.h"
 #include "vinylcontrol/defs_vinylcontrol.h"
 #include "waveform/visualplayposition.h"
 
+#ifdef __LINUX__
+// for PaAlsa_EnableRealtimeScheduling
+#include <pa_linux_alsa.h>
+#endif
 
 namespace {
 
 // Buffer for drift correction 1 full, 1 for r/w, 1 empty
-const int kDriftReserve = 1;
+constexpr int kDriftReserve = 1;
 
 // Buffer for drift correction 1 full, 1 for r/w, 1 empty
-const int kFifoSize = 2 * kDriftReserve + 1;
+constexpr int kFifoSize = 2 * kDriftReserve + 1;
+
+constexpr int kCpuUsageUpdateRate = 30; // in 1/s, fits to display frame rate
 
 // We warn only at invalid timing 3, since the first two
 // callbacks can be always wrong due to a setup/open jitter
@@ -352,20 +337,9 @@ SoundDeviceError SoundDevicePortAudio::open(bool isClkRefDevice, int syncBuffers
 
 
 #ifdef __LINUX__
-    //Attempt to dynamically load and resolve stuff in the PortAudio library
-    //in order to enable RT priority with ALSA.
-    QLibrary portaudio("libportaudio.so.2");
-    if (!portaudio.load())
-       qWarning() << "Failed to dynamically load PortAudio library";
-    else
-       qDebug() << "Dynamically loaded PortAudio library";
-
-    EnableAlsaRT enableRealtime = (EnableAlsaRT) portaudio.resolve(
-            "PaAlsa_EnableRealtimeScheduling");
-    if (enableRealtime) {
-        enableRealtime(pStream, 1);
+    if (m_deviceInfo->hostApi == paALSA) {
+        PaAlsa_EnableRealtimeScheduling(pStream, 1);
     }
-    portaudio.unload();
 #endif
 
     // Start stream
@@ -1074,8 +1048,7 @@ void SoundDevicePortAudio::updateCallbackEntryToDacTime(
 void SoundDevicePortAudio::updateAudioLatencyUsage(
         const SINT framesPerBuffer) {
     m_framesSinceAudioLatencyUsageUpdate += framesPerBuffer;
-    if (m_framesSinceAudioLatencyUsageUpdate
-            > (m_dSampleRate / CPU_USAGE_UPDATE_RATE)) {
+    if (m_framesSinceAudioLatencyUsageUpdate > (m_dSampleRate / kCpuUsageUpdateRate)) {
         double secInAudioCb = m_timeInAudioCallback.toDoubleSeconds();
         m_pMasterAudioLatencyUsage->set(
                 secInAudioCb
diff --git a/src/soundio/sounddeviceportaudio.h b/src/soundio/sounddeviceportaudio.h
index 91cdc22f0d..cd1feb3655 100644
--- a/src/soundio/sounddeviceportaudio.h
+++ b/src/soundio/sounddeviceportaudio.h
@@ -1,42 +1,17 @@
-/***************************************************************************
-                          sounddeviceportaudio.cpp
-                             -------------------
-    begin                : Sun Aug 15, 2007 (Stardate -315378.5417935057)
-    copyright            : (C) 2007 Albert Santoni
-    email                : gamegod \a\t users.sf.net
- ***************************************************************************/
+#pragma once
 
-/***************************************************************************
- *                                                                         *
- *   This program is free software; you can redistribute it and/or modify  *
- *   it under the terms of the GNU General Public License as published by  *
- *   the Free Software Foundation; either version 2 of the License, or     *
- *   (at your option) any later version.                                   *
- *                                                                         *
- ***************************************************************************/
-
-#ifndef SOUNDDEVICEPORTAUDIO_H
-#define SOUNDDEVICEPORTAUDIO_H
 
 #include <portaudio.h>
-
+#include <QHash>
 #include <QString>
-#include "util/performancetimer.h"
 
 #include "soundio/sounddevice.h"
 #include "util/duration.h"
-#include "util/fifo.h"
-
-#define CPU_USAGE_UPDATE_RATE 30 // in 1/s, fits to display frame rate
+#include "util/performancetimer.h"
 
 class SoundManager;
 class ControlProxy;
 
-/** Dynamically resolved function which allows us to enable a realtime-priority callback
-    thread from ALSA/PortAudio. This must be dynamically resolved because PortAudio can't
-    tell us if ALSA is compiled into it or not. */
-typedef int (*EnableAlsaRT)(PaStream* s, int enable);
-
 class SoundDevicePortAudio : public SoundDevice {
   public:
     SoundDevicePortAudio(UserSettingsPointer config,
@@ -104,4 +79,3 @@ class SoundDevicePortAudio : public SoundDevice {
     PaTime m_lastCallbackEntrytoDacSecs;
 };
 
-#endif
diff --git a/src/test/enginesynctest.cpp b/src/test/enginesynctest.cpp
index e1c22db18e..44721fa986 100644
--- a/src/test/enginesynctest.cpp
+++ b/src/test/enginesynctest.cpp
@@ -1,7 +1,6 @@
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
-#include <QtDebug>
 #include <string>
 
 #include "control/controlobject.h"
@@ -1344,19 +1343,68 @@ TEST_F(EngineSyncTest, ZeroBPMRateAdjustIgnored) {
             ControlObject::getControl(ConfigKey(m_sGroup2, "rate"))->get());
 }
 
-TEST_F(EngineSyncTest, ZeroLatencyRateChange) {
+TEST_F(EngineSyncTest, ZeroLatencyRateChangeNoQuant) {
     // Confirm that a rate change in an explicit master is instantly communicated
     // to followers.
     mixxx::BeatsPointer pBeats1 = BeatFactory::makeBeatGrid(*m_pTrack1, 128, 0.0);
     m_pTrack1->setBeats(pBeats1);
-    mixxx::BeatsPointer pBeats2 = BeatFactory::makeBeatGrid(*m_pTrack2, 128, 0.0);
+    mixxx::BeatsPointer pBeats2 = BeatFactory::makeBeatGrid(*m_pTrack2, 160, 0.0);
+    m_pTrack2->setBeats(pBeats2);
+
+    // Make Channel2 master to weed out any channel ordering issues.
+    ControlObject::getControl(ConfigKey(m_sGroup2, "sync_mode"))
+            ->set(SYNC_FOLLOWER);
+    ControlObject::getControl(ConfigKey(m_sGroup1, "sync_mode"))
+            ->set(SYNC_FOLLOWER);
+    // Exaggerate the effect with a high rate.
+    ControlObject::set(ConfigKey(m_sGroup2, "rate_ratio"), 10.0);
+
+    ControlObject::getControl(ConfigKey(m_sGroup1, "play"))->set(1.0);
+    ControlObject::getControl(ConfigKey(m_sGroup2, "play"))->set(1.0);
+
+    EXPECT_EQ(ControlObject::getControl(ConfigKey(m_sGroup2, "beat_distance"))
+                      ->get(),
+            ControlObject::getControl(ConfigKey(m_sGroup1, "beat_distance"))
+                    ->get());
+
+    for (int i = 0; i < 50; ++i) {
+        ProcessBuffer();
+        // Keep messing with the rate
+        float rate = i % 2 == 0 ? i / 10.0 : i / -10.0;
+        ControlObject::set(ConfigKey(m_sGroup2, "rate_ratio"), rate);
+
+        // Buffers should be in sync.
+        ASSERT_FLOAT_EQ(ControlObject::getControl(ConfigKey(m_sGroup2, "beat_distance"))
+                                ->get(),
+                ControlObject::getControl(ConfigKey(m_sGroup1, "beat_distance"))
+                        ->get());
+    }
+
+    // Make sure we're actually going somewhere!
+    EXPECT_GT(ControlObject::getControl(ConfigKey(m_sGroup1, "beat_distance"))
+                      ->get(),
+            0);
+    // Buffers should be in sync.
+    EXPECT_EQ(ControlObject::getControl(ConfigKey(m_sGroup2, "beat_distance"))
+                      ->get(),
+            ControlObject::getControl(ConfigKey(m_sGroup1, "beat_distance"))
+                    ->get());
+}
+
+TEST_F(EngineSyncTest, ZeroLatencyRateChangeQuant) {
+    // Confirm that a rate change in an explicit master is instantly communicated
+    // to followers.
+    mixxx::BeatsPointer pBeats1 = BeatFactory::makeBeatGrid(*m_pTrack1, 128, 0.0);
+    m_pTrack1->setBeats(pBeats1);
+    mixxx::BeatsPointer pBeats2 = BeatFactory::makeBeatGrid(*m_pTrack2, 160, 0.0);
     m_pTrack2->setBeats(pBeats2);
 
     ControlObject::getControl(ConfigKey(m_sGroup1, "quantize"))->set(1.0);
     ControlObject::getControl(ConfigKey(m_sGroup2, "quantize"))->set(1.0);
+
     // Make Channel2 master to weed out any channel ordering issues.
     ControlObject::getControl(ConfigKey(m_sGroup2, "sync_mode"))
-            ->set(SYNC_MASTER_EXPLICIT);
+            ->set(SYNC_FOLLOWER);
     ControlObject::getControl(ConfigKey(m_sGroup1, "sync_mode"))
             ->set(SYNC_FOLLOWER);
     // Exaggerate the effect with a high rate.
@@ -1370,14 +1418,121 @@ TEST_F(EngineSyncTest, ZeroLatencyRateChange) {
             ControlObject::getControl(ConfigKey(m_sGroup1, "beat_distance"))
                     ->get());
 
-    ProcessBuffer();
-    ProcessBuffer();
+    for (int i = 0; i < 50; ++i) {
+        ProcessBuffer();
+        // Keep messing with the rate
+        float rate = i % 2 == 0 ? i / 10.0 : i / -10.0;
+        ControlObject::set(ConfigKey(m_sGroup2, "rate_ratio"), rate);
+
+        // Buffers should be in sync.
+        ASSERT_FLOAT_EQ(ControlObject::getControl(ConfigKey(m_sGroup2, "beat_distance"))
+                                ->get(),
+                ControlObject::getControl(ConfigKey(m_sGroup1, "beat_distance"))
+                        ->get());
+    }
+
+    // Make sure we're actually going somewhere!
+    EXPECT_GT(ControlObject::getControl(ConfigKey(m_sGroup1, "beat_distance"))
+                      ->get(),
+            0);
+    // Buffers should be in sync.
+    EXPECT_EQ(ControlObject::getControl(ConfigKey(m_sGroup2, "beat_distance"))
+                      ->get(),
+            ControlObject::getControl(ConfigKey(m_sGroup1, "beat_distance"))
+                    ->get());
+}
+
+TEST_F(EngineSyncTest, ZeroLatencyRateDiffQuant) {
+    // Confirm that a rate change in an explicit master is instantly communicated
+    // to followers.
+    mixxx::BeatsPointer pBeats1 = BeatFactory::makeBeatGrid(*m_pTrack1, 128, 0.0);
+    m_pTrack1->setBeats(pBeats1);
+    mixxx::BeatsPointer pBeats2 = BeatFactory::makeBeatGrid(*m_pTrack2, 160, 0.0);
+    m_pTrack2->setBeats(pBeats2);
+
+    ControlObject::getControl(ConfigKey(m_sGroup2, "quantize"))->set(1.0);
+
+    // Make Channel2 master to weed out any channel ordering issues.
+    ControlObject::getControl(ConfigKey(m_sGroup2, "sync_mode"))
+            ->set(SYNC_FOLLOWER);
+    ControlObject::getControl(ConfigKey(m_sGroup1, "sync_mode"))
+            ->set(SYNC_FOLLOWER);
+    // Exaggerate the effect with a high rate.
+    ControlObject::set(ConfigKey(m_sGroup2, "rate_ratio"), 10.0);
+
+    ControlObject::getControl(ConfigKey(m_sGroup1, "play"))->set(1.0);
+    ControlObject::getControl(ConfigKey(m_sGroup2, "play"))->set(1.0);
+
+    EXPECT_EQ(ControlObject::getControl(ConfigKey(m_sGroup2, "beat_distance"))
+                      ->get(),
+            ControlObject::getControl(ConfigKey(m_sGroup1, "beat_distance"))
+                    ->get());
+
+    for (int i = 0; i < 50; ++i) {
+        ProcessBuffer();
+        // Keep messing with the rate
+        float rate = i % 2 == 0 ? i / 10.0 : i / -10.0;
+        ControlObject::set(ConfigKey(m_sGroup2, "rate_ratio"), rate);
+
+        // Buffers should be in sync.
+        ASSERT_FLOAT_EQ(ControlObject::getControl(ConfigKey(m_sGroup2, "beat_distance"))
+                                ->get(),
+                ControlObject::getControl(ConfigKey(m_sGroup1, "beat_distance"))
+                        ->get());
+    }
+
+    // Make sure we're actually going somewhere!
+    EXPECT_GT(ControlObject::getControl(ConfigKey(m_sGroup1, "beat_distance"))
+                      ->get(),
+            0);
+    // Buffers should be in sync.
+    EXPECT_EQ(ControlObject::getControl(ConfigKey(m_sGroup2, "beat_distance"))
+                      ->get(),
+            ControlObject::getControl(ConfigKey(m_sGroup1, "beat_distance"))
+                    ->get());
+}
+
+// In this test, we set play *first* and then turn on master sync.
+// This exercises a slightly different ordering of signals that we
+// need to check. The Sync feature is unfortunately brittle.
+// This test exercises https://bugs.launchpad.net/mixxx/+bug/1884324
+TEST_F(EngineSyncTest, ActivatingSyncDoesNotCauseDrifting) {
+    mixxx::BeatsPointer pBeats1 = BeatFactory::makeBeatGrid(*m_pTrack1, 150, 0.0);
+    m_pTrack1->setBeats(pBeats1);
+    mixxx::BeatsPointer pBeats2 = BeatFactory::makeBeatGrid(*m_pTrack2, 150, 0.0);
+    m_pTrack2->setBeats(pBeats2);
+
+    ControlObject::getControl(ConfigKey(m_sGroup1, "quantize"))->set(0.0);
+    ControlObject::getControl(ConfigKey(m_sGroup2, "quantize"))->set(1.0);
+
+    ControlObject::getControl(ConfigKey(m_sGroup1, "play"))->set(1.0);
+    ControlObject::getControl(ConfigKey(m_sGroup2, "play"))->set(1.0);
+
     ProcessBuffer();
 
+    // make sure we aren't out-of-sync from the start
+    EXPECT_EQ(ControlObject::getControl(ConfigKey(m_sGroup2, "beat_distance"))
+                      ->get(),
+            ControlObject::getControl(ConfigKey(m_sGroup1, "beat_distance"))
+                    ->get());
+
+    // engage first sync-master
+    ControlObject::getControl(ConfigKey(m_sGroup1, "sync_mode"))
+            ->set(SYNC_FOLLOWER);
+
+    // engage second Sync-master
+    ControlObject::getControl(ConfigKey(m_sGroup2, "sync_mode"))
+            ->set(SYNC_FOLLOWER);
+
+    // Run for a number of buffers
+    for (int i = 0; i < 25; ++i) {
+        ProcessBuffer();
+    }
     // Make sure we're actually going somewhere!
     EXPECT_GT(ControlObject::getControl(ConfigKey(m_sGroup1, "beat_distance"))
                       ->get(),
             0);
+
     // Buffers should be in sync.
     EXPECT_EQ(ControlObject::getControl(ConfigKey(m_sGroup2, "beat_distance"))
                       ->get(),
@@ -1419,7 +1574,6 @@ TEST_F(EngineSyncTest, HalfDoubleBpmTest) {
 
     // Do lots of processing to make sure we get over the 0.5 beat_distance barrier.
     for (int i = 0; i < 50; ++i) {
-        qDebug() << "bpm test loop iter" << i;
         ProcessBuffer();
         // The beat distances are NOT as simple as x2 or /2.  Use the built-in functions
         // to do the proper conversion.