Merge remote-tracking branch 'upstream/2.4' into 2.5

.pre-commit-config.yaml

@@ -32,7 +32,7 @@ default_language_version:
   rust: 1.64.0
 repos:
   - repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v4.6.0
+    rev: v5.0.0
     hooks:
       - id: fix-byte-order-marker
         exclude: ^.*(\.cbproj|\.groupproj|\.props|\.sln|\.vcxproj|\.vcxproj.filters|UTF-8-BOM.txt)$
@@ -66,18 +66,18 @@ repos:
           ]
         exclude: ^(packaging/wix/LICENSE.rtf.in|src/dialog/dlgabout\.cpp|.*\.(?:pot?|(?<!d\.)ts|wxl|svg))$
   - repo: https://github.com/pre-commit/mirrors-eslint
-    rev: v9.8.0
+    rev: v9.14.0
     hooks:
       - id: eslint
         args: [--fix, --report-unused-disable-directives]
         files: \.m?[jt]s$
         types: [file]
         stages:
-          - commit
+          - pre-commit
           - manual
         additional_dependencies:
-          - eslint@^8.48.0
-          - eslint-plugin-jsdoc@^v46.5.1
+          - eslint@^9.14.0
+          - eslint-plugin-jsdoc@^v50.4.3
           - "@typescript-eslint/eslint-plugin"
           - "@typescript-eslint/parser"
           - eslint-plugin-diff@^2.0.3
@@ -89,19 +89,19 @@ repos:
         entry: python tools/clang_format.py
         require_serial: true
         stages:
-          - commit
+          - pre-commit
           - manual
         language: python
         additional_dependencies:
-          - clang-format==16.0.6
+          - clang-format==19.1.3
         files: \.(c|cc|cxx|cpp|frag|glsl|h|hpp|hxx|ih|ispc|ipp|java|m|mm|proto|vert)$
   - repo: https://github.com/psf/black
-    rev: 24.4.2
+    rev: 24.10.0
     hooks:
       - id: black
         files: ^tools/.*$
   - repo: https://github.com/pycqa/flake8
-    rev: "7.1.0"
+    rev: "7.1.1"
     hooks:
       - id: flake8
         files: ^tools/.*$
@@ -111,11 +111,11 @@ repos:
     hooks:
       - id: shellcheck
   - repo: https://github.com/DavidAnson/markdownlint-cli2
-    rev: v0.13.0
+    rev: v0.14.0
     hooks:
       - id: markdownlint-cli2
   - repo: https://github.com/python-jsonschema/check-jsonschema
-    rev: 0.29.1
+    rev: 0.29.4
     hooks:
       - id: check-github-workflows
   - repo: https://github.com/pre-commit/mirrors-prettier
@@ -141,7 +141,7 @@ repos:
         types: [text]
         files: ^.*\.qss$
         stages:
-          - commit
+          - pre-commit
           - manual
       - id: changelog
         name: changelog
@@ -165,8 +165,8 @@ repos:
         pass_filenames: false
         language: python
         additional_dependencies:
-          - beautifulsoup4==4.11.1
-          - lxml==4.9.3
-          - Markdown==3.4.1
+          - beautifulsoup4==4.12.3
+          - lxml==5.3.0
+          - Markdown==3.7
         types: [text]
         files: ^(CHANGELOG\.md|res/linux/org\.mixxx\.Mixxx\.metainfo.xml)$

src/controllers/hid/hidioglobaloutputreportfifo.cpp

@@ -15,7 +15,8 @@ constexpr size_t kSizeOfFifoInReports = 32;
 } // namespace
 
 HidIoGlobalOutputReportFifo::HidIoGlobalOutputReportFifo()
