Skip to content

fix: split multi-artist tags from yt-dlp downloads into separate artists - #444

Open
firatege wants to merge 2 commits into
Kopuz-org:masterfrom
firatege:fix/multi-artist-314
Open

fix: split multi-artist tags from yt-dlp downloads into separate artists#444
firatege wants to merge 2 commits into
Kopuz-org:masterfrom
firatege:fix/multi-artist-314

Conversation

@firatege

@firatege firatege commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #314. Tracks downloaded from YouTube Music with multiple credited artists ended up as a single phantom artist in the library (e.g. Kero Kero Bonito, Douglas Lobban, Sarah Perry as one entry).

Root cause

Two layers:

  1. yt-dlp embeds multiple artists as one comma-joined artist tag. The scanner only splits artists on ;, so the joined string stayed in one piece.
  2. Even with a splittable tag, the singular artist field kept the whole joined string, and the album-artist fallback (album.artist) inherits it. The artists page builds its list from both track.artists and album.artist, so the phantom persisted either way (caught by the new integration tests).

Changes

  • pages/ytdlp.rs: when --embed-metadata is on, record yt-dlp's structured artists list per finished file via --print-to-file "after_move:%(artists)j\t%(filepath)s", then rewrite the tag joined with ; (which the scanner already splits). Working from the structured list instead of guessing on commas means a name like Tyler, The Creator (list of one) is left untouched, and extractors without an artists list fall back to current behavior. Best-effort: a failed rewrite only logs, the download never fails because of it.
  • reader/metadata.rs: the singular artist field is now the primary (first) credited artist — same as the YT Music streaming path (search.rs) already does. The full list stays in track.artists. Also adds set_artist_tag() used by the rewrite above.

Tests

  • crates/reader/tests/artist_parsing.rs + tiny generated silent opus fixtures (~550 bytes each): single artist unchanged, ; tag splits with primary artist, comma tag is not guessed, comma-in-name stays intact, explicit ALBUMARTIST wins.
  • pages::ytdlp unit tests: manifest line parsing (incl. null/NA extractors) and the tag rewrite incl. the single-artist skip.

Verified end to end

Downloaded the album from the issue with the patched build:

before  artist tag: "Kero Kero Bonito, Douglas Lobban, Sarah Perry"
after   artist tag: "Kero Kero Bonito;Douglas Lobban;Sarah Perry"

library after rescan:
  track.artist  = "Kero Kero Bonito"
  track.artists = ["Kero Kero Bonito", "Douglas Lobban", "Sarah Perry"]
  album.artist  = "Kero Kero Bonito"

Artists page now lists the three artists separately, no phantom entry.

Closes #314

Summary by CodeRabbit

Release Notes

  • New Features

    • Enhanced multi-artist metadata support for downloads with automatic structured artist tag rewriting when metadata embedding is enabled (updates the primary artist tag based on the parsed multi-artist list).
    • Improved artist field handling to derive the primary artist from the first structured artist entry and to avoid stale split values.
  • Tests

    • Added regression tests covering multi-artist parsing/formatting, including semicolon splitting, comma preservation within real names, and album artist precedence.

yt-dlp embeds multiple artists as one comma-joined artist tag
(e.g. "Kero Kero Bonito, Douglas Lobban, Sarah Perry"). The library
scanner only splits artists on ';', and the singular artist field kept
the whole joined string, which leaked into the album-artist fallback
and showed up as a single phantom artist in the library.

Three changes:

* ytdlp downloads: record yt-dlp's structured artists list per finished
  file (--print-to-file manifest) and rewrite the tag joined with ';'.
  Working from the structured list avoids comma guessing, so a name
  like "Tyler, The Creator" stays intact. Best-effort: a failed rewrite
  only logs and the file keeps yt-dlp's own tag.
* scanner: the singular artist field is now the primary (first) credited
  artist, matching what the YT Music streaming path already does. The
  full list stays in track.artists.
* Jellyfin sync: the artist fallback uses the first artist instead of
  comma-joining the list (same phantom pattern, different path).

Covered by reader integration tests with generated opus fixtures and
ytdlp manifest unit tests; verified end to end with the album from the
issue.

Closes Kopuz-org#314
@coderabbitai

coderabbitai Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 313d1147-6b93-43e5-9a24-4f150fe00a94

📥 Commits

Reviewing files that changed from the base of the PR and between 86ee27a and 80b8532.

📒 Files selected for processing (1)
  • crates/pages/src/ytdlp.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/pages/src/ytdlp.rs

📝 Walkthrough

Walkthrough

Adds structured multi-artist support for yt-dlp downloads. The reader crate gains a set_artist_tag function that writes a semicolon-joined artist string and removes the stale TrackArtists key. The yt-dlp driver uses --print-to-file after_move to capture per-file artist JSON, then rewrites tags on success. extract_metadata is fixed to derive the primary artist from the parsed artists list.

Changes

Multi-artist tag rewrite

Layer / File(s) Summary
set_artist_tag helper and extract_metadata fix
crates/reader/src/metadata.rs, crates/reader/src/lib.rs, crates/reader/tests/artist_parsing.rs
extract_metadata now sets track.artist from the first element of the parsed artists vector. New public set_artist_tag joins an artist slice with ;, removes the stale TrackArtists key, and saves the tag. Re-exported from the crate root. Regression tests cover single-artist, semicolon-split, comma-preservation, and ALBUMARTIST precedence cases.
yt-dlp manifest helpers and build_command extension
crates/pages/src/ytdlp.rs
build_command gains an Option<&Path> artists_manifest parameter; when set, --print-to-file after_move is appended to the yt-dlp invocation. parse_artist_manifest_line and apply_artist_manifest parse the manifest and call set_artist_tag per output file. Unit tests validate parsing and the multi-artist vs. comma-name rewrite distinction.
Per-job manifest lifecycle in blocking download task
crates/pages/src/ytdlp.rs
The blocking download task derives a job-specific temp manifest path when embed_metadata is enabled, passes it to build_command, calls apply_artist_manifest on success, and removes the manifest file afterward.

Sequence Diagram(s)

sequenceDiagram
  participant DownloadTask
  participant YtDlp
  participant ManifestFile
  participant TagRewriter
  participant AudioFile
  DownloadTask->>YtDlp: start download with manifest output path
  YtDlp->>ManifestFile: write artists JSON and output path lines
  YtDlp-->>DownloadTask: report successful completion
  DownloadTask->>TagRewriter: apply artist manifest
  TagRewriter->>ManifestFile: read manifest lines
  TagRewriter->>AudioFile: rewrite artist tag from structured artists
  DownloadTask->>ManifestFile: remove temporary manifest file
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

  • temidaradev

Poem

🐇 A rabbit once sorted its warren with care,
"Artist A, Artist B" tangled like hair!
Now semicolons split what commas confused,
Manifest parsed, no more data abused.
Each tag writes clean, the tmp file swept away—
Structured artists hop neatly in display! 🎵

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change: splitting multi-artist tags from yt-dlp downloads into separate artists, which directly addresses issue #314.
Linked Issues check ✅ Passed The PR fully implements both solutions proposed in issue #314: using primary artist for singular field and supporting multi-artist tags with semicolon separators for proper parsing.
Out of Scope Changes check ✅ Passed All changes directly support the PR objectives: yt-dlp manifest-based rewriting, metadata reader updates, and test coverage for multi-artist parsing are all scope-aligned.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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 and usage tips.

@UMCEKO UMCEKO left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

code quality lgtm

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.

[Bug]: Artist names are incorrectly merged into a single string when multiple artists are present

2 participants