Updates for improving Jellyfin track matching and downloading#199
Updates for improving Jellyfin track matching and downloading#199gtkashi wants to merge 26 commits into
Conversation
Adding retry logic for timeouts
Refactor title cleaning functions to improve clarity and maintainability. Introduced StripFeat for removing feat annotations and updated NormalizeTitle and NormalizeArtist to use it.
Updated functions to handle featuring clauses more accurately and normalize artist names with semicolons.
Refactor artist matching logic to improve accuracy by isolating the primary artist and updating fuzzy matching.
Refactor SearchSongs to improve search parameters by stripping punctuation, isolating primary artist, and using explicit filters.
Updated search logic to improve reliability and added limit to prevent truncation of results.
Added regex to clean search titles and improved normalization of item titles.
Refactor comments for clarity and structure in SearchSongs method.
Updated comments for clarity and added diagnostic logging for search results and item comparisons.
Updated search query to use explicit Name lookup to avoid indexing bugs in Jellyfin. Removed diagnostic logging for search results and item comparisons.
Updated search functionality to clean typography drift and revert to indexed SearchTerm query for better results.
Removed unused regexp import from jellyfin.go
Refactor fallback search logic to use the first complete word of the song title and improve punctuation handling.
Refined fallback search logic to isolate the first alphanumeric block from the song title for better search accuracy.
Cleaning up for pull request
Cleanup for pull request
LumePart
left a comment
LumePart
left a comment
There was a problem hiding this comment.
I think there are some good ideas here, especially around improving track matching.
I've left some comments on areas where the PR needs to be improved. Some of the changes make sense in isolation, but some functions need to be refactored to better fit the existing codebase.
Feel free to address those changes, or alternatively I can take some of these ideas and implement them in a way that fits better with the existing architecture.
| if track.File != "" && artistMatch && pathMatch { | ||
| targetAlnumTitle := util.NormalizeTitle(track.CleanTitle) | ||
| rawIncomingArtist := strings.ToLower(track.MainArtist) | ||
| normalizedMainArtist := util.NormalizeArtist(util.StripFeat(track.MainArtist)) |
There was a problem hiding this comment.
mainArtist doesn't need NormalizeArtist() or StripFeat() here. It is always a single primary artist that released the track.
| for _, individualArtist := range item.Artists { | ||
| normalizedIndiv := util.NormalizeArtist(individualArtist) | ||
| if normalizedIndiv == normalizedMainArtist || | ||
| strings.Contains(normalizedMainArtist, normalizedIndiv) || | ||
| util.ContainsFold(rawIncomingArtist, individualArtist) { | ||
| artistMatch = true | ||
| break | ||
| } | ||
| } |
There was a problem hiding this comment.
Could probably be simplified with slices.ContainsFunc()
| func retry(attempts int, baseDelay time.Duration, label string, fn func() error) error { | ||
| var err error | ||
| for i := 1; i <= attempts; i++ { | ||
| if err = fn(); err == nil { | ||
| return nil | ||
| } | ||
| if i < attempts { | ||
| slog.Warn("retrying after failure", | ||
| "step", label, "attempt", i, "max", attempts, "context", err.Error()) | ||
| time.Sleep(time.Duration(i) * baseDelay) | ||
| } | ||
| } | ||
| return err | ||
| } |
There was a problem hiding this comment.
I don't think retrying should be handled globally for all downloaders. For example, slskd already has its own retry logic for search and download attempts, and different providers may need different retry functions.
For YouTube specifically, multiple retries could have some downsides:
- If using the official API, it could consume API tokens quickly.
- More requests to youtube will result in more often rate-limiting
If the issue is mainly timeout-related, maybe increasing DOWNLOAD_LIMITER by a second or two would be a better first step (default is currently 1). It could be that multiple frequent operations are causing the timeouts.
| // 2. Robust Root-Word Fallback: If primary search returns 0 results, | ||
| // isolate the very first continuous block of alphanumeric characters (stops at spaces, apostrophes, etc.) | ||
| if len(results.Items) == 0 && len(cleanSearchTitle) > 0 { | ||
| endIdx := 0 | ||
| for i, r := range cleanSearchTitle { | ||
| if (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || (r >= '0' && r <= '9') { | ||
| endIdx = i + 1 | ||
| } else if endIdx > 0 { | ||
| break | ||
| } | ||
| } | ||
|
|
||
| if musicBrainzMatch || (titleMatch && artistMatch) { | ||
| track.ID = item.ID | ||
| track.Present = true | ||
| break | ||
| if endIdx > 0 { | ||
| firstWord := cleanSearchTitle[:endIdx] | ||
| broadParam := fmt.Sprintf("/Items?IncludeMediaTypes=Audio&SearchTerm=%s&Recursive=true&Limit=300&Fields=Path,ProviderIDs", url.QueryEscape(firstWord)) | ||
| broadBody, err := c.HttpClient.MakeRequest("GET", c.Cfg.URL+broadParam, nil, c.Cfg.Creds.Headers) | ||
| if err == nil { | ||
| _ = util.ParseResp(broadBody, &results) | ||
| } | ||
| } |
There was a problem hiding this comment.
I like the idea of having a fuzzy fallback, but I'd rather keep it generic instead of implementing it only for jellyfin. It feels like something that could be shared across clients via a helper, with each client only performing the actual search.
2 participants
After hitting my head against a wall trying to get track matching to work nicely in Jellyfin, ran this through Gemini several times to get the hit rate up to 100%.
Improves Jellyfin client track searching mechanism, resolving issues where Explo repeatedly logs [jellyfin] failed to find X for songs that are already present in the media library.
Changes (mainly jellyfin.go):
Alphanumeric Matching:
Replaced string matches with alphanumeric-only normalization comparisons on both sides (currentAlnumItemTitle == targetAlnumTitle) to bypass typography differences (e.g., curly vs. straight apostrophes like Where’d and Where'd).
Array/Substring Multi-Artist Tracking:
Upgraded the artist matching verification block to evaluate elements against Jellyfin’s discrete split-string JSON array slices (item.Artists). This handles collaborations cleanly (e.g., Artist A & Artist B vs Artist A feat. Artist B).
Result Pool Expansion:
Increased the API query fetch scope to Limit=300 to prevent Jellyfin from truncating matching tracks inside generic search pools.
Fuzzy Root-Word Fallback Strategy:
Implemented a non-alphanumeric split fail-safe. If an exact database query fails due to complex brackets or parentheses syntax breaking the index parser, the code automatically falls back to searching Jellyfin by the track's first continuous block of alphanumeric characters (e.g., matching Rollin' (Air Raid Vehicle) seamlessly via a root search for Rollin).
Changes to downloader.go:
Adding retry attempts due to YT time outs. 49/50 success rater, which was good enough for me to move on to dealing with all of the above items.
Change to config.go:
Just adding attempts value.
Change to sanitize.go:
Changes to string normalization. This is where the first changes were attempted, but eventually the lion's share ended up in jellyfin.go.