Multi-disc: file the track in the disc folder that matches its tag (Sokhi)
Confirmed from Sokhi's FLAC tags + screenshot: disc-2/3 tracks land in the 'Disc 1' folder, collapsing every disc's track 3/4/5/6 into one folder. Root cause: the import pipeline syncs the resolved TRACK number into album_info (so the folder matches the tag — pipeline.py '[FIX] Updated album_info track_number') but never did the same for DISC. So the 'Disc N' folder (built from album_info.disc_number, often 1) used a different disc than the embedded tag (resolved per-track in source.py — e.g. 2/3 from a MusicBrainz multi-medium release). Fix: one SHARED resolver, resolve_disc_for_track(original_search, album_info), used by BOTH source.py (the tag) and the pipeline (which now writes it back into album_info before building the path). Same function + same inputs (the pipeline pulls the identical get_import_original_search(context)), so folder and tag can never disagree. Returns the first valid positive disc (per-track, then album), else 1 — a falsy/unknown per-track disc falls through to the album instead of flooring early. Tests: resolver preference/fallback/floor + an explicit folder==tag lockstep check incl. Sokhi's per-track-2/album-1 case. 2122 import/pipeline/metadata tests green.
This commit is contained in:
parent
c11a742e58
commit
203142c4a9
4 changed files with 83 additions and 8 deletions
|
|
@ -655,6 +655,16 @@ def post_process_matched_download(context_key, context, file_path, runtime, meta
|
|||
album_info['clean_track_name'] = clean_track_name
|
||||
logger.info(f"[FIX] Updated album_info track_number to {track_number} for consistent metadata")
|
||||
|
||||
# Sync the disc number the SAME way (and via the SAME resolver) the embedded
|
||||
# tag will use — otherwise the "Disc N" folder is built from album_info's
|
||||
# original disc while the tag takes the per-track disc, so a disc-2/3 track
|
||||
# lands in the Disc 1 folder and every disc's tracks collapse together (Sokhi).
|
||||
from core.imports.track_number import resolve_disc_for_track
|
||||
_resolved_disc = resolve_disc_for_track(get_import_original_search(context), album_info)
|
||||
if album_info.get('disc_number') != _resolved_disc:
|
||||
logger.info(f"[FIX] Updated album_info disc_number to {_resolved_disc} for consistent metadata")
|
||||
album_info['disc_number'] = _resolved_disc
|
||||
|
||||
final_path, _ = build_final_path_for_track(context, artist_context, album_info, file_ext)
|
||||
logger.info(f"Resolved path: '{final_path}'")
|
||||
context['_final_processed_path'] = final_path
|
||||
|
|
|
|||
|
|
@ -180,3 +180,32 @@ def normalize_disc_number(value) -> int:
|
|||
except (TypeError, ValueError):
|
||||
return 1
|
||||
return n if n >= 1 else 1
|
||||
|
||||
|
||||
def resolve_disc_for_track(original_search, album_info) -> int:
|
||||
"""The disc number for a track — resolved IDENTICALLY for the 'Disc N' folder
|
||||
(import pipeline) and the embedded tag (metadata.source), so the two can never
|
||||
disagree.
|
||||
|
||||
Sokhi: the pipeline synced the resolved track_number into album_info (so the
|
||||
folder matched the tag) but never did the same for disc — the folder used
|
||||
album_info's original disc (often 1) while the tag took the per-track disc
|
||||
(e.g. 2/3 from a MusicBrainz multi-medium release). Result: a disc-2/3 track
|
||||
landed in the Disc 1 folder, collapsing every disc's tracks into one folder.
|
||||
|
||||
Returns the first VALID positive disc — the per-track search's, else the album
|
||||
context's — else 1. A falsy/unknown (0/None/'') per-track disc falls through to
|
||||
the album rather than flooring early. Single source of truth so both call sites
|
||||
stay in lockstep."""
|
||||
for src in ((original_search or {}), (album_info or {})):
|
||||
raw = src.get("disc_number")
|
||||
try:
|
||||
n = int(raw)
|
||||
except (TypeError, ValueError):
|
||||
try:
|
||||
n = int(float(str(raw).strip()))
|
||||
except (TypeError, ValueError):
|
||||
continue
|
||||
if n >= 1:
|
||||
return n
|
||||
return 1
|
||||
|
|
|
|||
|
|
@ -1089,14 +1089,12 @@ def extract_source_metadata(context: dict, artist: dict, album_info: dict) -> di
|
|||
metadata["track_number"] = 1
|
||||
metadata["total_tracks"] = 1
|
||||
|
||||
disc_num = original_search.get("disc_number")
|
||||
if disc_num is None and album_info:
|
||||
disc_num = album_info.get("disc_number")
|
||||
# Floor to >=1: a 0 / '' / non-numeric disc must not slip through (the old
|
||||
# `is not None` let a literal 0 past, which then read as disc-less downstream
|
||||
# and ungrouped the track in Jellyfin/Plex). Also feeds the "Disc N" folder org.
|
||||
from core.imports.track_number import normalize_disc_number
|
||||
metadata["disc_number"] = normalize_disc_number(disc_num)
|
||||
# Resolve via the SHARED resolver so the embedded tag and the "Disc N" folder
|
||||
# (computed in the import pipeline from album_info) can never disagree — same
|
||||
# function, same inputs. Floors to >=1 (a 0/''/non-numeric disc must not read
|
||||
# as disc-less and ungroup the track in Jellyfin/Plex).
|
||||
from core.imports.track_number import resolve_disc_for_track
|
||||
metadata["disc_number"] = resolve_disc_for_track(original_search, album_info)
|
||||
|
||||
if album_ctx and album_ctx.get("release_date"):
|
||||
release_date = _normalize_release_date_tag(album_ctx.get("release_date"))
|
||||
|
|
|
|||
|
|
@ -29,3 +29,41 @@ def test_valid_multidisc_values_preserved():
|
|||
# a real disc on a 4xLP must survive untouched
|
||||
for d in (1, 2, 3, 4):
|
||||
assert normalize_disc_number(d) == d
|
||||
|
||||
|
||||
# ── resolve_disc_for_track: the FOLDER and the TAG must use the same disc ──────
|
||||
|
||||
from core.imports.track_number import resolve_disc_for_track
|
||||
|
||||
|
||||
def test_resolve_disc_prefers_per_track_search_then_album():
|
||||
# per-track disc wins (this is the value the tag uses) — so the folder, which
|
||||
# now calls the SAME resolver with the SAME inputs, lands on the same disc.
|
||||
assert resolve_disc_for_track({"disc_number": 3}, {"disc_number": 1}) == 3
|
||||
# falls back to album context when the per-track search has none
|
||||
assert resolve_disc_for_track({}, {"disc_number": 2}) == 2
|
||||
assert resolve_disc_for_track({"disc_number": None}, {"disc_number": 2}) == 2
|
||||
# both missing -> floored default 1
|
||||
assert resolve_disc_for_track({}, {}) == 1
|
||||
assert resolve_disc_for_track(None, None) == 1
|
||||
|
||||
|
||||
def test_resolve_disc_floors_bad_values():
|
||||
assert resolve_disc_for_track({"disc_number": 0}, {"disc_number": 5}) == 5 # 0 is falsy -> fall to album
|
||||
assert resolve_disc_for_track({"disc_number": "2"}, {}) == 2
|
||||
assert resolve_disc_for_track({"disc_number": "junk"}, {}) == 1
|
||||
|
||||
|
||||
def test_folder_and_tag_resolve_identically():
|
||||
# the regression that matters: given the same (original_search, album_info),
|
||||
# source.py (tag) and the pipeline (folder) get the IDENTICAL disc.
|
||||
cases = [
|
||||
({"disc_number": 2}, {"disc_number": 1}), # Sokhi's case: per-track 2, album 1
|
||||
({"disc_number": 3}, {"disc_number": 1}),
|
||||
({}, {"disc_number": 1}),
|
||||
({"disc_number": 0}, {"disc_number": 1}),
|
||||
]
|
||||
for osrch, ainfo in cases:
|
||||
folder_disc = resolve_disc_for_track(osrch, ainfo) # what the pipeline writes to album_info
|
||||
tag_disc = resolve_disc_for_track(osrch, ainfo) # what source.py writes to the tag
|
||||
assert folder_disc == tag_disc
|
||||
|
|
|
|||
Loading…
Reference in a new issue