C Port of RADE V2

[drowe67]

Claude doing his magic, backed by ctests from drowe67/radae#70.

See https://github.com/drowe67/radae_nopy/tree/dr-radev2 for V2 C Port test results.

WIP - do not merge.

The on-air waveform is not frozen and is likely to change at anytime

[drowe67]

@peterbmarks @tmiw - thinking about naming conventions for V1 & V2 code in this repo.

So Claude has come up with rade_v2_tx and radae_v2_rx for the cmd line V2 Tx and Rx. Should the current rade_tx and rade_rx be renamed rade_v1_tx and rade_v1_rx for consistency? There are probably other example in the code where we could apply similar conventions.

In drowe67/radae#70 I've renamed the ctests radae_nopy_v1_xxx and radae_nopy_v2_xxx.

[peterbmarks]

I think rade_vX_tx etc is good. (Leaves the door open to v3)
Should we keep old public methods so as not to break any uses in existing code, we could mark the old ones with deprecated to generate a warning.
I think we should also get rid of any mention of "nopy" as it's clearly already in the filename extension.

[drowe67]

Should we keep old public methods so as not to break any uses in existing code, we could mark the old ones with deprecated to generate a warning.

You mean the API? Lets take a look at that next.

[tmiw]

Using a version in the function names would be good.

Also, what about adding a function to the existing API to indicate which version to use (and then eventually default to using the RADEV2 implementations or something)? For example:

struct rade* rade = rade_open(...); // uses RADEV1, will eventually be the same as rade_open_with_version(RADE_VERSION_2, ...)
struct rade* rade2 = rade_open_with_version(RADE_VERSION_2, ...);

Then internally (for example):

struct rade* rade_open_with_version(int version, ...) {
    struct rade* result = malloc(sizeof(struct rade));

    if (version == RADE_VERSION_1) result->impl = rade_v1_get_impl();
    else if (version == RADE_VERSION_2) result->impl = rade_v2_get_impl();
    ...

    // additional initialization here
    return result;
}

int rade_tx(struct rade* rade, ...) {
    return rade->impl.tx(rade, ...);
}

Anyway, just thinking from the perspective of RADEV2 being a thing for potentially quite a while before there's a V3. If we want something quick we can just have e.g. rade_v2_tx and rade_v2_open and expect users to use those if they want V2.

EDIT: whoops, I think @drowe67 and I commented at the same time. We can look at rade_api after for sure.

[drowe67]

EDIT: whoops, I think @drowe67 and I commented at the same time. We can look at rade_api after for sure.

That's fine - what you are brainstorming is the topic I was suggesting we discuss.

TBH I'm not as worried about breaking the API at this stage - it's pretty early days and the fix (like adding a version argument to rade_open) will be pretty straight fwd. End users will only be upgrading to get V2, so will already be making changes to the way they open the library.

But I'm not mandating anything atm - lets ponder it and form a consensus.

[peterbmarks]

I'm not sure that sharing rade_open returning a pointer to the rade structure can work as that struct might change with future versions. It contains rade_tx_state and rade_rx_state and they could also be different.

Maybe we should just go with rade and rade2?

Also, it's good to build a few clients apps that use the API and collect ideas of what the API should expose.

[tmiw]

I'm not sure that sharing rade_open returning a pointer to the rade structure can work as that struct might change with future versions. It contains rade_tx_state and rade_rx_state and they could also be different.

Maybe we should just go with rade and rade2?

Also, it's good to build a few clients apps that use the API and collect ideas of what the API should expose.

There could also be a private state inside the struct that could be initialized/used in a version dependent manner. But as mentioned, if we're already going to be breaking compatibility anyway...

[drowe67]

@tmiw @peterbmarks - I'm working on the V2 API changes.

1. Claude has worked out how to do it so no existing V1 apps break, there's already a flags argument we can use in the rade_open()

   V1:                                                                                                                                                               
   rade_open(model_name, 0)                                                                                                                                          
                                                                                                                                                                    
   V2:             
   rade_open(model_name, RADE_MODE_V2) 

3. Would it be OK to change rade_snrdB_3k_est() to return a float rather than an int? It's a slight break in the API but I suspect it will link OK with existing apps, perhaps with a warning. I need float output for a V2 ctest, so otherwise I'll need to add a float getter function to the API.

[peterbmarks]

My apps use this method:
int rade_decoder_snr(rade_decoder *dec);
so I don't expect breakage at the external API level. Easy to fix if it comes up.

