aethersdr / aethersdr/AetherSDR

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

Open
#5,802 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

claude-active
Dominant language
C++
Stars
221
Forks
117
Avg merge
2d 9h
Merged PRs (30d)
302

Description

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

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with MetisProtocol.h and MetisProtocol.cpp to trace Ep4Stats accumulation, merge(), rmsDbfs(), and the consumers adcCrestDb and bandscopeHeadroom(). Resolve whether peakAbs should be mean-referred and whether DC should be exposed, then update the statistics consistently and verify that merged packet blocks preserve the intended RMS and crest-factor behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.