Implement audio sample rate selection

[vladisslav2011]

Close #615
Includes #1036, that closes #1030 and #1018.

[argilo]

Could you please limit this pull request to just the audio rate selection? As I've mentioned previously, combining multiple changes into a single pull request makes it difficult to review.

[vladisslav2011]

Could you please limit this pull request to just the audio rate selection?

Making audio rate selection work properly automatically closes #1030 and #1018.
I can split bug fixes into separate PR and make this PR depend on new one.

[argilo]

I think this would be clearer and more consistent with the rest of the code if make_audio_device returned the device instead of internally assigning it to audio_snk.

[vladisslav2011]

make_audio_device

Just a bad name selection.
Renamed to create_audio_device. I hope, this should not interfere with GNU Radio naming conventions.
This function was created to eliminate some common code and get rid of some #ifdefs. Making it return the device would add more #ifdefs.

[argilo]

Making it return the device would add more #ifdefs.

I suppose it would. I guess this is fine as it is.

[argilo]

Was this just for debugging?

[vladisslav2011]

Yes. I forgot to remove it.
Removed.

[argilo]

I noticed a bug: the frequency axis on the audio FFT seems to be incorrect when lower audio sample rates are used.

[vladisslav2011]

Thanks for a review.

the frequency axis on the audio FFT seems to be incorrect

Confirmed. It may be fixed by setting correct sampling rate on receiver::audio_fft. Wait some minutes, I'll check it and reopen the PR.

[argilo]

Another problem: filter widths are not restricted, and so aliasing can now occur when the audio rate is decreased. For instance, when I widen the USB filter, a tone that is outside the displayed filter skirts can be heard:

[argilo]

This is because the demod ranges defined in selectDemod are built under the assumption that the audio rate will be 48000. For instance:

gqrx/src/applications/gqrx/mainwindow.cpp

Line 1157 in 7b736c4

ui->plotter->setDemodRanges(0, 5000, 100, 40000, false);

[argilo]

It looks like this will need some careful consideration to account for all the places in the UI and DSP that are making assumptions about the audio rate.

[vladisslav2011]

For instance, when I widen the USB filter, a tone that is outside the displayed filter skirts can be heard:

That's just filter leakage. It can be heard at 48000Hz rate too. Setting the filter to 'sharp' reduces the peak and tone, setting the filter to 'soft' increases it.
I see no aliasing even when the peak is in filter range:

Audio is resampled always correctly with filtering applied before decimation. https://github.com/gqrx-sdr/gqrx/blob/master/src/dsp/resampler_xx.cpp#L52-L60

On one hand, limiting the maximum filter width (AM, SSB, CW, RAW IQ modes) to a value, that will always fit into output bandwidth is a good idea, but on the other hand, altering user settings in unexpected way may be bad.
In case of FM the bandwidth includes the deviation, so in case of WFM it can be very wide even when modulating signal bandwidth is narrow. So, we should not limit the filter width in FM modes.
I've fixed the bug with incorrect frequency axis on the audio FFT. Reopening.

[argilo]

That's just filter leakage.

Ah, I think you're right. After looking closer, I see that nbrx and wfmrx have audio resamplers late in their receive chains, which should prevent aliasing. My suggestion to limit the demod ranges was based on that incorrect assumption, so I think it's fine to leave them as they are.

I'll still need to do a bit more testing to make sure that there aren't any other bugs.

[argilo]

I see now why I was confused by the behaviour when I expanded the filter bandwidth. The filter's transition width depends on the low & high filter cutoffs, so it becomes very large when the filter is expanded:

gqrx/src/applications/gqrx/receiver.cpp

Lines 678 to 705 in 7b736c4

	receiver::status receiver::set_filter(double low, double high, filter_shape shape)
	{
	double trans_width;
	if ((low >= high) || (std::abs(high-low) < RX_FILTER_MIN_WIDTH))
	return STATUS_ERROR;
	switch (shape) {
	case FILTER_SHAPE_SOFT:
	trans_width = std::abs(high - low) * 0.5;
	break;
	case FILTER_SHAPE_SHARP:
	trans_width = std::abs(high - low) * 0.1;
	break;
	case FILTER_SHAPE_NORMAL:
	default:
	trans_width = std::abs(high - low) * 0.2;
	break;
	}
	rx->set_filter(low, high, trans_width);
	return STATUS_OK;
	}

This is surprising. I would have expected the transition width to depend only on the mode and the filter shape.

[argilo]

Another issue. While in 8 kHz mode, switching from USB to FM (stereo) leaves the audio FFT in a weird configuration where negative frequencies are displayed:

[argilo]

Maybe that happens because the requested range exceeds what is possible given the current audio sample rate. There's even a FIXME about that:

gqrx/src/applications/gqrx/mainwindow.cpp

Line 1132 in 4eaeb16

uiDockAudio->setFftRange(0,24000); /** FIXME: get audio rate from rx **/

Many other modes similarly request FFT ranges that may not be possible.

[argilo]

I guess all the ranges should be capped to a maximum of half the audio rate.

[vladisslav2011]

I guess all the ranges should be capped to a maximum of half the audio rate.

Thanks for suggestion. This works very well.

[vladisslav2011]

Rebased on top of current master.
It was quite hard. The previous "recreate the demod from scratch" approach was much, much easier.
I was hit by gnuradio/gnuradio#5436 several times during rebase, so this PR needs some testing to ensure, that it is free from regressions...

I have added 88200 Hz and 96000 Hz audio rates (mostly for "RAW IQ" mode).

[vladisslav2011]

Rebased on top of current master.
And closed by accident.
How can I reopen this PR? Pushing new commits, as github suggests, does not help...

[vladisslav2011]

OK. Pushing a new commit has solved the issue.

[argilo]

Sorry, it looks like I caused a merge conflict when I merged #1066.

I just had a quick look over this code and it's looking decent. I think I'll try to including this in the next feature release (2.16) since it's a new feature.

[vladisslav2011]

I caused a merge conflict

Resolved.

[sultanqasim]

Aside from my nitpick regarding the double precision argument to the set_rate function in fm_deemph.h, this patch is working very well for me and I have not noticed any regressions in my testing. I would support this being merged once this the warning causing double precision nitpick is fixed.