aethersdr / aethersdr/AetherSDR

[RFC] Operator control of AGC position relative to noise reduction

Open
#5,704 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

audio GUI maintainer-review New Feature rfc
Dominant language
C++
Stars
221
Forks
117
Avg merge
2d 9h
Merged PRs (30d)
302

Description

Preflight
  • I have read GOVERNANCE.md and confirmed this change requires an RFC
  • I have searched existing issues and this RFC has not been proposed before
  • I have not opened a PR for this change yet
Problem

Noise reduction has a preferred position relative to the AGC, that position is
not the same for every algorithm, and AetherSDR cannot express any of it.

1. What WDSP actually does

Every NR stage in the RXA chain is called twice, once before the AGC and
once after, with a position field deciding which call acts
(third_party/wdsp/upstream/RXA.c):

xanf(0)  xanr(0)  xemnr(0)  xnnr(0)     <- pre-AGC
xwcpagc(agc)
xanf(1)  xanr(1)  xemnr(1)  xnnr(1)     <- post-AGC

The defaults are not uniform:

stage WDSP default position
ANF (auto notch) 0 pre-AGC
ANR (leaky LMS) 0 pre-AGC
EMNR (spectral — our NR2) 0 pre-AGC
NNR (neural) 1 post-AGC

Warren's reasoning, from the WDSP Guide §"A Word About Using Noise Reduction":

If using Post-AGC NR, the AGC Threshold should be as far as practical above
the noise floor. A Pre-AGC position will generally offer the best
signal-to-noise ratio going into the NR, which is beneficial; however, a
Post-AGC position may offer better immunity to sudden noise changes.

For a statistical algorithm, cleaner input wins — hence pre-AGC for the older
three. A trained network takes the opposite trade: it learned on material with a
bounded dynamic range, and un-AGC'd HF hands it static crashes and strong-signal
swings well outside it, so the AGC is doing useful work before the model sees
anything.

2. What AetherSDR does

Everything is post-AGC, by construction and without a choice. All seven
client methods run in AudioEngine, which sits at the end of the chain:

  • on a Flex or an Icom the AGC is in the radio, upstream and unreachable;
  • on HL2/ANAN/RTL the AGC is WDSP's own inside WdspChannel, and our NR still
    runs after the whole channel rather than inside its chain.

So NNR lands where upstream wants it, by luck rather than design, and NR2 lands
where upstream would rather it didn't
— our port of emnr.c runs post-AGC
while WDSP defaults it pre-AGC for the SNR reason above.

3. How this surfaced

An operator reported NNR sounding poor on the air, and found by experiment that
switching the AGC off fixed it. That is the documented interaction, not a defect:
NNR normalises input power with a 2-second time constant
(NNET_TAU_DEFAULT), and an AGC riding the noise floor moves the level faster
than that tracker follows. The remedy in the Guide is the AGC threshold, which
is now noted in the NNR tab — but "turn the AGC off" should not be the only way
an operator can put NR ahead of gain control.


Proposal

Two halves, because the problem is not the same on both sides of the seam.

4. Where we demodulate (HL2, ANAN, RTL) — use the chain we already open

No new DSP is needed. WdspChannel already opens a chain with every NR stage
present at both positions; nothing exposes the position field. Wiring
SetRXAEMNRPosition / SetRXAANRPosition / SetRXANNRPosition through the
facade gives operator-controlled placement for host-demodulated backends at the
cost of a few facade entries.

The wrinkle: our NR runs in AudioEngine, not in that chain, so using the chain
means a second implementation of the same methods for these backends — or
accepting that position control only applies to the WDSP-chain copies. §7 covers
this.

5. Where the radio demodulates (Flex, Icom) — a client-side AGC stage

The AGC that matters is the radio's, and the only way to get NR in front of gain
control is to move the gain control to where we can reach it: set the radio's AGC
off or fixed, and run one client-side.

This does not mean writing an AGC. WDSP's wcpAGC is already vendored and
already compiled in, and it runs standalone exactly as NNR and the impulse
blanker do — create_wcpagc(), xwcpagc(WCPAGC), setBuffers_wcpagc(),
setSamplerate_wcpagc() all take a handle; only the SetRXAAGC* property
wrappers touch ch[]/rxa[]. It is the same algorithm the HL2 path already
runs and the same family the Flex runs, positioned where we choose.

5.1 The transport constraint, corrected

A first pass at this assumed un-AGC'd audio would not survive the wire. That is
wrong for the primary path and right for two others:

path on the wire un-AGC'd audio
narrow (normal LAN) VITA PCC 0x03E3, big-endian float32 fine — full float headroom
reduced bandwidth PCC 0x0123, Int16 mono ~96 dB; static crashes clip
SmartLink PCC 0x8005, Opus lossy and tuned for speech at moderate level; the real constraint

So the Flex half is viable on the path most operators use, and needs a policy for
the other two — most likely: offer client-side AGC only when the narrow float32
path is active, and keep the radio's AGC authoritative otherwise.

6. Governance: Principle III

Principle III says radio-persistable settings live on the radio, and explicitly
carves out "client-side-only DSP" as state the client may persist. A client
AGC is therefore permitted — provided it never becomes a second control for the
radio's AGC
. The line this RFC proposes:

  • the radio's AGC mode/threshold stay radio-owned and radio-persisted, exactly as
    today;
  • the client AGC is a distinct stage with its own client-persisted config, shown
    as such;
  • the UI must never present one control that could mean either. Two AGCs with one
    label is precisely the second source of truth Principle III exists to prevent.

7. Risks

  1. Two AGCs running at once. If the operator enables the client stage without
    turning the radio's down, they get cascaded gain control and worse pumping
    than today. The feature has to make the radio's state visible, and arguably
    set it.
  2. Metering. AGC gain is displayed from the radio (agc_gain, and
    RXA_AGC_GAIN on the WDSP path). With gain control client-side, that readout
    is measuring the wrong stage.
  3. AgcTCalibrator and AgcCalibrationDialog are built around the radio's
    AGC-T. They would need to know which AGC is authoritative.
  4. Per-backend divergence. Position control via §4 and via §5 are different
    mechanisms with different capabilities; operators would see one control
    behaving differently per radio unless the UI reconciles them.
  5. It is the audio pipeline. New stage, new ordering, new interaction with
    the transport — the class of change this RFC process exists for.

8. Alternatives considered

Do nothing; document the AGC-threshold guidance. Already done for NNR, and it
is what the Guide recommends. Cheapest, and leaves NR2 permanently in the
position upstream considers second-best.

Move all client NR into the WDSP chain. Would give position control
everywhere we demodulate and nothing on Flex, which is the majority of users.
Rejected as backwards.

Expose position only (§4), skip the client AGC. Small, no transport
questions, helps HL2/ANAN/RTL only. A reasonable first step, and the fallback if
§5 does not survive scrutiny.

9. Open questions

  1. Is the Flex half worth it given it requires operators to turn the radio's
    AGC off — a significant behavioural ask for a benefit measured in NR quality?
  2. Should §4 and §5 ship together or separately? They are independent and §4
    is much smaller.
  3. What happens on SmartLink and reduced-bandwidth paths — refuse the client
    AGC, or allow it with a warning?
  4. Which AGC feeds the meter, and does the S-meter's calibration survive the
    change?
Cross-platform impact

None specific. wcpAGC is the same vendored C on all three platforms, and the
transport paths are identical. The work is in AudioEngine, the facade and the
UI, none of it platform-conditional.

Implementation scope
  1. Facade entries for the three Set*Position calls; wire position for the
    WDSP-chain NR on host-demodulated backends (§4).
  2. Measure the Flex narrow path with the radio's AGC off — actual dynamic range
    against the float32 wire — and confirm §5.1 before building anything.
  3. WcpAgcFilter as a standalone client stage, shaped after NnrFilter.
  4. UI and policy: which AGC is authoritative per backend, metering, and the
    Principle III boundary in §6.

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 the RXA chain in third_party/wdsp/upstream/RXA.c, then trace AudioEngine, WdspChannel, the facade position setters, and NnrFilter. Before implementation, resolve the open questions around host-demodulated versus radio-demodulated backends, transport limits, authoritative AGC state, and metering. Done requires an agreed scope and policy for position control or a client-side AGC, with the affected UI and persistence behavior defined.

Written by the indexing model from the issue text.

Assessment

Tech stack
c, cpp
Domain
audio-video-rtc, desktop
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Needs clarification
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.