[drowe67]

@peterbmarks - I can't seem to find rade_decoder_snr() in rade_api.h?

[peterbmarks]

Hmm, I see what you mean. A wrapper that simply wraps rade_snrdB_3k_est(); The type conversion might generate a warning but will be fine.

[tmiw]

@tmiw @peterbmarks - I'm working on the V2 API changes.

1. Claude has worked out how to do it so no existing V1 apps break, there's already a flags argument we can use in the rade_open()

   V1:                                                                                                                                                               
   rade_open(model_name, 0)                                                                                                                                          
                                                                                                                                                                    
   V2:             
   rade_open(model_name, RADE_MODE_V2) 

3. Would it be OK to change rade_snrdB_3k_est() to return a float rather than an int? It's a slight break in the API but I suspect it will link OK with existing apps, perhaps with a warning. I need float output for a V2 ctest, so otherwise I'll need to add a float getter function to the API.

We can try it. freedv-gui builds with -Wall -Werror so if this change causes issues we'll know pretty quickly.

[drowe67]

@peterbmarks - I've changed a few file names, removing the nopy suffix. Part of a general reframe I'm suggesting from from "this doesn't use Python" to "this is a verified C port of the reference Python".

While the move away from Python was a very big deal at the time (and a major innovation - thanks again) it's backstory we probably don't need to emphasize with new users of the C library implementation of RADE.

Pls see b9f2433 and tell me if this is OK with you. Please also see the README, which has been revamped for V2.

@tmiw - pls feel free to comment on the README too.

[peterbmarks]

Yes, that's great @drowe67. I agree we have moved on from the python days and there's no need to draw attention to it except for history purposes perhaps. Quite an interesting technical story of prototyping in python and using an LLM to port to C with good success.
And fine on the changes around the to and from wav test programs. I made them at the time to help me prove that it would work.
Thanks David.

[drowe67]

@tmiw @peterbmarks - should the lpcnet_* functionality be absorbed into the C port API calls? I can't think of a use case (except for ctest-ing) where we don't use them together, and we are already building lpcnet_* in this repo. By keeping them separate we're currently asking users of the API to head scratch a fair bit to put everything together.

For example (rx side):
cat tx.iq | ./src/radae_rx --v2 | sox -t .s16 -r 16000 -c 1 - decoded.wav

[peterbmarks]

Thanks David,

With --clip 10 I look pretty good.
(.venv) marksp@Mac FreeDVNeo % python loss.py temp/featuresv2_tx.f32 temp/featuresv2_rx.f32 --clip_start 10 --plot
Loss between temp/featuresv2_tx.f32 and temp/featuresv2_rx.f32
loss: 0.083 start: 134 acq_time: 0.34 s

[tmiw]

I drop to 0.099 with --clip_start 10 (with the IC-705 as TX):

I'll play around some more here too as I just remembered that freedv-gui/freedv-backend is passing the mic signal through its own AGC and RNNoise before the RADE encoder, so that may be affecting the loss figures too.

[drowe67]

@peterbmarks - are your results "over the cable" i.e. two HF radios, attenuators, linked via coax etc?

[peterbmarks]

My results are via WAV recordings. I'm just working on integrating the new c repo but have a two radio setup ready to go.

[tmiw]

I used the rade_loss test in freedv-gui (virtual devices only, no OTC) with RNNoise and AGC disabled and loss did drop a little bit (not in front of a computer right now but IIRC something like 0.13 to 0.11 or maybe 0.10). I'll confirm with actual OTC testing but I wouldn't be surprised if loss dropped by the same magnitude there.

[peterbmarks]

I've just done a loss test really over the air. From my radio shack about 20m away running 1W to a radio in my computer room with a whip antenna. RadeV1 at this stage.

(.venv) marksp@Mac FreeDVNeo % python loss.py overtheair/features.f32 overtheair/features_decodedv1_air.f32 --clip_start 10 --plot
Loss between overtheair/features.f32 and overtheair/features_decodedv1_air.f32
loss: 0.129 start: 146 acq_time: 0.46 s

This is the macOS client now built with the new race_c repo. https://github.com/freedv/rade_c

[drowe67]

I used the rade_loss test in freedv-gui (virtual devices only, no OTC) with RNNoise and AGC disabled and loss did drop a little bit (not in front of a computer right now but IIRC something like 0.13 to 0.11 or maybe 0.10). I'll confirm with actual OTC testing but I wouldn't be surprised if loss dropped by the same magnitude there.

