Read embedded ASIN/ISBN tags during scan (fixes Rescan Metadata for tagged files)

[dny238]

Fixes #780

Problem

When ListenArr scans an existing audio file that has an ASIN embedded in its tags (the freeform iTunes atom ----:com.apple.iTunes:ASIN that Audible rips and most audiobook taggers write), the ASIN is never extracted. The imported audiobook gets no identifier, and Rescan Metadata then fails immediately with "No ASIN or ISBN identifiers are available for metadata rescan" — even though the value is sitting in the file.

Root cause: FfprobeTagMetadataMapper.Apply() maps title/artist/album/track/disc/year but never reads the ASIN (or ISBN) tag.

Fix

• FfprobeTagMetadataMapper.Apply() now reads the ASIN and ISBN tags into AudioMetadata. AudioMetadata already had Asin/Isbn properties, and GetTag already matches case-insensitively (so the ----:com.apple.iTunes:ASIN atom, which ffprobe surfaces as ASIN, is picked up).
• AudiobookFileService.EnsureAudiobookFileAsync() — the shared file-registration path the scan job uses — now adopts those identifiers onto the audiobook when it has none, persists via the audiobook repository, and records a history entry. Existing identifiers are never overwritten. Since AudiobookIdentifierMapper.GetEffectiveIdentifiers backfills from the legacy Asin/Isbn fields, setting them is sufficient for Rescan Metadata to succeed.

Scope / notes

• Adopts only when the audiobook is missing the identifier — no clobbering of user- or metadata-supplied values.
• Follows the issue's suggested approach (steps 1–3). ISBN is included alongside ASIN so nothing the mapper reads goes unused.

Tests

• New FfprobeTagMetadataMapperTests: ASIN + ISBN extraction, case-insensitive ASIN match, absent-tag leaves null, and existing ASIN not overwritten.
• dotnet build clean (0 warnings). Related suites (Files / Scanning / Ffmpeg / Metadata) — 174/174 pass.

🤖 Generated with Claude Code

[m4bard]

On #780 I said the MP4 key ought to be checked before anyone leaned on it, so I checked it rather than leave the hedge sitting there. Your assumption holds. Tagged files in each dialect, read with the command Listenarr runs (ffprobe -v quiet -print_format json -show_format -show_streams), tag appearing under format.tags:

dialect	written as	ffprobe exposes
MP4 / M4B	----:com.apple.iTunes:ASIN	ASIN
ID3v2.3	TXXX:ASIN	ASIN
ID3v2.4	TXXX:ASIN	ASIN (and AUDIBLE_ASIN)
Vorbis / FLAC	asin	asin

The freeform atom does come through unprefixed, so GetTag(tags, "ASIN", "asin") covers all four. The lowercase spelling is doing real work for FLAC. Nothing to change there.

There is one thing I'd think about before this lands, because it runs into a bug that's already live.

AdoptFileIdentifiersAsync takes the ASIN off a linked file and writes it onto the record, which is only as reliable as file-to-book attribution is. On canary that attribution is wrong in a way I measured on #784: scanning Ghost Stories of an Antiquary by M. R. James also linked Henry James' The Turn of the Screw and James M. Barrie' Peter Pan in Kensington Gardens, because the matcher will accept "this file sits somewhere on this author's shelf" as grounds for attribution.

The adopt only fires when the audiobook has no ASIN, which is the library-import case this PR is aimed at, so the two conditions overlap rather than excluding each other. A wrongly linked file donates its ASIN, TryPopulateMissingMetadataAsync resolves that ASIN upstream and fills the empty fields, and it all gets persisted with a history entry. The book now claims to be a different book, and that's harder to notice afterwards than a stray file in a list.

A cheap guard would be to require the linked files to agree: if the files linked to one audiobook carry more than one distinct ASIN, that disagreement is itself a signal the attribution is off, so adopt nothing instead of picking one. When attribution is right it changes nothing. Waiting until attribution is tightened would also do it, since #717 rewrites this area and wants a title match rather than accepting the author on its own.

Two smaller things:

• The ISBN path doesn't have the guard the ASIN path has. ASIN is adopted only when audiobook.Asin is blank, but ISBN appends any new value found in any scanned file regardless of what the book already holds. The doc comment says existing identifiers are never overwritten, which is true, but it doesn't mention the accumulation, and that's the route a wrongly linked file takes.
• The new if (exists) branch re-extracts metadata for every already-registered file whenever the book still has no ASIN. For a book that genuinely has no embedded ASIN, that's an ffprobe per file on every scan, with nothing remembering the negative result.

