Skip to content

Give the audio quality rules a single home - #6

Merged
Kentarohakase merged 1 commit into
mainfrom
refactor/shared-audio-thresholds
Aug 12, 2026
Merged

Give the audio quality rules a single home#6
Kentarohakase merged 1 commit into
mainfrom
refactor/shared-audio-thresholds

Conversation

@Kentarohakase

Copy link
Copy Markdown
Owner

Stacked on #5. The last open item from the improvement plan.

The problem

The numbers that decide when audio counts as weak were written out four times:

Location What it decided
AudioAnalysisInsightService.cs:22-84 The analysis score and its findings
AudioProfileAdvisorService.cs:22-30 Which preset is suggested
AudioValidationService.cs:315-323 The findings on the exported file
MainViewModel.Insights.cs:55-88 The warning strip in the window

Each copy carried the same bitrate bounds (96 000 mono / 128 000 otherwise), the same sample-rate bound (32 000), the same headroom limit (−1.0 dB) and the same loudness band (−28 / −9 LUFS). Changing one of them would have made the application contradict itself — the warning strip saying one thing and the analysis report another — with no test to catch it, because each site was tested against its own copy of the number.

They happened to agree today. That is the point at which to merge them, not after they have drifted.

The change

Services/AudioQualityThresholds.cs holds the constants and the checks built on them. The four call sites ask that type instead of repeating the comparison, and AudioProfileAdvisorService.HasLowBitrate is gone since it was that check verbatim.

Two invariants the copies already agreed on are now stated once instead of being re-derived at each site:

  • Low headroom excludes actual clipping. Clipping is reported on its own, so counting it as low headroom too would put the same problem in the findings twice. Every copy had the else if for this; now the predicate carries it.
  • The two loudness bands cannot overlap, since −28 < −9.

Net: 24 lines added, 45 removed across the four call sites.

Why this is behaviour preserving

The rules are unchanged, and the existing suite already covers them from every side — AudioAnalysisInsightServiceTests (12 scoring cases), AudioProfileAdvisorServiceTests (7 advice cases) and AudioValidationServiceTests (18 report cases). All of them pass untouched.

The 26 new tests in AudioQualityThresholdsTests pin the boundaries themselves — 95 999 vs 96 000, 31 999 vs 32 000, −1.1 vs −1.0 — so a later change to one of those numbers has to be deliberate rather than accidental.

Verification

dotnet format .\AudioQualityEnhancer.slnx --verify-no-changes   -> clean
dotnet build  .\AudioQualityEnhancer.slnx -c Release            -> 0 warnings, 0 errors
dotnet test   .\AudioQualityEnhancer.slnx -c Release            -> 265 passed, 0 failed

No changelog entry

Nothing changes for a user of the application: the same audio produces the same findings, the same advice and the same warnings as before.

@Kentarohakase
Kentarohakase force-pushed the build/shared-properties-and-coverage branch from 876c30f to ad373ef Compare August 12, 2026 00:56
@Kentarohakase
Kentarohakase force-pushed the refactor/shared-audio-thresholds branch from eab2d29 to 57aef9c Compare August 12, 2026 00:56
@Kentarohakase
Kentarohakase force-pushed the build/shared-properties-and-coverage branch from ad373ef to ffc4ba1 Compare August 12, 2026 01:06
The numbers that decide when audio counts as weak were written out four times:
in the analysis scoring, in the profile advice, in the result validation and in
the view model warnings. Each copy carried the same bitrate bounds, the same
sample rate bound, the same headroom limit and the same loudness band. Changing
one of them would have made the interface contradict itself - the warning strip
saying one thing, the analysis report another - with nothing to catch it.

They move into AudioQualityThresholds as named constants with the checks built
on them, and the four call sites now ask that type instead of repeating the
comparison. The rules themselves are unchanged, which the existing analysis,
advice and validation tests confirm; the new tests pin the boundaries so a later
change to a number is a deliberate one.

Two details the copies had already agreed on are now stated once: low headroom
excludes actual clipping, because clipping is reported on its own and would
otherwise appear twice, and the two loudness bands cannot overlap.
@Kentarohakase
Kentarohakase changed the base branch from build/shared-properties-and-coverage to main August 12, 2026 01:08
@Kentarohakase
Kentarohakase force-pushed the refactor/shared-audio-thresholds branch from 57aef9c to 153c588 Compare August 12, 2026 01:08
@Kentarohakase
Kentarohakase merged commit 1497c04 into main Aug 12, 2026
1 check passed
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.

1 participant