@tmiw - so this was a software only (no hardware in the loop) test?

How do you reconcile this high loss result with the successful freedv-backend results above with loss ~ 0.08?

[drowe67]

@tmiw - with your OTC tests - are the Tx and Rx radios physically separate (like different rooms) or on the test bench? RF tends to jump across if you are not careful. One test is to click down a few attenuating steps on the Rx side - and check the Rx signal level (e.g. s-meter/SDR or spec-an used to monitor test) drops by the same amount. If not - the signal is getting in via a parasitic path.

Liberal ferrites on the USB cables is a good idea too.

@Tyrbiter - pls advise/help @tmiw with suggestions.

I'm setting up my OTC test today.

[tmiw]

I used the rade_loss test in freedv-gui (virtual devices only, no OTC) with RNNoise and AGC disabled and loss did drop a little bit (not in front of a computer right now but IIRC something like 0.13 to 0.11 or maybe 0.10). I'll confirm with actual OTC testing but I wouldn't be surprised if loss dropped by the same magnitude there.

@tmiw - so this was a software only (no hardware in the loop) test?

How do you reconcile this high loss result with the successful freedv-backend results above with loss ~ 0.08?

Correct, no HW. Not sure yet, I'm currently investigating. Right now I don't think RNNoise/AGC are having much impact on this particular test, so I'm looking into other possibilities.

[peterbmarks]

I'm puzzled about why disabling RNNoise and AGC, which both affect microphone in audio, would affect the features -> signal -> features loss measurement.

[Tyrbiter]

Hi all

I think that @tmiw probably needs to fully debug his software only tests first, but I would definitely agree that RF testing via cables is likely to cause problems unless care is taken.

It's best to use the lowest possible transmit power and probably to put a fixed attenuator immediately on the output of the Tx radio. Be aware that bayonet connectors can be pretty leaky, although at HF this should be better than at UHF. If possible try to use threaded connectors if you have them, SMA will do but maybe N is better if available. You can also determine if there is a leakage path by moving things about and running your fingers along the cables, if any energy has got onto the coax screen then fingers will perturb it and you should see changes in signal levels on the Rx. Be careful with cable screen coverage, if they have <100% screen coverage then they will leak (this is exactly what a leaky feeder antenna system does).

Of course if there is leakage then a typical symptom will be that the RSSI will tend to not decrease past a certain point, but even then it might be that if a leakage signal is even 10-15dB below the wanted signal that it's enough to cause a partial cancellation and give a rise in BER (although of course we can't actually measure that directly).

I have been forced to use a screened chamber as one end of a test system a number of times in the past to get the isolation to adequate levels, you also need to think about the potential for off-air signals to get in to the Rx, a careful look at the noise level on the display with a dummy load connected is a good way of seeing what happens without the antenna/cable and then you can find a quiet spot to remove the radios from the immediate equation. It might also be sensible to turn off Rx preamps as that means a lower sensitivity which can be compensated for by reducing the attenuator settings which gives a further check that all things are equal.

HTH.

[peterbmarks]

Thanks @Tyrbiter I've only got about 20dB of in-line attenuation so haven't plugged radios together directly yet. I'm guessing about 100dB would be wise but, as you say, there may be leakage. So far I've tested pure audio (encode to wav saving features, decode from wave saving features) loss.py on the two feature file. I'm getting "loss: 0.084 start: 134 acq_time: 0.34 s" which is similar to David's pure command line.
Over the air from my shack which is about 20m away to a short receiving whip in the house loss is up at 0.88. There is obvious receive noise from the home solar etc.

[drowe67]

@tmiw it would be useful if we had a description of the signal processing steps (e.g. a block diagram) in the test and freedv-gui(-backend). There is a lot going on and I gather this code is a common source or bugs and rework for you.

Perhaps Claude can draw an ascii art or markdown block diagram that you could share? It would be great to show the signal processing blocks plus where features/audio signal I/O.

[tmiw]

@tmiw it would be useful if we had a description of the signal processing steps (e.g. a block diagram) in the test and freedv-gui(-backend). There is a lot going on and I gather this code is a common source or bugs and rework for you.

Perhaps Claude can draw an ascii art or markdown block diagram that you could share? It would be great to show the signal processing blocks plus where features/audio signal I/O.

Unfortunately it's not letting me share directly on OpenAI's website for some reason but I was able to get Claude to generate this:

RADE Signal Paths.pdf