-        : m_fifoQueue(kSizeOfFifoInReports) {
+        : m_fifoQueue(kSizeOfFifoInReports),
+          m_hidWriteErrorLogged(false) {
 }
 
 void HidIoGlobalOutputReportFifo::addReportDatasetToFifo(const quint8 reportId,
@@ -70,10 +71,21 @@ bool HidIoGlobalOutputReportFifo::sendNextReportDataset(QMutex* pHidDeviceAndPol
             reinterpret_cast<const unsigned char*>(reportToSend.constData()),
             reportToSend.size());
     if (result == -1) {
-        qCWarning(logOutput) << "Unable to send data to" << deviceInfo.formatName() << ":"
-                             << mixxx::convertWCStringToQString(
-                                        hid_error(pHidDevice),
-                                        kMaxHidErrorMessageSize);
+        if (!m_hidWriteErrorLogged) {
+            qCWarning(logOutput)
+                    << "Unable to send data to" << deviceInfo.formatName()
+                    << ":"
+                    << mixxx::convertWCStringToQString(
+                               hid_error(pHidDevice), kMaxHidErrorMessageSize)
+                    << "Note that, this message is only logged once and may "
+                       "not appear again until all hid_read errors have "
+                       "disappeared.";
+
+            // Stop logging error messages if every hid_write() fails to avoid large log files
+            m_hidWriteErrorLogged = true;
+        }
+    } else {
+        m_hidWriteErrorLogged = false;
     }
 
     hidDeviceLock.unlock();

src/controllers/hid/hidioglobaloutputreportfifo.h

@@ -38,4 +38,5 @@ class HidIoGlobalOutputReportFifo {
   private:
     // Lockless FIFO queue
     rigtorp::SPSCQueue<QByteArray> m_fifoQueue;
+    bool m_hidWriteErrorLogged;
 };

src/controllers/hid/hidiooutputreport.cpp

@@ -16,6 +16,7 @@ constexpr size_t kMaxHidErrorMessageSize = 512;
 HidIoOutputReport::HidIoOutputReport(
         const quint8& reportId, const unsigned int& reportDataSize)
         : m_reportId(reportId),
+          m_hidWriteErrorLogged(false),
           m_possiblyUnsentDataCached(false),
           m_lastCachedDataSize(0) {
     // First byte must always contain the ReportID - also after swapping, therefore initialize both arrays
@@ -128,10 +129,20 @@ bool HidIoOutputReport::sendCachedData(QMutex* pHidDeviceAndPollMutex,
             reinterpret_cast<const unsigned char*>(m_lastSentData.constData()),
             m_lastSentData.size());
     if (result == -1) {
-        qCWarning(logOutput) << "Unable to send data to device :"
-                             << mixxx::convertWCStringToQString(
-                                        hid_error(pHidDevice),
-                                        kMaxHidErrorMessageSize);
+        if (!m_hidWriteErrorLogged) {
+            qCWarning(logOutput)
+                    << "Unable to send data to device :"
+                    << mixxx::convertWCStringToQString(
+                               hid_error(pHidDevice), kMaxHidErrorMessageSize)
+                    << "Note that, this message is only logged once and may "
+                       "not appear again until all hid_writee errors have "
+                       "disappeared.";
+
+            // Stop logging error messages if every hid_write() fails to avoid large log files
+            m_hidWriteErrorLogged = true;
+        }
+    } else {
+        m_hidWriteErrorLogged = false;
     }
 
     hidDeviceLock.unlock();

src/controllers/hid/hidiooutputreport.h

@@ -25,6 +25,7 @@ class HidIoOutputReport {
   private:
     const quint8 m_reportId;
     QByteArray m_lastSentData;
+    bool m_hidWriteErrorLogged;
 
     /// Mutex must be locked when reading/writing m_cachedData
     /// or m_possiblyUnsentDataCached

src/controllers/hid/hidiothread.cpp

@@ -40,6 +40,7 @@ HidIoThread::HidIoThread(
           m_pHidDevice(pHidDevice),
           m_lastPollSize(0),
           m_pollingBufferIndex(0),
+          m_hidReadErrorLogged(false),
           m_globalOutputReportFifo(),
           m_runLoopSemaphore(1) {
     // Initializing isn't strictly necessary but is good practice.
@@ -99,16 +100,27 @@ void HidIoThread::pollBufferedInputReports() {
         int bytesRead = hid_read(m_pHidDevice, m_pPollData[m_pollingBufferIndex], kBufferSize);
         if (bytesRead < 0) {
             // -1 is the only error value according to hidapi documentation.
-            qCWarning(m_logOutput) << "Unable to read buffered HID InputReports from"
-                                   << m_deviceInfo.formatName() << ":"
-                                   << mixxx::convertWCStringToQString(
-                                              hid_error(m_pHidDevice),
-                                              kMaxHidErrorMessageSize);
             DEBUG_ASSERT(bytesRead == -1);
+            if (!m_hidReadErrorLogged) {
+                qCWarning(m_logOutput)
+                        << "Unable to read buffered HID InputReports from"
+                        << m_deviceInfo.formatName() << ":"
+                        << mixxx::convertWCStringToQString(
+                                   hid_error(m_pHidDevice),
+                                   kMaxHidErrorMessageSize)
+                        << "Note that, this message is only logged once and "
+                           "may not appear again until all hid_read errors "
+                           "have disappeared.";
+                // Stop logging error messages if every hid_read() fails to avoid large log files
+                m_hidReadErrorLogged = true;
+            }
             break;
-        } else if (bytesRead == 0) {
-            // No InputReports left to be read
-            break;
+        } else {
+            m_hidReadErrorLogged = false; // Allow to log new errors
+            if (bytesRead == 0) {
+                // No InputReports left to be read
+                break;
+            }
         }
         processInputReport(bytesRead);
     }

src/controllers/hid/hidiothread.h

@@ -79,6 +79,7 @@ class HidIoThread : public QThread {
     unsigned char m_pPollData[kNumBuffers][kBufferSize];
     int m_lastPollSize;
     int m_pollingBufferIndex;
+    bool m_hidReadErrorLogged;
 
     /// Must be locked when a operation changes the size of the m_outputReports map,
     /// or when modify the m_outputReportIterator

src/library/tabledelegates/coverartdelegate.cpp

@@ -5,6 +5,7 @@
 #include <algorithm>
 
 #include "library/coverartcache.h"
+#include "library/dao/trackschema.h"
 #include "library/trackmodel.h"
 #include "moc_coverartdelegate.cpp"
 #include "track/track.h"
@@ -29,7 +30,8 @@ CoverArtDelegate::CoverArtDelegate(QTableView* parent)
         : TableItemDelegate(parent),
           m_pTrackModel(asTrackModel(parent)),
           m_pCache(CoverArtCache::instance()),
-          m_inhibitLazyLoading(false) {
+          m_inhibitLazyLoading(false),
+          m_column(m_pTrackModel->fieldIndex(LIBRARYTABLE_COVERART)) {
     if (m_pCache) {
         connect(m_pCache,
                 &CoverArtCache::coverFound,
@@ -59,20 +61,44 @@ void CoverArtDelegate::emitRowsChanged(
     emit rowsChanged(std::move(rows));
 }
 
+void CoverArtDelegate::requestUncachedCover(
+        const CoverInfo& coverInfo,
+        int width,
+        int row) const {
+    if (coverInfo.imageDigest().isEmpty()) {
+        // This happens if we have the legacy hash
+        // The CoverArtCache will take care of the update
+        const auto pTrack = m_pTrackModel->getTrackByRef(
+                TrackRef::fromFilePath(coverInfo.trackLocation));
+        CoverArtCache::requestUncachedCover(this, pTrack, width);
+    } else {
+        // This is the fast path with an internal temporary track
+        CoverArtCache::requestUncachedCover(this, coverInfo, width);
+    }
+    m_pendingCacheRows.insert(coverInfo.cacheKey(), row);
+}
+
 void CoverArtDelegate::slotInhibitLazyLoading(
         bool inhibitLazyLoading) {
     m_inhibitLazyLoading = inhibitLazyLoading;
     if (m_inhibitLazyLoading || m_cacheMissRows.isEmpty()) {
         return;
     }
-    // If we can request non-cache covers now, request updates
-    // for all rows that were cache misses since the last time.
-    // Reset the member variable before mutating the aggregated
-    // rows list (-> implicit sharing) and emitting a signal that
-    // in turn may trigger new signals for CoverArtDelegate!
-    QList<int> staleRows = std::move(m_cacheMissRows);
-    DEBUG_ASSERT(m_cacheMissRows.isEmpty());
-    emitRowsChanged(std::move(staleRows));
+    VERIFY_OR_DEBUG_ASSERT(m_pTrackModel) {
+        return;
+    }
+    const double scaleFactor = m_pTableView->devicePixelRatioF();
+    const int width = static_cast<int>(m_pTableView->columnWidth(m_column) * scaleFactor);
+
+    for (int row : std::as_const(m_cacheMissRows)) {
+        const QModelIndex index = m_pTableView->model()->index(row, m_column);
+        const QRect rect = m_pTableView->visualRect(index);
+        if (rect.intersects(m_pTableView->rect())) {
+            const CoverInfo coverInfo = m_pTrackModel->getCoverInfo(index);
+            requestUncachedCover(coverInfo, width, row);
+        }
+    }
+    m_cacheMissRows.clear();
 }
 
 void CoverArtDelegate::slotCoverFound(
@@ -95,58 +121,33 @@ void CoverArtDelegate::slotCoverFound(
     }
 }
 
-TrackPointer CoverArtDelegate::loadTrackByLocation(
-        const QString& trackLocation) const {
-    VERIFY_OR_DEBUG_ASSERT(m_pTrackModel) {
-        return TrackPointer();
-    }
-    return m_pTrackModel->getTrackByRef(
-            TrackRef::fromFilePath(trackLocation));
-}
-
 void CoverArtDelegate::paintItem(
         QPainter* painter,
         const QStyleOptionViewItem& option,
         const QModelIndex& index) const {
-    paintItemBackground(painter, option, index);
-
-    CoverInfo coverInfo = m_pTrackModel->getCoverInfo(index);
     VERIFY_OR_DEBUG_ASSERT(m_pTrackModel) {
         return;
     }
+    CoverInfo coverInfo = m_pTrackModel->getCoverInfo(index);
     bool drewPixmap = false;
     if (coverInfo.hasImage()) {
         VERIFY_OR_DEBUG_ASSERT(m_pCache) {
             return;
         }
-        const double scaleFactor = qobject_cast<QWidget*>(parent())->devicePixelRatioF();
-        QPixmap pixmap = CoverArtCache::getCachedCover(
-                coverInfo,
-                static_cast<int>(option.rect.width() * scaleFactor));
+        const double scaleFactor = m_pTableView->devicePixelRatioF();
+        const int width = static_cast<int>(option.rect.width() * scaleFactor);
+        QPixmap pixmap = CoverArtCache::getCachedCover(coverInfo, width);
         if (pixmap.isNull()) {
             // Cache miss
             if (m_inhibitLazyLoading) {
                 // We are requesting cache-only covers and got a cache
-                // miss. Record this row so that when we switch to requesting
-                // non-cache we can request an update.
-                m_cacheMissRows.append(index.row());
-            } else {
-                if (coverInfo.imageDigest().isEmpty()) {
-                    // This happens if we have the legacy hash
-                    // The CoverArtCache will take care of the update
-                    const auto pTrack = loadTrackByLocation(coverInfo.trackLocation);
-                    CoverArtCache::requestUncachedCover(
-                            this,
-                            pTrack,
-                            static_cast<int>(option.rect.width() * scaleFactor));
-                } else {
-                    // This is the fast path with an internal temporary track
-                    CoverArtCache::requestUncachedCover(
-                            this,
-                            coverInfo,
-                            static_cast<int>(option.rect.width() * scaleFactor));
+                // miss. Record row in a list for later lookup.
+                if (!m_cacheMissRows.contains(index.row())) {
+                    cleanCacheMissRows();
+                    m_cacheMissRows.insert(index.row());
                 }
-                m_pendingCacheRows.insert(coverInfo.cacheKey(), index.row());
+            } else {
+                requestUncachedCover(coverInfo, width, index.row());
             }
         } else {
             // Cache hit
@@ -162,12 +163,31 @@ void CoverArtDelegate::paintItem(
     // Since the background color is calculated from the cover art image
     // it is optional and may not always be available. The background
     // color may even be set manually without having a cover image.
-    if (!drewPixmap && coverInfo.color) {
-        painter->fillRect(option.rect, mixxx::RgbColor::toQColor(coverInfo.color));
+    if (!drewPixmap) {
+        if (coverInfo.color) {
+            painter->fillRect(option.rect, mixxx::RgbColor::toQColor(coverInfo.color));
+        } else {
+            paintItemBackground(painter, option, index);
+        }
     }
 
     // Draw a border if the cover art cell has focus
     if (option.state & QStyle::State_HasFocus) {
         drawBorder(painter, m_focusBorderColor, option.rect);
     }
 }
+
+void CoverArtDelegate::cleanCacheMissRows() const {
+    auto it = m_cacheMissRows.begin();
+    while (it != m_cacheMissRows.end()) {
+        const QModelIndex index = m_pTableView->model()->index(*it, m_column);
+        const QRect rect = m_pTableView->visualRect(index);
+        if (!rect.intersects(m_pTableView->rect())) {
+            // Cover image row is no longer shown. We keep the set
+            // small which likely reuses the allocatd memory later
+            it = m_cacheMissRows.erase(it);
+        } else {
+            ++it;
+        }
+    }
+}