Skip to content

[Rekordbox] Fix CI warnings and formatting - #26

Open
0cwa wants to merge 101 commits into
mixxx/mainfrom
pr/rekordbox-v1-ci-fixes
Open

0cwa wants to merge 101 commits into
mixxx/mainfrom
pr/rekordbox-v1-ci-fixes

Conversation

@0cwa

@0cwa 0cwa commented Aug 20, 2026

Copy link
Copy Markdown
Owner

Summary

This fresh PR carries the CI fixes from PR #25 without changing PR #25:

  • Apply the repository clang-format result to the Rekordbox implementation and test files.
  • Fix the two Qt range-loop detach warnings reported by Clazy.
  • Pass QList<MemoryCueLoop> to normalizeMemoryCueLoops by const reference, with the declaration, definition, and caller updated consistently.

The previous PR remains unchanged. This branch is based on its validated head and targets mixxx/main so the fix can receive a fresh review.

Validation

  • Pinned clang-format check passed.
  • git diff --check passed.
  • mixxx-lib/fast passed in mixxx-build Distrobox.
  • mixxx-test/fast passed in mixxx-build Distrobox.
  • mixxx-test --gtest_filter='RekordboxFeatureTest.*:RekordboxXmlParserTest.*': 15/15 passed.
  • mixxx/fast passed in mixxx-build Distrobox.

The local Distrobox image does not contain Clazy or clang-tidy, so those exact CI tools remain delegated to GitHub Actions. A local WARNINGS_FATAL=ON reconfigure was attempted but stopped during a long Qt QML import scan.

Scope

Pre-existing unrelated local changes (AGENTS.md and src/test/echoreverbchain_test.cpp) are not included.

acolombier and others added 30 commits January 25, 2026 21:32
Add the Bungee scaler implementation and wire it into Mixxx's keylock engine path without carrying vendored Bungee source.
Add regression coverage for Bungee keylock scaler selection and buffer-window accounting behavior.
Resolve Bungee through package, vcpkg, or module discovery and normalize every provider to the Bungee::Bungee target.
Add the pinned ExternalProject fallback and permanent Bungee/pffft patch homes so non-vcpkg builds do not depend on the temporary overlay.
Default Bungee on, seed preference defaults, and keep Dual-threaded Stereo UI state consistent with the selected keylock engine.
Document the cleanup point in the clean stack: this branch was built without tracking lib/bungee source, and dependency paths now resolve through package, vcpkg, module, or source-fetch providers.
Add Flatpak Eigen3, pffft, and Bungee modules before Mixxx and point Bungee patches at the permanent cmake/patches/bungee home.
Add the Bungee ASan workflow, dependency path filters, sanitizer source-fetch environment, and build workflow coverage.
Document Bungee build modes, dependency fallback prerequisites, Flatpak/CI notes, and the runtime invariants maintainers need for validation.
@coderabbitai

coderabbitai Bot commented Aug 20, 2026

Copy link
Copy Markdown

Review Change Stack

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 2be85cd0-2a1a-4449-81ca-2e66eecba9e3

📝 Walkthrough

Walkthrough

This change adds optional Bungee and SignalSmith keylock engines, per-deck engine settings, Rekordbox XML import, memory-cue and downbeat controls, waveform rendering, local build tooling, packaging, and sanitizer coverage.

Changes

Build, packaging, and validation

