Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RADE V1 SNR estimator #40

Merged
merged 18 commits into from
Jan 17, 2025
Merged

RADE V1 SNR estimator #40

merged 18 commits into from
Jan 17, 2025

Conversation

drowe67
Copy link
Owner

@drowe67 drowe67 commented Dec 23, 2024

WIP SNR estimator based on pilots. Not sure if it will work for multipath....

  1. Single 1 second time window AWGN test:

    python3 est_snr.py --single --snrdB 10
    
  2. Single 1 second time window AWGN test, test S1 expression:

    python3 est_snr.py --single --snrdB 10 --test_S1
    
  3. Single 1 second time window MPP test:

    octave:20> Fs=8000; Rs=50; Nc=30; multipath_samples('mpp', Fs, Rs, Nc, 600, 'h_nc30_mpp.f32',"",1);
    python3 est_snr.py --single --h_file h_nc30_mpp.f32
    
  4. Complete algorithm using least squares estimator for pilot phase:

    python3 est_snr.py --h_file h_nc30_mpp.f32 --single --snrdB 10 --eq_ls
    
  5. Scatter plot using 60 consecutive 1 second time windows, on MPP channel

    python3 est_snr.py --eq_ls --Nt 60 -T 1 --h_file h_nc30_mpp.f32
    

@drowe67
Copy link
Owner Author

drowe67 commented Jan 7, 2025

@tmiw - a first pass is ready for integration, the SNR estimates are available via rade_api.h. I'll come back to this PR in a few days when I'm fresh to check a few internal things.

@tmiw
Copy link
Collaborator

tmiw commented Jan 7, 2025

@tmiw - a first pass is ready for integration, the SNR estimates are available via rade_api.h. I'll come back to this PR in a few days when I'm fresh to check a few internal things.

Thanks, will work on this.

BTW I merged down from main since this branch apparently didn't have the EOO stuff before (and was causing freedv-gui to not build as a result). Hopefully that didn't interfere with anything you're doing.

@tmiw
Copy link
Collaborator

tmiw commented Jan 7, 2025

First pass done; see drowe67/freedv-gui#797. I'm having it report -10 dB if there's no sync, but I'm not sure this is correct. Suggestions would be good here.

@drowe67
Copy link
Owner Author

drowe67 commented Jan 7, 2025

First pass done; see drowe67/freedv-gui#797. I'm having it report -10 dB if there's no sync, but I'm not sure this is correct. Suggestions would be good here.

As per the rade_api.h, the SNR is only valid when in sync:

// returns the current SNR estimate (in dB) of the Rx signal ( when rade_sync()!=0 )
RADE_EXPORT int rade_snrdB_3k_est(struct rade *r);

@Tyrbiter
Copy link

Tyrbiter commented Jan 7, 2025

I'm now running drowe67/freedv-gui@93c1a01

As for the SNR display, how difficult would it be to show '--' in RADE mode without sync?

@tmiw
Copy link
Collaborator

tmiw commented Jan 7, 2025

As per the rade_api.h, the SNR is only valid when in sync:

Yep, this is for the GUI display when not in sync. Unless I pass something else up to the GUI, it's basically stuck at the last SNR reported. As one can imagine, it's probably not good to be reporting an SNR of like 5 when there's no signal.

Currently SNR retrieval is implemented as follows:

float FreeDVInterface::getSNREstimate()
{
    if (txMode_ >= FREEDV_MODE_RADE)
    {
        // Special handling for RADE
        return (getSync() ? rade_snrdB_3k_est(rade_) : -10);
    }
    else
    {
        return getCurrentRxModemStats()->snr_est;
    }
}

As for the SNR display, how difficult would it be to show '--' in RADE mode without sync?

I think that won't be a huge deal for now, but might be when it's time to get multi-RX working with RADE.

EDIT: updated the other PR to do this.

@drowe67
Copy link
Owner Author

drowe67 commented Jan 7, 2025

Yep, this is for the GUI display when not in sync. Unless I pass something else up to the GUI, it's basically stuck at the last SNR reported. As one can imagine, it's probably not good to be reporting an SNR of like 5 when there's no signal.

@tmiw @Tyrbiter - so what is the desired behavior here? I'd suggest SNR is "undefined" when not in sync, so should not be displayed (as Brian suggests). You can't measure the SNR of a signal that is not there.

I think that won't be a huge deal for now, but might be when it's time to get multi-RX working with RADE.

Please do not start any multi-rx work (or even consider it on the roadmap) without a PLT level discussion. My understanding of our end goal is one mode which means multi-rx can be rm-ed, relieving us of an unnecessary maintenance rabbit hole. If someone wants to use legacy FreeDV modes in 5 years time they can fire up freedv-gui 1.9

@tmiw
Copy link
Collaborator

tmiw commented Jan 7, 2025

@tmiw @Tyrbiter - so what is the desired behavior here? I'd suggest SNR is "undefined" when not in sync, so should not be displayed (as Brian suggests). You can't measure the SNR of a signal that is not there.

Agreed. Just updated the other PR accordingly.

Please do not start any multi-rx work (or even consider it on the roadmap) without a PLT level discussion. My understanding of our end goal is one mode which means multi-rx can be rm-ed, relieving us of an unnecessary maintenance rabbit hole. If someone wants to use legacy FreeDV modes in 5 years time they can fire up freedv-gui 1.9

The way that RADE was originally implemented means it will be pretty involved to make multi-RX work, so I'm fine putting it off for now. FWIW I think it's a good idea to enable multi-RX at some point as part of any transition plan to only having a single mode.

@drowe67
Copy link
Owner Author

drowe67 commented Jan 7, 2025

@Tyrbiter - if you are interested in the detail, checkout radae.pdf (the version in this PR) Section 9. Fig 14 & 15 give you a feel for it's accuracy.

@Tyrbiter
Copy link

Tyrbiter commented Jan 8, 2025

@Tyrbiter - if you are interested in the detail, checkout radae.pdf (the version in this PR) Section 9. Fig 14 & 15 give you a feel for it's accuracy.

Interesting, I see that in figure 15 the error in the straight lines down in the -5 to +5 dB SNR is actually quite small, and where it worsens in the MPP case at higher SNR we probably can live with the increased error as it's better to be accurate at the low SNR end.

@drowe67
Copy link
Owner Author

drowe67 commented Jan 16, 2025

@tmiw is it OK to merge this?

@tmiw
Copy link
Collaborator

tmiw commented Jan 17, 2025

@tmiw is it OK to merge this?

Yep. 👍

@drowe67 drowe67 merged commit 3edf80e into main Jan 17, 2025
1 check passed
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.

3 participants