Skip to content

feat(library): Add rating sync with file tags (FMPS_RATING standard) - #15952

Open
SimonDedman wants to merge 7 commits into
mixxxdj:mainfrom
SimonDedman:feature/rating-file-tags-9477
Open

feat(library): Add rating sync with file tags (FMPS_RATING standard)#15952
SimonDedman wants to merge 7 commits into
mixxxdj:mainfrom
SimonDedman:feature/rating-file-tags-9477

Conversation

@SimonDedman

Copy link
Copy Markdown

Adds support for saving and loading track ratings to/from file metadata tags, addressing a long-requested feature (#9477).

Overview

This implementation uses the FMPS_RATING standard (0.0-1.0 scale), which is compatible with Strawberry, Foobar2000, Quod Libet, and other music players that follow the freedesktop.org Media Player Specs. The feature is non-destructive (does not overwrite POPM/WMP ratings) and user opt-in (disabled by default).

Key design decisions:

  • Non-destructive: Does not overwrite POPM (Windows Media Player) or other rating formats
  • User opt-in: Disabled by default, configurable in Preferences > Library
  • Bidirectional: Separate options for import and export

Changes

TagLib Layer (Phase 1)

Rating import/export functions for all supported formats:

  • ID3v2 (MP3): TXXX:FMPS_Rating frame write, POPM fallback read
  • Xiph (FLAC/OGG): FMPS_RATING Vorbis comment
  • APE (WavPack/APE): FMPS_Rating item
  • MP4 (M4A): ----:org.freedesktop.FMPS:FMPS_Rating atom

Preferences UI (Phase 2)

Two new checkboxes in Library preferences under "Track Metadata Synchronization":

  • Export rating to file tags - Saves Mixxx ratings to files on metadata export or quit
  • Import rating from file tags - Loads ratings when tracks are selected/imported

Integration (Phase 3-4)

  • MetadataSourceTagLib: New importRating() and exportRating() virtual methods
  • Track::exportMetadata(): Exports rating after metadata when preference enabled
  • SoundSourceProxy::updateTrackFromSource(): Imports rating during track metadata update
  • TrackCollectionManager::exportTrackMetadataBeforeSaving(): Respects rating export preference
  • RatingSyncWorker: Background worker for syncing ratings when opening playlists/crates

Rating Format Support

Format Tag Type Read Write
MP3 TXXX:FMPS_Rating
MP3 POPM (fallback)
FLAC/OGG FMPS_RATING
M4A FMPS_Rating atom
WavPack FMPS_Rating (APE)

Conversion Scale

Mixxx FMPS Display
0 0.0 No rating
1 0.2
2 0.4 ★★
3 0.6 ★★★
4 0.8 ★★★★
5 1.0 ★★★★★

Testing

  • ✅ Tested bidirectional sync with Strawberry music player
  • ✅ Ratings export correctly to FMPS_Rating format on quit
  • ✅ Ratings import correctly when selecting tracks in library
  • ✅ Verified with kid3: exported rating shows FMPS_Rating = 1.0 for 5★

Screenshots

image

The new preferences options are located in Preferences > Library > Track Metadata Synchronization section

Test plan

  • Enable "Export rating to file tags" preference
  • Rate a track 5 stars in Mixxx
  • Close Mixxx
  • Verify file has FMPS_Rating = 1.0 (use exiftool, kid3, or similar)
  • Enable "Import rating from file tags" preference
  • Add a file with existing FMPS_Rating to library
  • Verify rating appears in Mixxx

Closes #9477

Adds support for saving and loading track ratings to/from file metadata
tags, addressing a long-requested feature (mixxxdj#9477).

## Overview

This implementation uses the FMPS_RATING standard (0.0-1.0 scale),
which is compatible with Strawberry, Foobar2000, Quod Libet, and other
music players. The feature is non-destructive (does not overwrite
POPM/WMP ratings) and user opt-in (disabled by default).

## Changes

### TagLib Layer (Phase 1)
- ID3v2: FMPS_Rating TXXX frame read/write + POPM fallback read
- Xiph: FMPS_RATING Vorbis comment for FLAC/OGG
- APE: FMPS_Rating item for WavPack/APE files
- MP4: ----:org.freedesktop.FMPS:FMPS_Rating atom for M4A

### Preferences (Phase 2)
- Two new checkboxes in Library preferences:
  - "Export rating to file tags" - saves Mixxx ratings to files
  - "Import rating from file tags" - loads ratings from files

### Integration (Phase 3-4)
- MetadataSourceTagLib: importRating() and exportRating() methods
- Track: Exports rating after metadata export when preference enabled
- SoundSourceProxy: Imports rating during track metadata update
- TrackcollectionManager: Export-on-quit now respects rating preference
- RatingSyncWorker: Background sync when opening playlists/crates

## Rating Format Support

| Format | Tag Type | Read | Write |
|--------|----------|------|-------|
| MP3 | TXXX:FMPS_Rating | ✅ | ✅ |
| MP3 | POPM (fallback) | ✅ | ❌ |
| FLAC/OGG | FMPS_RATING | ✅ | ✅ |
| M4A | FMPS_Rating atom | ✅ | ✅ |
| WavPack | FMPS_Rating (APE) | ✅ | ✅ |

## Conversion Scale

| Mixxx | FMPS | Display |
|-------|------|---------|
| 0 | 0.0 | No rating |
| 1 | 0.2 | ★ |
| 2 | 0.4 | ★★ |
| 3 | 0.6 | ★★★ |
| 4 | 0.8 | ★★★★ |
| 5 | 1.0 | ★★★★★ |

Closes mixxxdj#9477

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@JoergAtGithub

Copy link
Copy Markdown
Member

Welcome at Mixxx!
As a first-time contributor we need you to sign the Mixxx Contributor Agreement and comment here when you have done so. It gives us permission to distribute your contribution under the GPL v2 or later license and the Apple Mac App Store. It is also helpful for us to have contact information for contributors in case we may need it in the future.

@acolombier acolombier left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some high level comments before doing a more deep dived review:

  • Please make sure to use smart pointer
  • Use snake_case for new config keys
  • Please explain why we need a custom worker as opposed to use the same metadata runner
  • We will need to have some test to ensure the supported file types are indeed well exported

@SimonDedman

Copy link
Copy Markdown
Author

Thanks for the review! All four items addressed in the latest push:

  1. Smart pointer: Removed RatingSyncWorker entirely, so the raw pointer is gone. The existing SoundSourceProxy::updateTrackFromSource() pipeline already handles rating import via syncParams.importRatingFromFile, making the custom worker unnecessary.

  2. snake_case config keys: Renamed to export_rating_to_file_tags and import_rating_from_file_tags.

  3. Why custom worker?: See master sync first merge #1 — we don't need one. Removed it along with all references in baseplaylistfeature and cratefeature.

  4. Tests: Added ratingexportimport_test.cpp with:

    • Parameterized round-trip test across all 8 supported formats (MP3, FLAC, OGG, Opus, M4A, WAV, AIFF, WavPack)
    • All rating values (1–5) round-trip test
    • Clear rating test (export 0, verify no rating returned)
    • No-rating-initially test (fresh file returns nullopt)

- Remove RatingSyncWorker (custom worker unnecessary; existing
  SoundSourceProxy pipeline handles rating import)
- Rename config keys to snake_case: export_rating_to_file_tags,
  import_rating_from_file_tags
- Add ratingexportimport_test.cpp with round-trip tests for all 8
  supported formats, all rating values, clear, and no-rating cases

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
SimonDedman and others added 2 commits February 7, 2026 13:40
TagLib normalizes APE tag keys to uppercase on save/reload, causing
the WavPack rating round-trip test to fail. Changed FMPS_Rating to
FMPS_RATING in trackmetadata_ape.cpp to match this behavior (consistent
with the Xiph Comment code which already used uppercase).

Also fix clang-format issues: indentation in trackcollectionmanager.cpp,
trailing blank lines, include ordering in metadatasourcetaglib.h, and
double blank line in trackmetadata_mp4.cpp.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Break long namespace-qualified config key identifiers across lines
to satisfy clang-format's line length rules.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@SimonDedman

Copy link
Copy Markdown
Author

Some high level comments before doing a more deep dived review:

* Please make sure to use smart pointer

* Use `snake_case` for new config keys

* Please explain why we need a custom worker as opposed to use the same metadata runner

* We will need to have some test to ensure the supported file types are indeed well exported

Cheers for the notes Antoine. PR should be fixed now. Cheers!

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@SimonDedman

Copy link
Copy Markdown
Author

@acolombier Antoine any chance someone could have a peek at this?

@SimonDedman

Copy link
Copy Markdown
Author

@acolombier @JoergAtGithub it would be great if someone could have a look at this. It resolves a longstanding community request to add basic functionality which is sorely lacking in the core product. I've built this (with heavy AI assistance obviously) over a few weeks of evenings, done all the due diligence I could think of, and answered requests immediately.

It would be great if this work could be made available to others, including myself in future builds, and would allow me to recapture 5gb of space on my home drive as well.#

Thanks all.

@acolombier

Copy link
Copy Markdown
Member

Hi, we are still assessing our AI policy and aren't yet sure where we stand. As you may be aware, Flathub has recently announced a global ban on AI assisted application code, and not complying would mean having to remove our app from their store (200k installs)

Overall, the fix looks like it is aiming in the right direction, though there is a few unhandled values around numeric conversion, which could use a second pass. I appreciate the code was heavily written with AI, so a careful review of the code before we review it would be appreciated.

@daschuer
daschuer changed the base branch from 2.6 to main August 2, 2026 12:20
@daschuer

daschuer commented Aug 2, 2026

Copy link
Copy Markdown
Member

This required a full beta cycle. That's why I have changed the base branch to main.

SimonDedman and others added 2 commits August 7, 2026 08:33
Introduce a shared conversion unit used by all four TagLib backends
instead of four copy-pasted implementations:

- Validate FMPS values at the string level with a regex before
  converting. QString::toDouble() accepts "nan" and "inf", and since
  Mixxx compiles with -ffast-math, floating-point classification
  functions cannot be relied on to reject non-finite values afterwards.
- Use TrackRecord::isValidRating()/kMinRating/kMaxRating instead of
  magic numbers.
- Guard the full int domain of TagLib's POPM rating() accessor.
- Keep an existing POPM frame in sync on export (rating byte only,
  preserving the identifier and play counter) and zero it when the
  rating is cleared, so a cleared rating is not resurrected via the
  POPM import fallback.
- Read the new config key with the typed getValue() accessor.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Do not strip ID3v1/APE tags when exporting a rating into MPEG files:
  TagLib's save(tags) defaults to StripOthers, which silently deleted
  existing ID3v1 and APE tags (e.g. mp3gain). Mirror the arguments of
  MpegTagSaver::saveModifiedTags() instead.
- Keep every tag location consulted by the rating import in sync on
  export (APE item of MPEG files, ID3v2 frame of FLAC files) so that a
  stale secondary location cannot resurrect a changed or cleared
  rating.
- Export the rating only when it differs from the value in the file
  tags, including removal when the rating has been cleared. Refresh the
  synchronization time stamp afterwards so the file is not considered
  externally modified by the rating write.
- Add a rating-only export path: enabling only rating export no longer
  triggers a full metadata export of dirty tracks, and unchanged tracks
  are no longer rewritten on every save.
- Import ratings fill-only during partial imports (file unchanged since
  the last synchronization, an existing Mixxx rating wins) and
  file-wins during full imports (file changed or first import).
- Rename SyncTrackMetadataParams::syncRating to exportRatingToFile, add
  syncTrackMetadata, and align both preference tooltips with the actual
  behavior.
- Cover all of the above with tests: format round trips, malformed and
  non-finite values, POPM band boundaries and byte preservation, tag
  preservation on MPEG, clear/skip semantics at the Track level, and
  partial vs full import semantics.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@SimonDedman

SimonDedman commented Aug 7, 2026

Copy link
Copy Markdown
Author

@acolombier The careful review pass you asked for is done — two new commits. It confirmed your suspicion about numeric conversion and turned up a couple of real bugs beyond it.

Numeric conversion (your original point)

  • The four copy-pasted conversion implementations are consolidated into src/track/taglib/fmpsrating.h/.cpp, built on TrackRecord::isValidRating()/kMinRating/kMaxRating instead of magic numbers.
  • FMPS values are now validated with a regex at the string level before conversion. QString::toDouble() accepts "nan" and "inf", and because Mixxx compiles with -ffast-math, std::isfinite() and NaN range comparisons are unreliable. The previous code genuinely accepted "nan" as a rating under this build configuration.
  • POPM conversion now guards the full int domain of TagLib's rating() accessor.

Bugs found in the self-review pass

  • Exporting a rating into an MPEG file called save(ID3v2), which defaults to StripOthers in TagLib 2 and silently deleted existing ID3v1/APE tags (e.g. mp3gain's). It now mirrors the arguments of MpegTagSaver::saveModifiedTags().
  • A cleared rating was never exported (the removal branch was unreachable), so the stale file tag re-imported the old rating on the next scan. Clearing now removes the FMPS tag and zeroes an existing POPM rating byte, preserving its play counter.
  • Export now keeps every tag location the importer consults in sync (APE item on MPEG, ID3v2 frame on FLAC), so a stale secondary location can't resurrect a changed or cleared rating.
  • The rating is only written when it differs from the file's value, and the synchronisation timestamp is refreshed afterwards; unchanged saves no longer rewrite files, and rated files no longer appear externally modified.
  • Enabling only the rating-export preference no longer triggers a full metadata export of dirty tracks; a rating-only path writes just the rating tag.

One deliberate behaviour change to flag for review: rating import is now fill-only during partial imports (file unchanged since last sync; an existing Mixxx rating wins) and file-wins during full imports (file changed, or first import). Previously the file value unconditionally overwrote the Mixxx rating on every track load, which could silently revert a user's rating. Both preference tooltips have been rewritten to describe the actual behaviour.

Test coverage went from 4 to 27 rating-related tests: format round-trips, malformed and non-finite values, POPM band boundaries and byte preservation, ID3v1/APE preservation on MPEG export, clear/skip semantics at the Track level, and partial-vs-full import semantics. The full suite is green apart from failures that reproduce identically on the merge base without this branch.

On the AI policy: understood that it's still being worked out, no pressure from my side on that. This pass was the careful review you asked for before the deep dive — happy to walk through any part of it.

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.

Saving ratings in file tags

4 participants