I also spent quite a while working with it last night to chase down other possibilities and didn't really get anywhere. I'll see if I can modify freedv-ka9q to generate RX features and compare that with what freedv-gui generated; hopefully it should produce results similar to MinimalTxRxThreadTest.

[drowe67]

Nice write up @tmiw & Claude ... take your time exploring the problem. It sounds like a subtle issue and it's really important we have good verification results before proceeding to the Phase D V2 tests.

[drowe67]

@tmiw - just checking that you're disabling all those signal processing steps between the source audio (file) and the RADE encoder? RNNNoise/AGC/EQ etc? Each one of them is source of bugs and could "color" the input audio compared to what we trained for.

We want freedv-gui/backend to have a transfer function of 1 - it should look like a piece of wire. You should be able to do a MSE test on the buffers of audio from the audio file and at the input of the RADE encoder and find 0.0 difference.

Claude and I have added some features to loss.py, e.g. PNG generation, histogram, and --stats. Example for software only C enc/dec:

  mean:    0.081  median:  0.069
  p95:     0.169  p99:     0.271
  max:     1.212  outlier threshold:  0.207
  outliers (>0.207): 108 / 4332 frames (2.5%)

[drowe67]

So I tried a couple of OTC tests here, first with wav/brian_g8sez.wav (short and convenient), then wav/all.wav. I used radae/ota_test.sh as it automates everything for me, including time aligning the samples and loss measurements.

Results for brian_g8sez.wav:

Mode	loss (software loopback)	loss (OTC)	delta	% change
RADE V1	0.115	0.121	0.006	5.2%
RADE V2	0.086	0.087	0.001	1.2%

Results for all.wav

Mode	loss (software loopback)	loss (OTC)	delta	% change
RADE V1	0.110	0.111	0.001	0.9%
RADE V2	0.082	0.088	0.007	8.5%

My experimental config (bear is a laptop, deep my desktop):

bear -> USB -> IC7200 -> ~85dB attn -> 10m cable to another room -> ~40dB switched attn -> FT817 > Mountain Radio -> USB -> deep

On bear (Tx):

1. Create the tx.wav file:

   ./ota_test.sh wav/all.wav -x -d
   

2. Then each time you want to Tx:

   ./ota_test.sh tx.raw -t -d -o 3061 -f 7175
   

3. Just before you start the Tx, start recording using Audacity (or your chosen recording software) on the Rx machine, in this example I record to ~/Downloads/260717-otc-02.wav.

On deep (Rx):

1. This step also creates the reference feature files which get used for loss measurement, just do it once:

   ./ota_test.sh wav/all.wav -x -d
   

2. Then, using the recorded Rx file (the -l option makes it do a loss measurement):

   ./ota_test.sh -r ~/Downloads/260717-otc-02.wav -l wav/all.wav
   

   The location of the chirp is used to sync all the files up and get the right time window for the loss measurements.

Some notes:

1. ota_test.sh does the whole chirp-SSB-V1-V2 sequence (used for Phase C stored file test campaign). Overkill for these V2-only OTC tests but it's a reliable, well tested script so a good reference. To save time maybe start with a shorter file (I used brian) until you get good results.
2. ota_test.sh decoded the 3 signals, plots some graphs, and provides a report file (useful to see if anything odd is going on like loss of sync, or large sample clock or freq offsets)
3. Min power, ALC not being triggered on Tx
4. S8 on the FT-817
5. Clicking in more attn steps at rx moves s-meter, verifying the RF side.
6. I used Audacity to do the rx side recording, lets me see the waveform as it comes in, west mountain USB device selected by Ubuntu settings, adjust level to by ~ 0.5 on Audacity.
7. Brian - you sound really good on the decoded V2 wave file!

[drowe67]

Claude helped identitify a transient at the end of the brian file, example of plotting the software and OTC results together. When that was removed with --clip_end, the OTC loss dropped from 0.087 to 0.083 (lower than the software, i.e. statistical noise):

[tmiw]

I spent way too much time investigating this today and discovered the following:

• RNNoise/AGC being enabled actually reduced loss for all.wav in freedv-gui/freedv-backend, on the order of 0.01-0.015 less loss vs. having both disabled.
• There's a final LevelAdjustStep after the RADE encoder that uses different values on freedv-gui vs. freedv-backend. For the latter, it always doubles the sample values (i.e. 6 dB gain) while on freedv-gui it depends on the "TX Attenuation" slider and in theory shouldn't apply any gain either way if that slider is set to zero.

Anyway, I forced a +6 dB gain in freedv-gui to match freedv-backend (diff below):

