lp1828212: Fix missing key analysis

build/depends.py

@@ -862,6 +862,7 @@ def sources(self, build):
                    "src/analyzer/plugins/analyzersoundtouchbeats.cpp",
                    "src/analyzer/plugins/analyzerqueenmarybeats.cpp",
                    "src/analyzer/plugins/analyzerqueenmarykey.cpp",
+                   "src/analyzer/plugins/buffering_utils.cpp",
 
                    "src/controllers/controller.cpp",
                    "src/controllers/controllerdebug.cpp",

src/analyzer/plugins/analyzerqueenmarykey.cpp

@@ -7,6 +7,7 @@
 #include "analyzer/plugins/analyzerqueenmarykey.h"
 
 #include "analyzer/constants.h"
+#include "util/assert.h"
 #include "util/math.h"
 
 using mixxx::track::io::key::ChromaticKey;
@@ -26,7 +27,7 @@ constexpr int kTuningFrequencyHertz = 440;
 // See also: https://bugs.launchpad.net/mixxx/+bug/1813413
 QMutex s_mutex;
 
-}  // namespace
+} // namespace
 
 AnalyzerQueenMaryKey::AnalyzerQueenMaryKey()
         : m_currentFrame(0),
@@ -40,30 +41,36 @@ bool AnalyzerQueenMaryKey::initialize(int samplerate) {
     m_prevKey = mixxx::track::io::key::INVALID;
     m_resultKeys.clear();
     m_currentFrame = 0;
-    m_pKeyMode = std::make_unique<GetKeyMode>(samplerate, kTuningFrequencyHertz,
-                                              kChromaWindowLength, kChromaWindowLength);
+    m_pKeyMode = std::make_unique<GetKeyMode>(samplerate, kTuningFrequencyHertz, kChromaWindowLength, kChromaWindowLength);
     size_t windowSize = m_pKeyMode->getBlockSize();
     size_t stepSize = m_pKeyMode->getHopSize();
 
     QMutexLocker locked(&s_mutex);
     return m_helper.initialize(
-        windowSize, stepSize,
-        [this](double* pWindow, size_t) {
-            const int iKey = m_pKeyMode->process(pWindow);
-
-            // Should not happen.
-            if (!ChromaticKey_IsValid(iKey)) {
-                return false;
-            }
-            const auto key = static_cast<ChromaticKey>(iKey);
-            if (key != m_prevKey) {
-                // TODO(rryan) reserve?
-                m_resultKeys.push_back(qMakePair(
-                    key, static_cast<double>(m_currentFrame)));
-                m_prevKey = key;
-            }
-            return true;
-        });
+            windowSize, stepSize, [this](double* pWindow, size_t) {
+                int iKey = m_pKeyMode->process(pWindow);
+                // NOTE(uklotzde): Due to a rounding error in GetKeyMode::process()
+                // the Queen Mary analyzer v1.7.1 returns values in the range [1, 25]
+                // instead of [1, 24]. Instead of patching the original file GetKeyMode.cpp
+                // we compensate this error here to avoid losing the fix after reimporting
+                // the QM plugins!
+                if (iKey == 25) {
+                    iKey = 24;
+                }
+
+                VERIFY_OR_DEBUG_ASSERT(ChromaticKey_IsValid(iKey)) {
+                    qWarning() << "No valid key detected in analyzed window:" << iKey;
+                    return false;
+                }
+                const auto key = static_cast<ChromaticKey>(iKey);
+                if (key != m_prevKey) {
+                    // TODO(rryan) reserve?
+                    m_resultKeys.push_back(qMakePair(
+                            key, static_cast<double>(m_currentFrame)));
+                    m_prevKey = key;
+                }
+                return true;
+            });
 }
 
 bool AnalyzerQueenMaryKey::process(const CSAMPLE* pIn, const int iLen) {
@@ -88,4 +95,4 @@ bool AnalyzerQueenMaryKey::finalize() {
     return true;
 }
 
-}  // namespace mixxx
+} // namespace mixxx

src/analyzer/plugins/analyzerqueenmarykey.h

@@ -18,9 +18,9 @@ class AnalyzerQueenMaryKey : public AnalyzerKeyPlugin {
   public:
     static AnalyzerPluginInfo pluginInfo() {
         return AnalyzerPluginInfo(
-            "qm-keydetector",
-            QObject::tr("Queen Mary University London"),
-            QObject::tr("Queen Mary Key Detector"));
+                "qm-keydetector",
+                QObject::tr("Queen Mary University London"),
+                QObject::tr("Queen Mary Key Detector"));
     }
 
     AnalyzerQueenMaryKey();
@@ -46,6 +46,6 @@ class AnalyzerQueenMaryKey : public AnalyzerKeyPlugin {
     mixxx::track::io::key::ChromaticKey m_prevKey;
 };
 
-}  // namespace mixxx
+} // namespace mixxx
 
 #endif /* ANALYZER_PLUGINS_ANALYZERQUEENMARYKEY_H */

src/analyzer/plugins/buffering_utils.cpp