Layer / File(s) Summary
Optional audio-engine dependency integration
CMakeLists.txt, cmake/modules/*, cmake/patches/*
CMake discovers or fetches Bungee, Eigen3, pffft, and SignalSmith. Compatibility patches and imported targets are added.
Packaging and local development tooling
packaging/flatpak/*, localuser.sh, docs/bungee-integration.md, AGENTS.md
Flatpak modules, a container-based local build wrapper, Bungee documentation, and updated submission guidance are added.
CI and sanitizer validation
.github/workflows/*
The Ubuntu build enables Bungee. A dedicated ASan workflow builds mixxx-test and runs Bungee and SettingsManager regression tests.

Audio engines and keylock routing

Layer / File(s) Summary
Scaler interfaces and implementations
src/engine/bufferscalers/*
Bungee and SignalSmith scalers implement tempo, pitch, buffering, channel conversion, reset, and visual-position behavior.
EngineBuffer and per-deck keylock routing
src/engine/enginebuffer.*, src/engine/enginemixer.*, src/preferences/dialog/dlgprefsound*, src/preferences/settingsmanager.*, src/qml/qmlsoundmanagerproxy.*, res/qml/Settings/SoundHardware.qml
Keylock selection now supports four deck-specific settings. Engine changes are queued, applied at device close, and tracked atomically.
Scaler and alignment regression coverage
src/test/*
Tests cover scaler output, buffer invariants, engine switching, visual prediction, read-ahead behavior, and settings migration.

Rekordbox XML import and cue policy

Layer / File(s) Summary
Parser and Main Cue policy contracts
src/library/memorycuepromotion*, src/library/rekordbox/rekordboxxmlparser.*
Public XML data structures, diagnostics, annotation serialization, and configurable Main Cue promotion are added.
Collection import and ANLZ application
src/library/rekordbox/rekordboxfeature.*
XML collections are imported into temporary library views. Track annotations, playlists, cues, loops, beatgrids, and ANLZ data are applied with duplicate tracking.
Rekordbox import validation
src/test/rekordbox*
Parser and feature tests cover malformed input, path normalization, cue normalization, source identity, metadata application, and BPM-lock preservation.
Rekordbox preference wiring
src/preferences/dialog/dlgpreflibrary*
A preference controls promotion of the first non-loop memory cue to Main Cue.

Memory cues, downbeats, and waveform controls

Layer / File(s) Summary
Memory-cue and downbeat data controls
src/engine/controls/*, src/track/*, src/proto/beats.proto, src/controllers/controlpickermenu.cpp
Memory-cue management, 30-second seeking, downbeat offsets, beat-grid persistence, stem controls, and cue navigation are added.
Waveform cue mark model
src/waveform/renderers/waveformmark*, src/waveform/renderers/waveformrendermarkbase.cpp, src/widget/woverview.cpp
Waveform marks now support fixed-position memory cues and safe local positions.
Shader waveform rendering
src/waveform/renderers/allshader/*, src/waveform/widgets/allshader/*
Downbeat and memory-cue shader renderers, configurable opacity, colors, geometry, and digit alignment are added.
Skin and legacy memory-cue widgets
res/skins/*, src/skin/legacy/*, src/widget/*
LateNight controls, tooltips, legacy skin parsing, cue-menu visibility, and memory-cue popup behavior are added.

Estimated code review effort: 5 (Critical) | ~120 minutes

Merge Risk: 🔴 Critical · up to 8dcd9

The PR introduces unresolved runtime, audio-thread, persistence, and build failures that can cause crashes, audio stalls, lost beatmap data, incorrect cue editing, or failed platform builds; merge should be blocked until the high-impact correctness and integration issues are fixed.

Sequence Diagram(s)

sequenceDiagram
  participant EngineBuffer
  participant ReadAheadManager
  participant EngineBufferScaleBungee
  participant WaveformWidget
  EngineBuffer->>ReadAheadManager: read ahead audio frames
  EngineBuffer->>EngineBufferScaleBungee: scaleBuffer output request
  EngineBufferScaleBungee->>ReadAheadManager: consume input window
  EngineBufferScaleBungee->>EngineBuffer: return scaled audio and visual offset
  EngineBuffer->>WaveformWidget: update visual play position
Loading
sequenceDiagram
  participant User
  participant RekordboxPlaylistModel
  participant RekordboxXmlParser
  participant LibraryDatabase
  User->>RekordboxPlaylistModel: select XML import
  RekordboxPlaylistModel->>RekordboxXmlParser: parse XML library
  RekordboxXmlParser-->>RekordboxPlaylistModel: tracks, playlists, and annotations
  RekordboxPlaylistModel->>LibraryDatabase: replace temporary view data
  LibraryDatabase-->>User: display imported playlists and tracks
Loading

Suggested reviewers: daschuer

Poem

A rabbit tuned the beat-grid bright,
Added cues that hop in sight.
Bungee stretches, tests run fast,
XML memories find their past.
Four decks choose their engine true—
“Hop, hop!” says the rabbit, “done for you!”

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 12.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately identifies the Rekordbox CI warning fixes and formatting changes described in the pull request.
Description check ✅ Passed The description clearly explains the Rekordbox formatting, Clazy warning, const-reference, and validation changes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch pr/rekordbox-v1-ci-fixes

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@0cwa

0cwa commented Aug 20, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 20, 2026

Copy link
Copy Markdown
❌ Action failed

Review failed.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@0cwa
0cwa marked this pull request as ready for review August 20, 2026 16:15
@0cwa

0cwa commented Aug 20, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 20, 2026

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants