squelch . . .

[charlie-foxtrot]

I've had several cases of squelch flapping open/closed during transmissions. I have noticed it on longer transmissions with strong signals that start cutting in/out towards the end. I've noticed this after the process is up for several days and it seems that restarting the process clears it up, but I can't prove that at this point.

I started looking at the squelch code to get an idea what could be happening and it is intertwined with AM/FM demodulation (my changes in #184 didn't help either). I have some observations / questions below, and based on the answers I'll take a pass at refactoring the squelch code and cleaning up what I did with the bandwidth filter.

• WAVE_RATE is either 8,000 samples per second or 16,000 samples per second if NFM support is compiled in
• WAVE_BATCH is a function of WAVE_RATE and is the number of samples to make up 1/8th of a second of output audio.
• AGC_EXTRA is the the number of samples used to fade-in / fade-out the audio for an AM signal when squelch changes. At the moment this is not a function of WAVE_RATE so equates to a different amount of time when NFM is compiled in. Should this become a function of WAVE_RATE, or does the value not matter as long as some fading happens?
• The squelch level sqlevel is either manually set or computed as 3 * fparms->agcmin
• The for loop that starts on rtl_airband.cpp:528 loops over j = AGC_EXTRA; j < WAVE_BATCH + AGC_EXTRA, that is from 100 to either 1099 or 2099 depending on if NFM is compiled in. fparms->agcmin is updated whenever j % 16 == 0. j starts at j % 16 == 4 and without NFM ends at j % 16 == 11, meaning the update of fparms->agcmin slips by 7 samples after every WAVE_BATCH (NOTE: this slip doesn't happen when NFM is enabled). I'm assuming this is an oversight and the intention is to update every 16 samples for both modes, is that correct? Is there anything special about every 16 samples, or could it be updated every sample with different weights?
• Once there is a change in squelch state (open -> closed or closed -> open) then channel->agcsq is used as a timer to prevent squelch from being changed for at least AGC_EXTRA * 2 samples (it may actually be AGC_EXTRA * 2 - 1), meaning the minimum time squelch is in a state is ~200 samples. Is there a reason for the minimum squelch time to be related to the fade-in / fade-out time? Is it just to prevent a fade-in from overlapping with a prior fade-out, in which case it only really matters for AM channels? Or is there some additional interplay between fade and squelch?
• There is a single agcsq per channel but that channel may be scanning across multiple frequencies (squelch and power level are stored per frequency). This means the squelch state can bleed across frequencies when scanning. I think all this would mean is that when squelch closes and it moves to the next frequency, that frequency may still be in the "waiting for minimum squelch time".
• channel->axcindicate is used to indicate if there is signal or not. Putting aside multi-threading, channel->axcindicate is updated on a per-sample basis in demodulate() but is read once per WAVE_BATCH samples in process_outputs(). This means whatever state the squelch is in at the last sample is applied to the entire 1/8th of a second's worth of audio. Individual audio samples are already 0'ed out when squelch is closed, so it seems better if channel->axcindicate is set to SIGNAL if squelch is open at any time in the batch, rather than just at the last sample (NOTE: I made this harder to do in Add bandwidth filter #184)
• fparms->filter_avg is used to track power level after applying a bandwidth filter (if configured), but is only updated while squelch is open, and not re-initialized. This means the next time squelch is opened (either because of power that is inside or outside of the bandwidth), fparms->filter_avg starts at a value based on the prior period of squelch. Instead fparms->filter_avg may need to be reset when squelch is closed, or first opened (this one is also my bad).
• in "text UI" mode, when a manual squelch is set the value is shown as the denominator is the squelch value, but if automatic squelch is used the denominator is 1/3rd of the current squelch value. Perhaps always showing the squelch level (either manual or automatic) would be more intuitive.

[szpajder]

Squelch has been implemented by the original author of this program. I hadn't done much to it.

Regarding all the constants in the algorithm (except WAVE_RATE) - I guess there isn't a lot of science in them, probably they've been chosen empirically, so that the result sounds good in most scenarios. Feel free to adjust them or modify the algo, but keep in mind that "works for me" is not necessarily the same thing as "works for most people". I recall that there were a few attempts to modify squelch behavior, often by tweaking the constants, but after all things got reverted to the previous state because people complained about the results being worse than before. However I am not particularly attached to the current algorithm - if you have an idea for a better one, then sure, go for it. If it's better, I'll be more than happy to merge it. If it's worse, I'll be more than happy to assign you all the issues that people file about it :-)

Small request - whatever you contribute, please do not add more C++ into the mix. The fact that the source files are named .cpp does not make this program written in this language. This is how the original author has named them and compile them, but in fact the only C++ thing there was iostream usage. C++ abstractions are completely unnecessary in low level DSP work that we're doing here. Cleaning this up is on my TODO list, however I'm currently out of bandwidth to work on this.

Now to your questions:

• whether AGC_EXTRA should be constant or depend on WAVE_RATE - I don't care, if it sounds good, then it's OK. Squelches in most SDR apps are awful, because they click. In fact, I don't event like that there are two WAVE_RATEs in the program now - however 8000 was not sufficient for NFM (the instantaneous phase rotates too quickly on most NFM voice channels, which causes garbled audio). 16000 was the minimal speed that I found NFM to perform correctly at. But I didn't want to switch the whole thing to 16000, since people who don't use NFM would pay the extra price without getting anything in return. This is why there is the NFM build option. But it has been introduced in the times of Raspberry Pi V2. I guess most people already switched to newer hardware, so maybe it's about time to ditch it and have 16000 rate only?

• the j % 16 thing was probably done to save some CPU cycles. Probably irrelevant on current hardware.

• minimum squelch time does not need to be related to the fade-in/out time. In fact, people requested to have the squelch time configurable.

• agcsq in scanning mode - well, I once thought that adding scanning support to the program would be quick and easy. Indeed, it was quick but broke plenty of things ;-)

• text UI - the best way would be to display signal levels (and configure squelch) in dBFS.

[charlie-foxtrot]

Sounds good, I'll take a crack at it.

I have a few test recordings that I'll use to compare before / after, but totally understand "works for me" != "works for all". I'll also try to keep the refactoring impact as small as possible with existing values, then tweak values (if necessary) under followup PR(s).

As for not adding more C++ to the mix I'll keep templates out and avoid iostream/stringstream. Are you OK with classes to encapsulate the data+logic, private vs public methods, objects created at startup that aren't repeatedly created/destroyed, RAII, and the like? Stuff that is on-par with the NotchFilter, LowpassFilter, and Signal classes but nothing more "fancy".

[LoganSound]

I'm also having this flappy problem, I think "squelch delay" (a feature of many scanners) that keeps squelch open a minimum of x seconds would solve a lot of the problem. Sorry if I'm a broken record but I'd love to test any fork that includes a configurable delay. I am publishing DCA tower transmissions automatically so it could help you test what "works for me" too:
https://twitter.com/UFOsofDC

[charlie-foxtrot]

@LoganSound my flappy squelch is intermittent enough that I haven't been able to capture a raw recording (not the audio but the raw samples). If your flapping is predictable such that you can record / upload somewhere when you have flapping, that would help. This is what I use to record directly from the dongle (beware, file gets large fast)

sudo rtl_sdr -d ${SERIAL} -g ${GAIN} -f ${CENTER} -p ${CORRECTION} -s ${SAMPLE_RATE} "${SERIAL}_${CENTER}_${SAMPLE_RATE}.dat"

If you want to try a crack at something, a few things that look suspicious to me are:

• The delay short-circuiting when power drops
• The gradual bumping up of the noise floor
• The magic numbers for squelch delay

Starting with https://github.com/charlie-foxtrot/RTLSDR-Airband/tree/refactor_squelch_update (same as PR #226) I would be interested if any combination of the following help in your case (NOTE I haven't compiled or tested any of these):

Option 1 - Remove the low power short circuit:

% git diff -p
diff --git a/squelch.cpp b/squelch.cpp
index 15d2fb8..8477b5b 100644
--- a/squelch.cpp
+++ b/squelch.cpp
@@ -111,7 +111,7 @@ void Squelch::process_reference_sample(const float &sample) {
                        low_power_count_++;
                        if (low_power_count_ >= low_power_abort_) {
                                debug_print("Closing at %zu: low power count %d\n", sample_count_, low_power_count_);
-                               set_state(CLOSING);
+//                             set_state(CLOSING);
                        }
                }
        }

Option 2 - Don't let noise floor creep up while squelch is open:

% git diff -p
diff --git a/squelch.cpp b/squelch.cpp
index 15d2fb8..729021c 100644
--- a/squelch.cpp
+++ b/squelch.cpp
@@ -89,7 +89,10 @@ void Squelch::process_reference_sample(const float &sample) {
 
        // auto noise floor and average power
        // TODO: what is the purpose of the NOISE_FLOOR_GROWTH?  This could account for squelch flap on marginal signal
-       noise_floor_ = noise_floor_ * DECAY_FACTOR + std::min(pre_filter_avg_, noise_floor_) * UPDATE_FACTOR + NOISE_FLOOR_GROWTH;
+       noise_floor_ = noise_floor_ * DECAY_FACTOR + std::min(pre_filter_avg_, noise_floor_) * UPDATE_FACTOR;
+       if(!is_open()) {
+               noise_floor_ += NOISE_FLOOR_GROWTH;
+       }
        pre_filter_avg_ = pre_filter_avg_ * DECAY_FACTOR + sample * UPDATE_FACTOR;
 
        // Check power against thresholds

Option 3 - Play around with open vs close delay

% git diff -p
diff --git a/squelch.cpp b/squelch.cpp
index 15d2fb8..7613e57 100644
--- a/squelch.cpp
+++ b/squelch.cpp
@@ -192,7 +192,7 @@ void Squelch::update_current_state(void) {
                }
        } else if (next_state_ == CLOSING) {
                if (current_state_ != CLOSING) {
-                       delay_ = flap_delay_;
+                       delay_ = flap_delay_ * 2;
                        current_state_ = next_state_;
                } else {
                        delay_--;

[szpajder]

Are you OK with classes to encapsulate the data+logic, private vs public methods, objects created at startup that aren't repeatedly created/destroyed, RAII, and the like? Stuff that is on-par with the NotchFilter, LowpassFilter, and Signal classes but nothing more "fancy".

Once you leave out the "fancy", you end up with things that can be easily done in plain C. Input device modules are an example - *_input_new() is a constructor, static functions are private methods, while the rest is public.

Another example with a vtable: https://github.com/szpajder/dumpvdl2/blob/master/src/output-file.c (and other output-*.c files there). There is an interface and a bunch of classes that implement it. The caller does not even know which class it's dealing with - but it doesn't care, it just calls interface methods (pointers to functions). No magic here, it's that simple.

[charlie-foxtrot]

#230 has a bunch of improvements to the squelch, including detecting and trying to reduce flapping.