Fix M4B re-grab loop: emit codec-prefixed quality labels

[timk75]

Summary

Closes #549.

AudiobookStatusEvaluator.DeriveQualityLabel was emitting quality labels ("M4B", "lossless", "320kbps", "64kbps") that never matched the codec-prefixed keys seeded by QualityProfileService.EnsureProfileHasRequiredQualitiesAsync ("AAC 320kbps" ... "AAC 64kbps", "MP3 320kbps" ... "MP3 64kbps").

ComputeStatus looks up the derived label in a case-insensitive dictionary built from QualityProfile.Qualities. With no match, priority defaults to int.MaxValue, which is never ≤ cutoffPriority, so the cutoff is reported as not met even when the file on disk is at or above the profile's cutoff. Combined with the ~6 h RSS cycle, any monitored audiobook with an .m4b on disk and a matching indexer release ends up re-downloaded every cycle — leaving a stack of …(1) / (2) / (3) … staging folders and duplicate metadata files in the library dir.

Today's repro on a current canary install (Listenarr 0.3.1):

[INF] Cutoff not met for "Artificial Condition" (id=179):
      cutoff met=False, best existing quality=M4B

Despite the file on disk being m4b / aac / 64 kbps, matching AAC 64kbps in the default QP.

Patch

DeriveQualityLabel now:

• detects the codec from Codec / Container / Format, including m4b / m4a / mp4 → aac (audiobooks effectively always use AAC inside MP4 containers);
• buckets the file bitrate to the nearest seeded rung (320 / 256 / 192 / 128 / 64);
• emits "<codec> <bucket>kbps" (for example "aac 64kbps"), which matches QualityProfile.Qualities keys via the existing case-insensitive dictionary lookup;
• still emits "lossless" for flac / alac / wav / aiff / ape / dsd / wavpack so user-defined lossless rungs keep matching;
• only short-circuits to the audiobookQuality hint when the hint is already in codec-prefixed shape ("MP3 320kbps", "AAC 64kbps", "FLAC", "MP3 VBR"). Legacy values such as "M4B" produced by LibraryController.DeriveAudiobookQualityAsync are now ignored so we re-derive from the actual file fields.

No public API change.

Test plan

tests/Features/Api/Services/AudiobookStatusEvaluatorTests.cs was updated to use a production-shaped QualityProfile (the same 11-rung ladder seeded by QualityProfileService) and now also covers:

