Skip to content

macOS/TagFetcher: improve Opus matching reliability (AcoustID POST + MusicBrainz fallback + safer fingerprinting)#2002

Open
OpsKonekt wants to merge 5 commits intostrawberrymusicplayer:masterfrom
OpsKonekt:fix/macos-opus-tagfetcher-fallback
Open

macOS/TagFetcher: improve Opus matching reliability (AcoustID POST + MusicBrainz fallback + safer fingerprinting)#2002
OpsKonekt wants to merge 5 commits intostrawberrymusicplayer:masterfrom
OpsKonekt:fix/macos-opus-tagfetcher-fallback

Conversation

@OpsKonekt
Copy link
Copy Markdown

Summary

This PR improves Tag Fetcher reliability for .opus files (notably files downloaded via yt-dlp) on macOS, while keeping the app self-contained (no external CLI dependency at runtime).

Problem

Some files failed in Tag Fetcher with errors like:

  • AcoustID HTTP 400 / invalid fingerprint
  • "No decoded audio samples were produced by GStreamer"
  • No useful fallback when AcoustID returned no recording IDs, especially when embedded tags were missing

What changed

1) AcoustID request robustness

  • Switched lookup request from query-style GET to form POST (application/x-www-form-urlencoded)
  • Improved API/network error parsing and diagnostics
  • Kept request semantics identical (format, client, duration, meta, fingerprint)

2) MusicBrainz fallback when AcoustID fails

  • Added metadata search fallback path in TagFetcher:
    • If AcoustID returns no MBIDs, perform a MusicBrainz recording search
  • Added MusicBrainzClient::StartSearchRequest(...)
  • Added query normalization/tokenization and result scoring (title/artist/album/duration heuristics)
  • Preserved existing MBID path behavior

3) Better handling of missing tags

  • If title/artist tags are missing, derive search metadata from filename
  • Supports common filename patterns like track-number + artist + title

4) Better user diagnostics

  • Track selection error page now shows technical details from backend failures
  • Keeps details readable as plain text

5) Fingerprinting hardening + memory safety

  • Hardened GStreamer decode path in Chromaprinter
  • Added explicit in-memory PCM cap to avoid unbounded growth
  • Added safer pad checks and clearer failure reasons

6) macOS bundle/runtime fixes

  • Improved bundled GStreamer plugin discovery on startup
  • Added plugin/runtime packaging adjustments needed for matroska/webm/opus flow
  • Added libgstriff bundling when required by libgstmatroska
  • Registry filename now includes executable timestamp to avoid stale cache reuse across rebuilds

Validation

  • Build: cmake --build build-m1-full -j8 (passes)
  • Manual tests on macOS Sonoma 14.6.1 (Apple Silicon):
    • Tag Fetcher with .opus files with and without embedded tags
    • AcoustID success path
    • MusicBrainz fallback path
    • Error diagnostics visibility in UI

Commit breakdown

  1. musicbrainz: add metadata-search fallback when AcoustID fails
  2. ui: show technical diagnostics in tag fetcher error page
  3. chromaprinter: harden decoding and cap in-memory PCM usage
  4. macos: bundle and discover GStreamer runtime dependencies reliably

OpsKonekt and others added 5 commits February 13, 2026 21:12
Bumps [vmactions/freebsd-vm](https://github.com/vmactions/freebsd-vm) from 1.4.1 to 1.4.2.
- [Release notes](https://github.com/vmactions/freebsd-vm/releases)
- [Commits](vmactions/freebsd-vm@v1.4.1...v1.4.2)

---
updated-dependencies:
- dependency-name: vmactions/freebsd-vm
  dependency-version: 1.4.2
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Copy link
Copy Markdown
Member

@jonaski jonaski left a comment

Choose a reason for hiding this comment

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

Thanks, since there is a lot of changes here addressing different things, could you open a PR for each? This way it's easier to review.
I suggest to split into 5 different PR's similar to your commits:

  1. Improved error handling
  2. Chromaprint pipeline changes
  3. Additional MusicBrainz fallback to tag search
  4. The gstreamer environmental variable changes (but I'm not sure we need these)
  5. macgstcopy

You should be able to create 4 separate branches and cherry-pick the relevant commit to each branch.
Also, please rebase the commit messages to include capitalized "ClassName:" as prefix if the changes only affect one class, otherwise the commit title should be without a prefix.
And was a AI tool used for any of these changes?

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.

This commit shouldn't be included here.

Comment thread dist/macos/macgstcopy.sh
libgstisomp4
libgstlame
# Needed for WebM/Matroska containers that often wrap Opus streams.
libgstmatroska
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.

Matroska isn't currently built, it needs to be enabled in https://github.com/strawberrymusicplayer/strawberry-macos-dependencies/
how did you test this?

Comment thread dist/macos/macgstcopy.sh
fi
done

# libgstmatroska depends on libgstriff, which is not always pulled automatically.
Copy link
Copy Markdown
Member

@jonaski jonaski Feb 18, 2026

Choose a reason for hiding this comment

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

Then we need to investigate why, this seems like a workaround, isn't libgstriff-1.0.0.dylib linked?

Song original_song_;
bool pending_;
QString progress_string_;
// Carries technical diagnostics returned by TagFetcher for the error page.
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.

Don't think we need too many comments like this, since no other variables are commented.

return GST_FLOW_OK;

}

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.

Avoid committing formatting changes in the same PR

Comment thread src/engine/gststartup.cpp
QString gst_registry_filename = StandardPaths::WritableLocation(StandardPaths::StandardLocation::AppLocalDataLocation) + QStringLiteral("/gst-registry-%1-bin").arg(QCoreApplication::applicationVersion());
const QFileInfo executable_info(QCoreApplication::applicationFilePath());
const qint64 executable_stamp = executable_info.exists() ? executable_info.lastModified().toSecsSinceEpoch() : 0;
// Include executable timestamp to avoid reusing stale GStreamer registries across rebuilds.
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.

We only distribute a version once, so this shouldn't be necessary as far as I can tell. During development you can easily delete these manually. Is there a specific reason this is needed?

@jonaski
Copy link
Copy Markdown
Member

jonaski commented Feb 18, 2026

Validation

  • Build: cmake --build build-m1-full -j8 (passes)

  • Manual tests on macOS Sonoma 14.6.1 (Apple Silicon):

    • Tag Fetcher with .opus files with and without embedded tags
    • AcoustID success path
    • MusicBrainz fallback path
    • Error diagnostics visibility in UI

I'm confused by this, does this mean you tested everything?
And, is it tested on macOS only, or Linux and Windows too?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants