Skip to content

streaming: saturate a transmit sample past full scale instead of wrapping - #47

Open
willcode wants to merge 1 commit into
pothosware:masterfrom
willcode:fix/saturate-writestream
Open

willcode wants to merge 1 commit into
pothosware:masterfrom
willcode:fix/saturate-writestream

Conversation

@willcode

@willcode willcode commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

writeStream converts each part of a CF32 or CF64 sample to the device's eight bits as (int8_t)(part * 127.0), with no bound. A part past full scale therefore wraps to the opposite sign: 1.1 becomes 139, which the cast reads as -117. A pulse-shaped transmit stream normalized to unit average power peaks well above 1.0 on a few percent of its samples, so every one of those samples goes out with the wrong polarity, in clusters, and a receiver sees a burst of errors on an otherwise clean signal.

The conversion now saturates each scaled part to [-127, 127] before the cast, through one file-local helper used at the four call sites (the two parts of each sample in the CF32 and CF64 branches). std::min and std::max rather than std::clamp, since the project builds as C++11. The CS16 branch is unchanged: it shifts a sixteen-bit sample right by eight, which cannot wrap.

…ping

writebuf's HACKRF_FORMAT_FLOAT32 and HACKRF_FORMAT_FLOAT64 branches multiplied
each part by 127.0 and cast the result to int8_t with nothing bounding it. A
part whose magnitude exceeds 1.0 - a sample past the device's full scale - does
not clip in that cast: the conversion is undefined for a value outside the
destination's range and in practice takes the low eight bits, so a part just
above full scale leaves the driver near the opposite sign's extreme. One sample
of a smooth waveform is turned into a full-scale step of the wrong polarity.

Both branches now bound each scaled part to [-127, 127] before the cast, so a
part past full scale is clipped. std::min and std::max are used rather than
std::clamp because the build selects -std=c++11.

A transmit stream at unit average power, measured before this conversion, had a
part past full scale on 9.72 % of its samples, and 7.97 % of samples reached the
point of wrapping; with a local-oscillator shift ahead of the conversion, which
rotates each sample's magnitude into the converted parts, those became 31.45 %
and 29.46 %. On the air at 915 MHz, a coded QPSK link transmitting at full scale
through the unbounded conversion needed 6.31 % of its coded bits corrected at
the receiver, while the same link at 0.7 to 0.25 of full scale needed 0.0003 %
to 0.017 % - three orders of magnitude fewer - with nothing else changed. At
full scale BPSK and 8PSK acquired no lock at all and a frequency-shifted variant
decoded no frame; all three carried every frame once the stream stayed inside
full scale.

The HACKRF_FORMAT_INT16 branch is left alone: it arithmetic-shifts right by
eight, whose result is always in [-128, 127], and cannot wrap.

Signed-off-by: Jeff Long <willcode4@gmail.com>
@zuckschwerdt

zuckschwerdt commented Sep 12, 2026

Copy link
Copy Markdown
Member

None of the TX-capable SoapySDR modules do this stream shaping. It might be better if you shape the sample stream before writing it to SoapySDR. You can then tune it for best performance and also perhaps implement different shapes, e.g. a safe amplitude bound below 0 dB.

We might need to document this requirement better.

@willcode

Copy link
Copy Markdown
Contributor Author

Ok, that's what we ended up doing in the caller as a fix, and this is the only tx I've tried that takes u8. No problem if you close this one.

@zuckschwerdt

Copy link
Copy Markdown
Member

It's a good point to bring up. And surely there is still room for improvement. I'd rather keep the default path simple and efficient (platform dependent, but there might be good SIMD solutions for clamping, branchless even).

Some warning and possibly an example solution could maybe go in the writeStream() docs.

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.

2 participants