On the tests: they assert against hand-built JSON, so they'd pass unchanged even if ffprobe surfaced the tag under some other key, which is the one thing this PR depends on. The table above is that assumption actually checked. I have a corpus of tagged public-domain files covering every dialect if a fixture test would help, and I'm happy to hand over the files or the generator that makes them.

[dny238]

Thanks for actually tagging files in each dialect and checking format.tags — that's the assumption the whole PR rests on, and having it verified rather than asserted is exactly what the tests couldn't give you. Appreciate the offer of the corpus; a fixture built from real tagged files would be a strict improvement over the hand-built JSON, and I'd take you up on the generator.

Pushed 4def14c addressing the three points:

• Attribution guard. Adoption now requires agreement: ResolveUnanimousFileAsinAsync collects the ASINs across every file linked to the book and adopts only when they're unanimous — on any disagreement it adopts nothing rather than picking one, so a mis-attributed file can't donate its identifier for the auto-refresh to then act on. This is defense-in-depth; the real fix is attribution itself, which is why Match scanned files when the on-disk name drops "The" or author credentials #784 now drops its author arm and Harden path identity, durable moves, scans, and root relocation #717 tightens the matcher.
• ISBN. No longer accumulates — it's adopted only when the book has none, same rule as ASIN. That closes the route you flagged where a stray file appends onto an existing set.
• ffprobe cost. The already-registered re-read now reuses the per-file/mtime cache the create path populates, so a book with no embedded ASIN doesn't re-run ffprobe on every file each scan.

One honest limitation I'd rather name than paper over: the agreement check sees the files linked at the moment adoption fires, and adoption fires on the first file that carries an ASIN, so a conflicting file linked later in the same scan can still slip through the window. Tightening that fully means moving adoption to a post-scan step once the file set is settled — happy to do that if you'd prefer it over the incremental guard, but it felt like more surface than this PR should carry given #717/#784 are removing the mis-attribution at the source.

[m4bard]

All three land, and the unanimity rule is a better shape than what I suggested. Refusing on disagreement rather than trying to pick a winner is the part that makes it safe.

Readarr's answer to this is neither of the two options we were choosing between. It never adopts an identifier onto a book at all: it groups the files, identifies the group against candidate editions, and treats the embedded ASIN as one weighted input to that choice.

IdentificationService.cs#L50-L55 groups first, then identifies each group:

releases = _trackGroupingService.GroupTracks(localTracks);
foreach (var localRelease in releases) { ... IdentifyRelease(localRelease, ...); }

DistanceCalculator.cs#L89-L98 reads the ASIN across that group and scores it:

var asin = localTracks.MostCommon(x => x.FileTrackInfo.Asin);
if (asin.IsNotNullOrWhiteSpace() && edition.Asin.IsNotNullOrWhiteSpace())
    dist.AddBool("asin", asin != edition.Asin);
else if (asin.IsNullOrWhiteSpace() != edition.Asin.IsNullOrWhiteSpace())
    dist.AddBool("asin_missing", true);

with the weights in Distance.cs#L16-L19.

Grouping before identification has been there since 45d49117 in July 2020, so this is a settled design rather than a recent experiment.

MostCommon means plurality decides across the group, so where your check refuses on disagreement, Readarr takes the majority. Both are defensible and I do not think this is my call: refusing is the safer default, taking the majority matches the prior art, and there is a middle version where you take the majority but log the disagreement so it is visible rather than silent. Which of those do you want here?

The weights are worth a look either way. 847a9eae, "Improve match by adding small penalty for missing isbn/asin", November 2021, puts a contradicting identifier at 10.0 and a missing one at 0.1. Absence of evidence costs almost nothing, conflicting evidence is close to fatal. That is the same instinct as your unanimity rule, expressed as a score rather than a veto.

On the window you asked about. It is narrower than "later in the same scan", and it cuts in a direction worth naming. Adoption runs from EnsureAudiobookFileAsync, so once per file, and only while audiobook.Asin is blank. It fires on the first file carrying an ASIN and never runs again for that book, so whatever happened to be registered before that file is the entire set the check gets to see.