diff --git a/src/pipeline/TxRxThread.cpp b/src/pipeline/TxRxThread.cpp
index 8fcae874..a101300b 100644
--- a/src/pipeline/TxRxThread.cpp
+++ b/src/pipeline/TxRxThread.cpp
@@ -297,7 +297,7 @@ void TxRxThread::initializePipeline_()
         
         // TX attenuation step
         auto txAttenuationStep = new LevelAdjustStep(outputSampleRate_, +[]() FREEDV_NONBLOCKING {
-            return g_txLevelScale.load(std::memory_order_acquire);
+            return 2.0f; //g_txLevelScale.load(std::memory_order_acquire);
         });
         pipeline_->appendPipelineStep(txAttenuationStep);
     }

and am now getting the same loss figures for the rade_loss ctest when run locally. I can do another OTC test with this change but not fully sure it's a good idea, especially if this is an indication of a problem with the C port.

[tmiw]

IC-705 RX, FT-817 TX. Including comparison with the features generated by ota_test.sh:

[tmiw]

TX feature-only comparison:

[drowe67]

and am now getting the same loss figures for the rade_loss ctest when run locally.

The gain mis-match work sounds solid. However you haven't presented the software only test results. The loss values on the screens shots are illegible. Please present your results in a markdown table in the format above, including % error.

RNNoise/AGC being enabled actually reduced loss for all.wav in freedv-gui/freedv-backend, on the order of 0.01-0.015 less loss vs. having both disabled.

That result is puzzling and likely a bug, or experimental error, or the processing may be shaping the features in a way the reference tests don't. We need these switched off and freedv--gui/backed configured as a straight piece of wire, transfer function of 1 for the Phase D tests. Please present these results in your table as well.

I can do another OTC test with this change but not fully sure it's a good idea, especially if this is an indication of a problem with the C port.

Any issues with RADE need to be demonstrated with the RADE command line tools.

[drowe67]

@tmiw - I pulled on the 6dB level thread. While it doesn't explain your results (+6dB should make the loss worse), I did find the AGC was never ported from the Python reference. Working on a fix in PR #4 - thanks 🙂

Separately, unrelated to this - the comments above around disabling RNN/AGC and tabulating results still stand - thanks.

[tmiw]

I updated the rade_rx_wav and rade_tx_wav helpers to emit feature files (freedv/rade_c#5) and generated some using the following:

$ ./src/rade_tx_wav -v 2 -f $(pwd)/feat_tx.f32 --v2 ../wav/all.wav tmp.wav
$ ./src/rade_rx_wav -v 2 -f $(pwd)/feat_rx.f32 --v2 tmp.wav /dev/null

I also reran the rade_loss ctest in freedv-gui with the 6 dB gain change (and RNNoise/AGC disabled) and got the following:

Feature Source 1	Feature Source 2	Loss
freedv-gui (TX)	freedv-gui (RX)	0.082
freedv-gui (TX)	Python ota_test.sh (all_features_in.f32)	0.173
freedv-gui (TX)	Python ota_test.sh (all_features_out_tx2.f32)	0.243
freedv-gui (TX)	rade_tx_wav	0.017
freedv-gui (RX)	rade_rx_wav	0.039
freedv-gui (RX)	Python ota_test.sh (all_features_out_tx2.f32)	0.227
rade_rx_wav	Python ota_test.sh (all_features_out_tx2.f32)	0.210
rade_tx_wav	Python ota_test.sh (all_features_in.f32)	0.158

Note all loss.py executions were done with --clip_start 25 to match what ota_test.sh does. Maybe once freedv/rade_c#4 merges I can redo the above comparisons?

[tmiw]

Reverted the 6 dB gain (while keeping AGC and RNNoise disabled):

Feature Source 1	Feature Source 2	Loss
freedv-gui (TX)	freedv-gui (RX)	0.108
freedv-gui (TX)	rade_tx_wav	0.017
freedv-gui (RX)	rade_rx_wav	0.067

[drowe67]

Thanks @tmiw - I think the first test freedv-gui (Tx) to freedv-gui (Rx) might be key. I feel we're zeroing in, and it might be time to come up with some concrete agreed steps that are a subset of what we have been doing so far.

Suggest we leave it there for the moment, I'll get back to you in a day or so with a draft test plan for review.

[peterbmarks]

I think we should move this discussion to a PLT group thread.

[drowe67]

Here is the revised test procedure in drowe67/radae#76