@@ -0,0 +1,60 @@
+#include "analyzer/plugins/buffering_utils.h"
+
+#include "util/math.h"
+
+namespace mixxx {
+
+bool DownmixAndOverlapHelper::initialize(size_t windowSize, size_t stepSize, WindowReadyCallback callback) {
+    m_buffer.resize(windowSize);
+    m_callback = callback;
+    m_windowSize = windowSize;
+    m_stepSize = stepSize;
+    m_bufferWritePosition = 0;
+    return m_windowSize > 0 && m_stepSize > 0 &&
+            m_stepSize <= m_windowSize && callback;
+}
+
+bool DownmixAndOverlapHelper::processStereoSamples(const CSAMPLE* pInput, size_t inputStereoSamples) {
+    const size_t numInputFrames = inputStereoSamples / 2;
+    size_t inRead = 0;
+    double* pDownmix = m_buffer.data();
+
+    while (inRead < numInputFrames) {
+        size_t writeAvailable = math_min(numInputFrames,
+                m_windowSize - m_bufferWritePosition);
+
+        for (size_t i = 0; i < writeAvailable; ++i) {
+            // We analyze a mono downmix of the signal since we don't think
+            // stereo does us any good.
+            pDownmix[m_bufferWritePosition + i] = (pInput[(inRead + i) * 2] +
+                                                          pInput[(inRead + i) * 2 + 1]) *
+                    0.5;
+        }
+        m_bufferWritePosition += writeAvailable;
+        inRead += writeAvailable;
+
+        if (m_bufferWritePosition == m_windowSize) {
+            bool result = m_callback(pDownmix, m_windowSize);
+
+            // If the callback said not to continue then stop.
+            if (!result) {
+                return false;
+            }
+
+            // If the window size equals the step size then this will result
+            // in m_bufferWritePosition == 0.
+            for (size_t i = 0; i < (m_windowSize - m_stepSize); ++i) {
+                pDownmix[i] = pDownmix[i + m_stepSize];
+            }
+            m_bufferWritePosition -= m_stepSize;
+        }
+    }
+    return true;
+}
+
+bool DownmixAndOverlapHelper::finalize() {
+    // TODO(rryan) flush support?
+    return true;
+}
+
+}

src/analyzer/plugins/buffering_utils.h

@@ -1,69 +1,30 @@
-#ifndef ANALYZER_PLUGINS_BUFFERING_UTILS_H
-#define ANALYZER_PLUGINS_BUFFERING_UTILS_H
+#pragma once
 
-#include "util/math.h"
+#include <vector>
+
+#include "util/types.h"
 
 namespace mixxx {
 
 // This is used for downmixing a stereo buffer into mono and framing it into
 // overlapping windows as is typically necessary when taking a short-time
 // Fourier transform.
 class DownmixAndOverlapHelper {
-    typedef std::function<bool(double* pBuffer, size_t frames)> WindowReadyCallback;
   public:
     DownmixAndOverlapHelper() = default;
 
-    bool initialize(size_t windowSize, size_t stepSize, WindowReadyCallback callback) {
-        m_buffer.resize(windowSize);
-        m_callback = callback;
-        m_windowSize = windowSize;
-        m_stepSize = stepSize;
-        m_bufferWritePosition = 0;
-        return m_windowSize > 0 && m_stepSize > 0 &&
-                m_stepSize <= m_windowSize && callback;
-    }
-
-    bool processStereoSamples(const CSAMPLE* pInput, size_t inputStereoSamples) {
-        const size_t numInputFrames = inputStereoSamples / 2;
-        size_t inRead = 0;
-        double* pDownmix = m_buffer.data();
-
-        while (inRead < numInputFrames) {
-            size_t writeAvailable = math_min(numInputFrames,
-                                             m_windowSize - m_bufferWritePosition);
-
-            for (size_t i = 0; i < writeAvailable; ++i) {
-                // We analyze a mono downmix of the signal since we don't think
-                // stereo does us any good.
-                pDownmix[m_bufferWritePosition + i] = (pInput[(inRead + i) * 2] +
-                                                       pInput[(inRead + i) * 2 + 1]) * 0.5;
-            }
-            m_bufferWritePosition += writeAvailable;
-            inRead += writeAvailable;
-
-            if (m_bufferWritePosition == m_windowSize) {
-                bool result = m_callback(pDownmix, m_windowSize);
+    typedef std::function<bool(double* pBuffer, size_t frames)> WindowReadyCallback;
 
-                // If the callback said not to continue then stop.
-                if (!result) {
-                    return false;
-                }
+    bool initialize(
+            size_t windowSize,
+            size_t stepSize,
+            WindowReadyCallback callback);
 
-                // If the window size equals the step size then this will result
-                // in m_bufferWritePosition == 0.
-                for (size_t i = 0; i < (m_windowSize - m_stepSize); ++i) {
-                    pDownmix[i] = pDownmix[i + m_stepSize];
-                }
-                m_bufferWritePosition -= m_stepSize;
-            }
-        }
-        return true;
-    }
+    bool processStereoSamples(
+            const CSAMPLE* pInput,
+            size_t inputStereoSamples);
 
-    bool finalize() {
-        // TODO(rryan) flush support?
-        return true;
-    }
+    bool finalize();
 
   private:
     std::vector<double> m_buffer;
@@ -75,6 +36,4 @@ class DownmixAndOverlapHelper {
     WindowReadyCallback m_callback;
 };
 
-}  // namespace mixxx
-
-#endif /* ANALYZER_PLUGINS_BUFFERING_UTILS_H */
+} // namespace mixxx