Ep4Stats::rmsDbfs measures about zero, not about the mean, so converter DC deflates adcCrestDb

[on8st]

Ep4Stats::rmsDbfs() computes the RMS about zero rather than about the sample mean, so any converter DC offset is carried into the result at full weight. That inflates the reported RMS, and since the crest factor is derived as peak − RMS, it deflates adcCrestDb.

The code

MetisProtocol.h, struct Ep4Stats, carries no sum member — only samples, peakAbs, sumSquares, clippedSamples — so the mean is not tracked and cannot be removed downstream:

struct Ep4Stats {
    int    samples        = 0;
    int    peakAbs        = 0;    // 0..kEp4FullScale
    double sumSquares     = 0.0;  // of raw codes, so rms shares peak's scale
    int    clippedSamples = 0;

MetisProtocol.cpp accumulates the raw code:

s.sumSquares += static_cast<double>(code) * static_cast<double>(code);

and Ep4Stats::rmsDbfs() divides by the sample count directly:

const double rms = std::sqrt(sumSquares / static_cast<double>(samples));
return 20.0 * std::log10(rms / static_cast<double>(kEp4FullScale));

For a signal with mean m and standard deviation σ, that yields √(m² + σ²) rather than σ.

Why it matters

Both adcCrestDb and bandscopeHeadroom() consume it. Crest factor is one of the few surfaces that distinguishes broadband noise from a discrete carrier — Gaussian noise over 2048 samples gives 11–12 dB, a sinusoid gives ~3 dB — and a DC pedestal pushes the reading toward the sinusoid end regardless of what the RF is actually doing.

This is not hypothetical; it cost us a diagnosis. Reading adcCrestDb = 3.71 dB on a 50 Ω dummy load, we concluded the input was a near-sinusoidal carrier at roughly −36 dBm. It was not. Solving peak = m + 3.4σ against rms = √(m² + σ²) shows a DC pedestal with σ/m = 0.163 fits that crest exactly as well as a sinusoid with σ/A = 0.025. The statistic cannot choose between them, and we spent a run's worth of effort on a characterisation the number could not support.

Suggested fix

Add a double sum to Ep4Stats, accumulate s.sum += code, merge it alongside sumSquares, and compute the variance form:

const double mean = sum / samples;
const double var  = std::max(0.0, sumSquares / samples - mean * mean);
const double rms  = std::sqrt(var);

merge() stays a plain addition for sum, exactly as it already is for sumSquares, so block-from-packets merging is unaffected.

Two things worth deciding rather than assuming, which is why this is an issue and not a PR:

1. Whether peakAbs should also become mean-referred. Leaving it absolute while RMS becomes AC-coupled changes the meaning of their difference. Probably both should be AC-referred for the crest to mean what its name says, but that is a call about what adcCrestDb is for.
2. Whether the DC itself is worth surfacing. If there is a real converter offset, a reported adcDcDbfs would be more useful than silently removing it — and would have answered our question directly.

Happy to send the patch once someone states a preference on (1).

Provenance

Found while investigating an HL2 receive noise floor on a terminated antenna port. Code read on origin/main; the struct, the accumulation site and rmsDbfs() were each checked rather than inferred. No claim here rests on the hardware measurement — the defect is visible in the source alone.

🤖 Generated with Claude Code

[on8st]

Correcting my own body before anyone acts on it: bandscopeHeadroom() does not consume rmsDbfs().

The line "Both adcCrestDb and bandscopeHeadroom() consume it" is wrong. Hl2BandscopeHeadroom.h reads peakDbfs(), peakAbs and clippedSamples — it never touches the RMS. Across src/, Ep4Stats::rmsDbfs() has exactly one production caller: the adcRmsDbfs / adcCrestDb health rows in Hl2Backend.

That makes the blast radius smaller than I claimed, not larger, so it is a correction that weakens my own case and is worth making for that reason. The defect itself is unchanged: the RMS is still taken about zero, converter DC is still carried at full weight, and adcCrestDb is still deflated by it.

Implemented locally, not filed

A fix is written and tested against main, held back pending a direction on question 1:

• double sum added to Ep4Stats, accumulated in ep4Stats() alongside the square, summed in merge(), and the variance form computed in rmsDbfs() with a std::max(0.0, …) clamp — a DC-only record can land a few ULPs below zero, and sqrt of that is a NaN that renders as "nan" in a health row forever. sumSquares keeps its raw-code scale, so rms still shares peak's scale and adcCrestDb stays scale-free.
• Full ctest: 100% of 520 passed, HAVE_HIDAPI OFF.
• The test fails before and passes after: reverting only rmsDbfs()'s body to the about-zero form gives three failures, including "a record that is pure DC has no AC content and reads the floor".

Two things found on the way that are worth recording here:

An existing assertion had the defect baked into it. A test fixture fed a constant +1024 while its own comments described it as being about constant magnitude. Under mean-referred RMS that record is a DC pedestal with no AC content at all. The fixture now alternates {1024, -1024}, which is what the assertions always said it was; every existing check keeps its text and holds under both definitions.

Question 1 is still open and I did not decide it. peakAbs stays absolute. What the patch does instead is state the boundary in the header and in the adcCrestDb comment: peak is absolute, RMS is AC, and a large crest therefore means either a peaky signal or a large DC offset under a quiet band. Question 2's adcDcDbfs row is not implemented either. Both are still yours to rule on, and I would rather the ambiguity be documented than silently resolved by whoever wrote the patch.

🤖 Generated with Claude Code