That makes the two paths behave differently. Re-scanning a book that already has its files linked, your new if (exists) path, hands it the settled set, and the agreement is real. A first scan reaches the first tagged file with almost nothing linked yet, often only that file, so the set is unanimous by having one member and the check passes without telling you anything. The first scan is also when misattribution happens, so the guard is weakest on the case that motivated it.

None of which is a reason to hold the PR. Deciding once the file set has settled is what actually closes the window, but it is a different shape of change, and you are right that #784 and #717 remove the cause rather than the symptom. The one thing I would add now is a sentence on ResolveUnanimousFileAsinAsync recording that the set is whatever was linked when the first tagged file arrived. Otherwise the limitation lives only in this thread, and the next person will read a guard that looks airtight and wonder why it did not fire.

I can measure it instead of leaving it as a reading. The corpus has a deliberate wrong-ASIN tag state and the attribution harness can build a library where a scan links a file belonging to another book, so pointing those at each other would show whether a conflicting ASIN reaches adoption before the check sees it. Say the word and I will run it against 4def14c9.

On fixtures, yes, take whatever is useful. tools/generate_library.py --scenario tag-dialects writes the same ASIN into MP4 atoms, ID3v2.3, ID3v2.4 and Vorbis comments, so you get one real file per dialect instead of four hand-written JSON blobs. Public domain, verified ASINs, one second of silence each, so the whole set is kilobytes. I can cut a fixture directory and open it as a PR against your branch if that is easier than wiring in a generator.

[therobbiedavis]

#717 changes AudiobookFile registration into a database-enforced filesystem identity/ownership contract and serializes path-bearing mutations through the audiobook operation boundary. The ASIN/ISBN tag extraction in this PR remains distinct, but please rebase on #717 and make identifier adoption run through the new registration/operation-lock flow so it cannot race file ownership or concurrent audiobook updates.

[dny238]

@therobbiedavis sounds good — I'll rebase this on #717 once it lands and route the identifier adoption through the new registration/operation-lock flow so it can't race file ownership or concurrent audiobook updates. Happy to hold until #717 merges so I'm building against its final shape rather than a moving target.

@m4bard thanks for the careful read. You're right that the guard is weakest on a first scan, where the first tagged file is "unanimous" just by being the only member — I've pushed a doc comment on ResolveUnanimousFileAsinAsync spelling out exactly that limitation (it refuses once a second, disagreeing file appears, but can't retroactively un-adopt an ASIN from a lone early file). And yes please on the fixtures — real tagged files across tag dialects would let me replace the hand-built metadata in the tests with something closer to what actually lands on disk.

[m4bard]

Fixtures are ready. tools/make_tag_fixtures.py in https://github.com/m4bard/listenarr-testdata builds them:

python3 tools/make_tag_fixtures.py --out fixtures/tag-adoption

Six directories, each one book whose files are tagged individually:

case	files	what it is
agree-same-dialect	3 tagged	the easy adopt
agree-mixed-dialect	3 tagged	same ASIN, spelled three ways
disagree	2 tagged	two different real ASINs
partial-one-tagged	1 of 3 tagged	one tagged file among bare ones
partial-lone-file	1 file	a single tagged file, nothing else
untagged	0 tagged	no identifier in any spelling

agree-mixed-dialect is the one that will exercise your extraction rather than your comparison. The same ASIN comes back from ffprobe as ASIN on the m4b, as both ASIN and AUDIBLE_ASIN on the mp3, and as lowercase asin on the flac, so agreement has to be decided after extraction and cannot be done by comparing raw tag keys.

partial-lone-file is the case from your doc comment. One tagged file is unanimous by construction, so a guard that adopts on agreement adopts from it, and there is no second file to disagree later.

partial-one-tagged is the question next to it: one tagged file plus two bare ones is unanimous among the files that carry an identifier at all, which is a choice worth making explicitly rather than falling into.

Each set writes a manifest.json listing, per file, which ASIN was written and under which keys, so a test can assert against that rather than against hardcoded strings. Audio is one second of generated silence, so the whole set is small. Both ASINs are real catalogue values from the corpus and a test asserts they resolve there.

Two things I checked rather than assumed. Every file was read back through ffprobe to confirm the identifier actually surfaces, because fixtures that silently carry nothing would turn a passing test into a test of nothing. And rebuilding replaces the set rather than merging into it, which matters because a stale file surviving into untagged would make that case quietly wrong.

If the shape is not what you need, say what is missing and I will add cases. Adding one is a few lines.