• ComputeStatus_ReturnsDownloading_WhenIsDownloading
• ComputeStatus_ReturnsQualityMatch_WhenProfileIsNull
• ComputeStatus_ReturnsQualityMatch_ForM4BFileMeetingAacCutoff — the issue AudiobookStatusEvaluator.DeriveQualityLabel produces labels that never match QualityProfile.Qualities keys → all books report quality-mismatch #549 regression
• ComputeStatus_TreatsM4AContainerAsAac
• ComputeStatus_BucketsBitrateDownToNearestRung (200 kbps must bucket to 192, not 256)
• ComputeStatus_AcceptsBitrateBelowLowestRungAsLowestRung (< 64 kbps still resolves to a known rung so we don't re-introduce a perpetual re-grab loop)
• ComputeStatus_UsesAudiobookQualityHintWhenCodecPrefixed
• ComputeStatus_IgnoresLegacyM4BHintAndRederivesFromFile
• ComputeStatus_TreatsFlacAsLossless
• ComputeStatus_TreatsAlacAsLossless
• existing ComputeStatus_TreatsWavPackAsLossless retained, now backed by a profile shape consistent with the rest of the tests.

Local results (.NET SDK 10.0.203, Release configuration):

Passed!  - Failed:     0, Passed:    16, Skipped:     0, Total:    16   (evaluator tests)
Passed!  - Failed:     0, Passed:   623, Skipped:     0, Total:   623   (full suite)
dotnet format listenarr.slnx --verify-no-changes  → exit 0

Things I deliberately did NOT change

• LibraryController.DeriveAudiobookQualityAsync still returns the legacy "M4B" string for the per-audiobook Quality column. That value is now harmless because the evaluator ignores hints that aren't in codec-prefixed shape, and changing the controller as well would require a DB-side rewrite of existing values. Happy to follow up if maintainers prefer it cleaned up in the same PR.
• No 96 kbps rung was added (none exists in the seed table) and VBR is not auto-detected (would require multi-sample probing) — files below 64 kbps map to the lowest rung so they still satisfy the lowest possible cutoff.

[T4g1]

I'll leave it to someone with more experience in codecs/bitrate to approve or not, but this is much needed and lgtm!

[T4g1]

Very minor: Why not use string.IsNullOrEmpty() instead of the classic Length > 0 ?

[timk75]

Good call — switched to string.IsNullOrEmpty() in 83fc8e6.

[T4g1]

Same remark here

[timk75]

Same — fixed in 83fc8e6.

[therobbiedavis]

@timk75 There's some more long-term questions we need to resolve here. Also this branch needs to be rebased, files moved to their new appropriate directories, and the tests should follow the new framework. This is rather new, in the last month, but you can find this info documented in CONTRIBUTING.md

[therobbiedavis]

So given this, shouldn't we also update the FE to remove the ability for users to create 96 kbps AAC/OPUS qualities? I think it would be confusing for it to get bucketed down to 64 here.

[therobbiedavis]

The normalization here is fine, but I think the returned label may still not match real FLAC profiles. A file with codec FLAC gets normalized to flac, passes IsLosslessCodec, and then returns lossless, but the profile model documents quality IDs like FLAC in QualityProfile.cs, and the quality profile UI parses/adds FLAC as FLAC in QualityProfileFormModal.vue. So the lookup can still miss unless the profile has a custom lossless quality.

There are two options: Revert back to the quality label or add a compatibility path and keep lossless:

For the former we'd need to use the profile quality IDs instead of using the a generic category to match. For FLAC that means returning flac/FLAC instead of lossless, or better, deriving against the current profile’s Qualities so custom lossless labels and future codec names don’t need special-case branches.

If we want to keep lossless as a generic label, I think the comparison needs to treat it as equivalent to the actual lossless profile keys. For example, when derived quality is lossless, check whether the profile contains FLAC, ALAC, WAV, etc., or map the detected codec to a profile key first and fall back to lossless only for legacy/custom profiles.

What do you think?

[therobbiedavis]

Small naming mismatch here: this returns vorbis, which I think is technically correct, but the UI/profile shape looks like OGG Vorbis. That means OGG Vorbis profiles still won’t match correctly.

[therobbiedavis]

Should we base this on the actual profile qualities instead of a fixed list of trusted labels? That would make custom qualities and future codecs work without needing to keep extending this method.

[therobbiedavis]

Depending on which we we go as far as "lossless" or not, could we add a test for the normal FLAC shape too?

[timk75]

Thanks for the review — you're right on all five points, and reading your comments together with the canary code (QualityDefinition.Codec/Bitrate/IsLossless are structured fields now) made it click: the right fix is profile-driven matching, not a smarter label emitter. That collapses the OGG Vorbis casing, the FLAC default-profile miss, the 96 kbps custom-rung question, and the hint allow-list into one approach.

Plan: rebase onto canary, move to the new file/test conventions, and replace DeriveQualityLabel's string emission with a small MatchFileToProfile(file, profile) walking profile.Qualities.

Two semantic decisions I'd like you to make before I push, so I'm not guessing your intent:

1. Within-codec rung selection — for an AAC 200kbps file against a profile containing [AAC 320, AAC 192, AAC 64], do you want round-down (→ AAC 192, treats cutoff as "≥ this rung") or nearest by distance (→ AAC 192 here but AAC 256 for 220 kbps)? I'd lean round-down because the existing cutoff semantics read as "file is at least X", but happy to follow whatever the codebase has established.

2. File codec not in profile at all — e.g. an OPUS file against a profile with only MP3/AAC/FLAC rungs. Return QualityMismatch? Today it silently hits int.MaxValue (the original bug shape) — I just want it to have a defined outcome.

On the side points: yes to "OGG Vorbis" matching by Codec field; yes to making the hint check profile.Qualities.Contains(hint, ignoreCase) so custom rungs work; the 96 kbps FE question dissolves once matching is profile-driven. I'll leave LibraryController.DeriveAudiobookQualityAsync's legacy "M4B" return value alone unless you'd prefer it cleaned up in the same PR.

Happy to force-push #581 or open a draft PR if you'd rather review the matcher fresh — let me know which you prefer.

[therobbiedavis]

Thanks for the review — you're right on all five points, and reading your comments together with the canary code (QualityDefinition.Codec/Bitrate/IsLossless are structured fields now) made it click: the right fix is profile-driven matching, not a smarter label emitter. That collapses the OGG Vorbis casing, the FLAC default-profile miss, the 96 kbps custom-rung question, and the hint allow-list into one approach.

Plan: rebase onto canary, move to the new file/test conventions, and replace DeriveQualityLabel's string emission with a small MatchFileToProfile(file, profile) walking profile.Qualities.

Two semantic decisions I'd like you to make before I push, so I'm not guessing your intent:

1. **Within-codec rung selection** — for an `AAC 200kbps` file against a profile containing `[AAC 320, AAC 192, AAC 64]`, do you want **round-down** (→ `AAC 192`, treats cutoff as "≥ this rung") or **nearest by distance** (→ `AAC 192` here but `AAC 256` for 220 kbps)? I'd lean round-down because the existing cutoff semantics read as "file is at least X", but happy to follow whatever the codebase has established.

2. **File codec not in profile at all** — e.g. an OPUS file against a profile with only MP3/AAC/FLAC rungs. Return `QualityMismatch`? Today it silently hits `int.MaxValue` (the original bug shape) — I just want it to have a defined outcome.

On the side points: yes to "OGG Vorbis" matching by Codec field; yes to making the hint check profile.Qualities.Contains(hint, ignoreCase) so custom rungs work; the 96 kbps FE question dissolves once matching is profile-driven. I'll leave LibraryController.DeriveAudiobookQualityAsync's legacy "M4B" return value alone unless you'd prefer it cleaned up in the same PR.

Happy to force-push #581 or open a draft PR if you'd rather review the matcher fresh — let me know which you prefer.

Yes, we should use round-down, using the highest rung that the profile actually meetsso we can keep the cutoff semantics as consistent as “file is at least this quality”.

As for "codec not in profile", yes we should make that defined as QualityMismatch. If the profile only allows MP3/AAC/FLAC and the file is OPUS, then logically it should not satisfy the cutoff unless OPUS is present in profile.Qualities.

For the legacy m4b return, we should clean that up, but I also think for longer term maintainability PreferredFormats should probably become PreferredContainers and only handle container/extension filtering. Qualities should handle codec/bitrate/lossless.

What that means for this PR is:
updating the status evaluator, automatic-search cutoff path, search scorer, quality profile form/types, and tests so the split is clear:
PreferredContainers: m4b, m4a, mp3, flac, ogg, opus, etc.
Qualities: AAC 128kbps, MP3 320kbps, FLAC, OGG Vorbis 192kbps, etc.

We also need to migrate user's currently existing PreferredFormats column/data, and have the matcher treat the new field strictly as container/extension tokens.

[timk75]

Round-down + QualityMismatch for missing-codec are clear, will implement.

On the larger refactor: the PreferredFormats → PreferredContainers split is right — the default {m4b, mp3, m4a, flac, opus} genuinely mixes containers and codecs. But it pulls in the domain rename + an EF migration with a user-data copy step, QualityProfileService / repo / configuration, the scorer + automatic-search cutoff path, and the FE side: QualityProfileFormModal.vue, QualityProfilesTab.vue, audiobookStatus.ts, plus the Vitest specs.

That's substantially separate from the evaluator fix in scope and risk — especially the migration. I'd rather land them as two PRs so the migration gets its own review; happy to file a follow-up issue with a concrete plan.

108 commits behind canary + new test framework means the rebase is a rewrite anyway. I'd rather open a fresh draft for the matcher and close #581 — OK?

[therobbiedavis]

Round-down + QualityMismatch for missing-codec are clear, will implement.

On the larger refactor: the PreferredFormats → PreferredContainers split is right — the default {m4b, mp3, m4a, flac, opus} genuinely mixes containers and codecs. But it pulls in the domain rename + an EF migration with a user-data copy step, QualityProfileService / repo / configuration, the scorer + automatic-search cutoff path, and the FE side: QualityProfileFormModal.vue, QualityProfilesTab.vue, audiobookStatus.ts, plus the Vitest specs.

That's substantially separate from the evaluator fix in scope and risk — especially the migration. I'd rather land them as two PRs so the migration gets its own review; happy to file a follow-up issue with a concrete plan.

108 commits behind canary + new test framework means the rebase is a rewrite anyway. I'd rather open a fresh draft for the matcher and close #581 — OK?

Sorry I somehow missed this, yes please go ahead.

[timk75]

Superseded by #648, which implements the profile-driven matching we discussed (round-down via a shared QualityMatcher, QualityMismatch for codecs not in the profile, OGG Vorbis matched by codec, and the cutoff check unified across the status evaluator, automatic search, and library controller). It also corrects the inverted priority comparison in automatic search. Rebasing this branch onto canary was a rewrite (108 commits behind + the new test framework), so #648 is a fresh branch as agreed. The PreferredFormats → PreferredContainers refactor is tracked separately in #647. Closing in favour of #648.