Merge pull request #16693 from daschuer/gh10516

AdjustReplayGainTest.AdjustReplayGainUpdates fix

Author: JoergAtGithub

src/engine/enginemixer.cpp

@@ -2,7 +2,9 @@
 
 #include <QObject>
 #include <QVarLengthArray>
+#include <array>
 #include <atomic>
+#include <memory>
 
 #include "audio/types.h"
 #include "control/controlobject.h"
@@ -15,6 +17,7 @@
 #include "recording/recordingmanager.h"
 #include "soundio/soundmanager.h"
 #include "soundio/soundmanagerutil.h"
+#include "util/parented_ptr.h"
 #include "util/samplebuffer.h"
 
 class EngineWorkerScheduler;
@@ -68,6 +71,7 @@ class EngineMixer : public QObject, public AudioSource {
 
     // Add an EngineChannel to the mixing engine. This is not thread safe --
     // only call it before the engine has started mixing.
+    // TODO: take std::unique_ptr<EngineChannel> instead.
     void addChannel(EngineChannel* pChannel);
     EngineChannel* getChannel(const QString& group);
     static inline CSAMPLE_GAIN gainForOrientation(EngineChannel::ChannelOrientation orientation,
@@ -87,7 +91,7 @@ class EngineMixer : public QObject, public AudioSource {
 
     // Provide access to the sync lock so enginebuffers can know what their rate controller is.
     EngineSync* getEngineSync() const{
-        return m_pEngineSync;
+        return m_pEngineSync.get();
     }
 
     // These are really only exposed for tests to use.
@@ -100,23 +104,20 @@ class EngineMixer : public QObject, public AudioSource {
     const CSAMPLE* getSidechainBuffer() const;
 
     EngineSideChain* getSideChain() const {
-        return m_pEngineSideChain;
+        return m_pEngineSideChain.get();
     }
 
     CSAMPLE_GAIN getMainGain(int channelIndex) const;
 
     struct ChannelInfo {
         ChannelInfo(int index)
-                : m_pChannel(NULL),
-                  m_pVolumeControl(NULL),
-                  m_pMuteControl(NULL),
-                  m_index(index) {
+                : m_index(index) {
         }
         ChannelHandle m_handle;
-        EngineChannel* m_pChannel;
+        std::unique_ptr<EngineChannel> m_pChannel;
         mixxx::SampleBuffer m_pBuffer;
-        ControlObject* m_pVolumeControl;
-        ControlPushButton* m_pMuteControl;
+        std::unique_ptr<ControlObject> m_pVolumeControl;
+        std::unique_ptr<ControlPushButton> m_pMuteControl;
         GroupFeatureState m_features;
         int m_index;
     };
@@ -242,9 +243,9 @@ class EngineMixer : public QObject, public AudioSource {
 
     // ControlObjects for switching off unnecessary processing
     // These are protected so tests can set them
-    ControlObject* m_pMainEnabled;
-    ControlObject* m_pHeadphoneEnabled;
-    ControlObject* m_pBoothEnabled;
+    std::unique_ptr<ControlObject> m_pMainEnabled;
+    std::unique_ptr<ControlObject> m_pHeadphoneEnabled;
+    std::unique_ptr<ControlObject> m_pBoothEnabled;
 
   private:
     // Processes active channels. The sync lock channel (if any) is processed
@@ -261,11 +262,9 @@ class EngineMixer : public QObject, public AudioSource {
             int iBufferSize);
     bool sidechainMixRequired() const;
 
+    // non-owning. lifetime bound to EffectsManager
     EngineEffectsManager* m_pEngineEffectsManager;
 
-    // List of channels added to the engine.
-    QVarLengthArray<ChannelInfo*, kPreallocatedChannels> m_channels;
-
     // The previous gain of each channel for each mixing output (main,
     // headphone, talkover).
     QVarLengthArray<GainCache, kPreallocatedChannels> m_channelMainGainCache;
@@ -281,42 +280,42 @@ class EngineMixer : public QObject, public AudioSource {
     mixxx::audio::SampleRate m_sampleRate;
 
     // Mixing buffers for each output.
-    mixxx::SampleBuffer m_outputBusBuffers[3];
+    std::array<mixxx::SampleBuffer, 3> m_outputBusBuffers;
     mixxx::SampleBuffer m_booth;
     mixxx::SampleBuffer m_head;
     mixxx::SampleBuffer m_talkover;
     mixxx::SampleBuffer m_talkoverHeadphones;
     mixxx::SampleBuffer m_sidechainMix;
 
-    EngineWorkerScheduler* m_pWorkerScheduler;
-    EngineSync* m_pEngineSync;
-
-    ControlObject* m_pMainGain;
-    ControlObject* m_pBoothGain;
-    ControlObject* m_pHeadGain;
-    ControlObject* m_pSampleRate;
-    ControlObject* m_pOutputLatencyMs;
-    ControlObject* m_pAudioLatencyOverloadCount;
-    ControlObject* m_pAudioLatencyUsage;
-    ControlObject* m_pAudioLatencyOverload;
-    EngineTalkoverDucking* m_pTalkoverDucking;
-    EngineDelay* m_pMainDelay;
-    EngineDelay* m_pHeadDelay;
-    EngineDelay* m_pBoothDelay;
-    EngineDelay* m_pLatencyCompensationDelay;
-
-    EngineVuMeter* m_pVumeter;
-    EngineSideChain* m_pEngineSideChain;
-
-    ControlPotmeter* m_pCrossfader;
-    ControlPotmeter* m_pHeadMix;
-    ControlPotmeter* m_pBalance;
-    ControlPushButton* m_pXFaderMode;
-    ControlPotmeter* m_pXFaderCurve;
-    ControlPotmeter* m_pXFaderCalibration;
-    ControlPushButton* m_pXFaderReverse;
-    ControlPushButton* m_pHeadSplitEnabled;
-    ControlObject* m_pKeylockEngine;
+    parented_ptr<EngineWorkerScheduler> m_pWorkerScheduler;
+    std::unique_ptr<EngineSync> m_pEngineSync;
+
+    std::unique_ptr<ControlObject> m_pMainGain;
+    std::unique_ptr<ControlObject> m_pBoothGain;
+    std::unique_ptr<ControlObject> m_pHeadGain;
+    std::unique_ptr<ControlObject> m_pSampleRate;
+    std::unique_ptr<ControlObject> m_pOutputLatencyMs;
+    std::unique_ptr<ControlObject> m_pAudioLatencyOverloadCount;
+    std::unique_ptr<ControlObject> m_pAudioLatencyUsage;
+    std::unique_ptr<ControlObject> m_pAudioLatencyOverload;
+    std::unique_ptr<EngineTalkoverDucking> m_pTalkoverDucking;
+    std::unique_ptr<EngineDelay> m_pMainDelay;
+    std::unique_ptr<EngineDelay> m_pHeadDelay;
+    std::unique_ptr<EngineDelay> m_pBoothDelay;
+    std::unique_ptr<EngineDelay> m_pLatencyCompensationDelay;
+
+    std::unique_ptr<EngineVuMeter> m_pVumeter;
+    std::unique_ptr<EngineSideChain> m_pEngineSideChain;
+
+    std::unique_ptr<ControlPotmeter> m_pCrossfader;
+    std::unique_ptr<ControlPotmeter> m_pHeadMix;
+    std::unique_ptr<ControlPotmeter> m_pBalance;
+    std::unique_ptr<ControlPushButton> m_pXFaderMode;
+    std::unique_ptr<ControlPotmeter> m_pXFaderCurve;
+    std::unique_ptr<ControlPotmeter> m_pXFaderCalibration;
+    std::unique_ptr<ControlPushButton> m_pXFaderReverse;
+    std::unique_ptr<ControlPushButton> m_pHeadSplitEnabled;
+    std::unique_ptr<ControlObject> m_pKeylockEngine;
 
     PflGainCalculator m_headphoneGain;
     TalkoverGainCalculator m_talkoverGain;
@@ -338,9 +337,14 @@ class EngineMixer : public QObject, public AudioSource {
     const ChannelHandleAndGroup m_busCrossfaderRightHandle;
 
     // Mix two Mono channels. This is useful for outdoor gigs
-    ControlObject* m_pMainMonoMixdown;
-    ControlObject* m_pMicMonitorMode;
+    std::unique_ptr<ControlObject> m_pMainMonoMixdown;
+    std::unique_ptr<ControlObject> m_pMicMonitorMode;
 
+    // TODO (Swiftb0y): remove volatile (probably supposed to be std::atomic instead).
     volatile bool m_bBusOutputConnected[3];
     bool m_bExternalRecordBroadcastInputConnected;
+
+    // Owning list of channels added to the engine.
+    // Keep at last position, it needs to be destroyed first
+    QVarLengthArray<std::unique_ptr<ChannelInfo>, kPreallocatedChannels> m_channels;
 };

src/engine/enginemixer.h

@@ -6,9 +6,9 @@
 #include "util/event.h"
 
 EngineWorkerScheduler::EngineWorkerScheduler(QObject* pParent)
-        : m_bWakeScheduler(false),
+        : QThread(pParent),
+          m_bWakeScheduler(false),
           m_bQuit(false) {
-    Q_UNUSED(pParent);
 }
 
 EngineWorkerScheduler::~EngineWorkerScheduler() {

src/engine/engineworkerscheduler.cpp

@@ -3,25 +3,23 @@
 #include <QMutex>
 #include <QThread>
 #include <QWaitCondition>
-
-// The max engine workers that can be expected to run within a callback
-// (e.g. the max that we will schedule). Must be a power of 2.
-#define MAX_ENGINE_WORKERS 32
+#include <atomic>
+#include <vector>
 
 class EngineWorker;
 
 class EngineWorkerScheduler : public QThread {
     Q_OBJECT
   public:
-    EngineWorkerScheduler(QObject* pParent=NULL);
-    virtual ~EngineWorkerScheduler();
+    EngineWorkerScheduler(QObject* pParent = nullptr);
+    ~EngineWorkerScheduler() override;
 
     void addWorker(EngineWorker* pWorker);
     void runWorkers();
     void workerReady();
 
   protected:
-    void run();
+    void run() override;
 
   private:
     // Indicates whether workerReady has been called since the last time
@@ -32,6 +30,7 @@ class EngineWorkerScheduler : public QThread {
 
     // mutex protects m_workers and m_bQuit
     QMutex m_mutex;
+    // containing pointers are non-owning
     std::vector<EngineWorker*> m_workers;
-    bool m_bQuit;
+    std::atomic<bool> m_bQuit;
 };

src/engine/engineworkerscheduler.h

@@ -39,7 +39,7 @@ class CueControlTest : public BaseSignalPathTest {
     }
 
     void loadTrack(TrackPointer pTrack) {
-        BaseSignalPathTest::loadTrack(m_pMixerDeck1, pTrack);
+        BaseSignalPathTest::loadTrack(m_pMixerDeck1.get(), pTrack);
         ProcessBuffer();
     }
 

src/test/cuecontrol_test.cpp

@@ -24,7 +24,7 @@ class EngineMicrophoneTest : public SignalPathTest {
 
         // No need for a real handle in this test.
         m_pMicrophone = new EngineMicrophone(
-                ChannelHandleAndGroup(ChannelHandle(), "[Microphone]"), m_pEffectsManager);
+                ChannelHandleAndGroup(ChannelHandle(), "[Microphone]"), m_pEffectsManager.get());
         m_pTalkover = ControlObject::getControl(ConfigKey("[Microphone]", "talkover"));
     }
 

src/test/enginemicrophonetest.cpp

@@ -62,7 +62,7 @@ TEST_F(EngineMixerTest, SingleChannelOutputWorks) {
     const QString testName = "SingleChannelOutputWorks";
 
     EngineChannelMock* pChannel = new EngineChannelMock(
-            "[Test1]", EngineChannel::CENTER, m_pEngineMixer);
+            "[Test1]", EngineChannel::CENTER, m_pEngineMixer.get());
     m_pEngineMixer->addChannel(pChannel);
 
     // Pretend that the channel processed the buffer by stuffing it with 1.0's
@@ -106,7 +106,7 @@ TEST_F(EngineMixerTest, SingleChannelPFLOutputWorks) {
     const QString testName = "SingleChannelPFLOutputWorks";
 
     EngineChannelMock* pChannel = new EngineChannelMock(
-            "[Test1]", EngineChannel::CENTER, m_pEngineMixer);
+            "[Test1]", EngineChannel::CENTER, m_pEngineMixer.get());
     m_pEngineMixer->addChannel(pChannel);
 
     // Pretend that the channel processed the buffer by stuffing it with 1.0's
@@ -150,10 +150,10 @@ TEST_F(EngineMixerTest, TwoChannelOutputWorks) {
     const QString testName = "TwoChannelOutputWorks";
 
     EngineChannelMock* pChannel1 = new EngineChannelMock(
-            "[Test1]", EngineChannel::CENTER, m_pEngineMixer);
+            "[Test1]", EngineChannel::CENTER, m_pEngineMixer.get());
     m_pEngineMixer->addChannel(pChannel1);
     EngineChannelMock* pChannel2 = new EngineChannelMock(
-            "[Test2]", EngineChannel::CENTER, m_pEngineMixer);
+            "[Test2]", EngineChannel::CENTER, m_pEngineMixer.get());
     m_pEngineMixer->addChannel(pChannel2);
 
     // Pretend that the channel processed the buffer by stuffing it with 1.0's
@@ -221,10 +221,10 @@ TEST_F(EngineMixerTest, TwoChannelPFLOutputWorks) {
     const QString testName = "TwoChannelPFLOutputWorks";
 
     EngineChannelMock* pChannel1 = new EngineChannelMock(
-            "[Test1]", EngineChannel::CENTER, m_pEngineMixer);
+            "[Test1]", EngineChannel::CENTER, m_pEngineMixer.get());
     m_pEngineMixer->addChannel(pChannel1);
     EngineChannelMock* pChannel2 = new EngineChannelMock(
-            "[Test2]", EngineChannel::CENTER, m_pEngineMixer);
+            "[Test2]", EngineChannel::CENTER, m_pEngineMixer.get());
     m_pEngineMixer->addChannel(pChannel2);
 
     // Pretend that the channel processed the buffer by stuffing it with 1.0's
@@ -292,13 +292,13 @@ TEST_F(EngineMixerTest, ThreeChannelOutputWorks) {
     const QString testName = "ThreeChannelOutputWorks";
 
     EngineChannelMock* pChannel1 = new EngineChannelMock(
-            "[Test1]", EngineChannel::CENTER, m_pEngineMixer);
+            "[Test1]", EngineChannel::CENTER, m_pEngineMixer.get());
     m_pEngineMixer->addChannel(pChannel1);
     EngineChannelMock* pChannel2 = new EngineChannelMock(
-            "[Test2]", EngineChannel::CENTER, m_pEngineMixer);
+            "[Test2]", EngineChannel::CENTER, m_pEngineMixer.get());
     m_pEngineMixer->addChannel(pChannel2);
     EngineChannelMock* pChannel3 = new EngineChannelMock(
-            "[Test3]", EngineChannel::CENTER, m_pEngineMixer);
+            "[Test3]", EngineChannel::CENTER, m_pEngineMixer.get());
     m_pEngineMixer->addChannel(pChannel3);
 
     // Pretend that the channel processed the buffer by stuffing it with 1.0's
@@ -389,13 +389,13 @@ TEST_F(EngineMixerTest, ThreeChannelPFLOutputWorks) {
     const QString testName = "ThreeChannelPFLOutputWorks";
 
     EngineChannelMock* pChannel1 = new EngineChannelMock(
-            "[Test1]", EngineChannel::CENTER, m_pEngineMixer);
+            "[Test1]", EngineChannel::CENTER, m_pEngineMixer.get());
     m_pEngineMixer->addChannel(pChannel1);
     EngineChannelMock* pChannel2 = new EngineChannelMock(
-            "[Test2]", EngineChannel::CENTER, m_pEngineMixer);
+            "[Test2]", EngineChannel::CENTER, m_pEngineMixer.get());
     m_pEngineMixer->addChannel(pChannel2);
     EngineChannelMock* pChannel3 = new EngineChannelMock(
-            "[Test3]", EngineChannel::CENTER, m_pEngineMixer);
+            "[Test3]", EngineChannel::CENTER, m_pEngineMixer.get());
     m_pEngineMixer->addChannel(pChannel3);
 
     // Pretend that the channel processed the buffer by stuffing it with 1.0's

src/test/enginemixertest.cpp

@@ -56,7 +56,7 @@ class HotcueControlTest : public BaseSignalPathTest {
     }
 
     void loadTrack(TrackPointer pTrack) {
-        BaseSignalPathTest::loadTrack(m_pMixerDeck1, pTrack);
+        BaseSignalPathTest::loadTrack(m_pMixerDeck1.get(), pTrack);
         ProcessBuffer();
     }
 

src/test/hotcuecontrol_test.cpp

@@ -165,8 +165,8 @@ TEST_F(AdjustReplayGainTest, AdjustReplayGainUpdatesPregain) {
 
     // Load the same track in decks 1 and 2 so we can see that the pregain is adjusted on both
     // decks.
-    loadTrack(m_pMixerDeck1, pTrack);
-    loadTrack(m_pMixerDeck2, pTrack);
+    loadTrack(m_pMixerDeck1.get(), pTrack);
+    loadTrack(m_pMixerDeck2.get(), pTrack);
 
     // Initialize fake track replaygain so it's not zero.
     mixxx::ReplayGain replayGain;