dawsonjon / PicoRX

Build a SDR SW/MW/LW Receiver with a Raspberry Pi Pico
MIT License
223 stars 30 forks source link

USB audio discussion #97

Open penfold42 opened 1 day ago

penfold42 commented 1 day ago

@mryndzionek excellent work on #96 ! :)

USB stdio: is there an underlying problem blocking this? Is it time I learnt to debug without printf? :)

Volume: should we add a second volume setting ? "Headphone\nVolume" "USB\nVolume" ?

Sample Rate: Can we resample to 22.050 or 44.1k ?

Windows has tiny dropouts: 33ms of audio followed by 1ms of silence (measured approximately with Audacity at 44.1k). I wonder if this is related to the oddball sample rate.

mryndzionek commented 1 day ago

@mryndzionek excellent work on #96 ! :)

USB stdio: is there an underlying problem blocking this? Is it time I learnt to debug without printf? :)

pico-sdk was preventing me to use USB for anything else than stdio when 'pico_enable_stdio_usb' was enabled. Might be possible to override, but I don't think it's worth the trouble. Plain UART is more reliable for debugging in my opinion, so having one spare pin for it would be ideal. SWDIO can also be used as a high-speed UART, I think.

Volume: should we add a second volume setting ? "Headphone\nVolume" "USB\nVolume" ?

I don't think USB volume is necessary. You control the volume from PC.

Sample Rate: Can we resample to 22.050 or 44.1k ?

On RP2040? I would say no. Even now, with 15625, it's almost at max speed/capabilities (+the additional memory needed).

Windows has tiny dropouts: 33ms of audio followed by 1ms of silence (measured approximately with Audacity at 44.1k). I wonder if this is related to the oddball sample rate.

Yeah, I also noticed some underruns on Linux. I also noticed some RP2040 lockups. Working on it now. Might also improve the underruns.

dawsonjon commented 1 day ago

I haven't had a chance to test this thoroughly, but it does seem to work very well and audio quality is great! Haven't had a chance to try this on windows yet though. I found that the computer volume control worked as expected, so would propose that the volume control in the menu should only effect the headphones output and that the USB output should be controlled by PC.

I have implemented cat control, but I haven't managed to get this working alongside usb audio yet. I think this should be possible by creating a composite usb device with CDC and audio endpoints. I did read somewhere that it might be possible to get usb stdio working alongside other USB stuff too, but not really sure about this. If necessary we could just use CDC API directly instead of stdio.

I did experience a software lockup when changing mode, I'm not sure of the cause though. I did notice that the tud_usb_task seems to be called from a timer interrupt, is this safe? I'm also not sure whether the tinyUSB code can be used safely across both cores (probably not a problem at the moment, but cat control more naturally sits on core 0 alongside the ui), I guess we could work around this by using the ring buffer to safely transfer data between cores, allowing the USB callbacks to always be run on the same core.

From an architectural point of view, I have an aspiration to keep rx_dsp.cpp code fairly generic so that it could be used in other receivers with minimal changes. I think it should be possible to keep it quite generic by moving the USB code up a level to rx.cpp which is intended to be hardware specific and knows about things like adcs and PWM (and now USB). I think it would make sense to add USB data to the ring buffer after each block is processed in the run function. (My inclination would be to also move the interpolation, volume control and PWM scaling to this level too, this way rx_dsp does the same processing on each block regardless of where it is destined.)

I'm really excited about these latest changes, it something I have been working towards for a while👍

penfold42 commented 1 day ago

+1 on the lockups too - forgot to mention that. :(

I suspect they're random - I've had it lockup when I picked up the boards, when just sitting there streaming lofi classic hits to the laptop and when using the UI.

mryndzionek commented 1 day ago

I did notice that the tud_usb_task seems to be called from a timer interrupt, is this safe?

Yeah, so probably not that safe, but I don't think we have other options. From my tests usb_task needs to be run very often and very "uniformly" in time. I tried "spreading" the calls throughout process_block and while we're waiting for ADC DMA to finish, but the results were very poor.

I think it should be possible to keep it quite generic by moving the USB code up a level to rx.cpp which is intended to be hardware specific and knows about things like adcs and PWM (and now USB). I think it would make sense to add USB data to the ring buffer after each block is processed in the run function. (My inclination would be to also move the interpolation, volume control and PWM scaling to this level too, this way rx_dsp does the same processing on each block regardless of where it is destined.)

Yeah, makes sense. I just wanted to make it working ASAP.

mryndzionek commented 1 day ago

Pushed #98 with some adjustments. Haven't seen a lockup with those changes.

penfold42 commented 1 day ago

Concur - 90+ mins with no lockup here.

I've had a close look at the audacity data of the dropouts now - not sure if these hint at a cause. At a sample rate of 44k1, the dropouts last 50 samples (1.1338 ms) At 15625 the dropouts last 18 samples (1.1520 ms). The audio is active for 720 samples (21 ms)

Another issue I found is at volume 9 - peaks that hit the ceiling go through it and wrap around to -ve value. Maybe just scale it back a bit when you remove the adjustable volume from USB?

dawsonjon commented 1 day ago

I spotted a *2 in the code when capturing the usb audio, I wonder if that might be the cause of the second issue?

penfold42 commented 1 day ago

I think it would be much worse if that were the cause.

I've got my TinySA blasting the radio with a -7dBm signal. This is volume 8 (interesting outlier peak) image

This is volume 9: image

dawsonjon commented 1 day ago

The AGC setpoint is at the half way point, e.g. 6dB below full scale, this was to allow headroom for overshoot when the AGC is slow to react to a change in power level. I then apply soft clipping to any signals that exceed this level and hard clip at full scale. With an additional 2*gain I would expect the AGC to scale signals to approximately full scale with a volume of 9, and overshooting signals would cause overflow. This seems to fit quite closely with your observations.

mryndzionek commented 1 day ago

I've had a close look at the audacity data of the dropouts now - not sure if these hint at a cause. At a sample rate of 44k1, the dropouts last 50 samples (1.1338 ms) At 15625 the dropouts last 18 samples (1.1520 ms). The audio is active for 720 samples (21 ms)

How do the dropouts show in audacity? In my audacity it doesn't show dropouts (I mean it coalesces the dropouts). I think that here we might want to send full buffer every time, even if it's zeros.

mryndzionek commented 1 day ago

With the latest code I've verified that occasionally the TinyUSB callback "runs out" of samples. We either produce the samples too slow, or USB stack requests them too fast. I think this in unavoidable. We can only minimize the effect.

dawsonjon commented 1 day ago

It this where the audio feedback comes in? There are some interesting comments about this here: https://github.com/hathach/tinyusb/blob/master/src/class/audio/audio_device.h#L469

mryndzionek commented 1 day ago

It this where the audio feedback comes in? There are some interesting comments about this here: https://github.com/hathach/tinyusb/blob/master/src/class/audio/audio_device.h#L469

Oh, right, so we need to measure the actual samplerate we have.

mryndzionek commented 1 day ago

The fact that we're changing CPU clock probably won't make things easier.

dawsonjon commented 1 day ago

I haven't fully understood it yet, but it looks like there are mechanisms to cope with the inevitable mismatch in sample frequency between the two ends. I think the ADC is running from the 48Mhz USB clock, so our sample rate should remain constant even when the sys_clk frequency changes.

On Mon, 30 Sep 2024, 17:30 mryndzionek, @.***> wrote:

The fact that we're changing CPU clock probably won't make things easier.

— Reply to this email directly, view it on GitHub https://github.com/dawsonjon/PicoRX/issues/97#issuecomment-2383663852, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFPFX3ODWFDQ6DXONMJGL3ZZF4BBAVCNFSM6AAAAABPC5DXRCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGOBTGY3DGOBVGI . You are receiving this because you commented.Message ID: @.***>

mryndzionek commented 1 day ago

I think the ADC is running from the 48Mhz USB clock, so our sample rate should remain constant even when the sys_clk frequency changes.

Yes, so I was thinking about our "producing" side - the ADCs. We essentially need to track the actual ADC rate and communicate it to the USB stack.

mryndzionek commented 1 day ago

With the latest code however, when I remove the periodic suspend, I'm not seeing the ring buffer being emptied out.

mryndzionek commented 23 hours ago

Made some more tests today with weather fax. With yesterday's code the whole image was chopped up. With today's samplerate adjustment to 16k I have this:

output_1727719223 so a jump every few minutes.

Then I've increased the 16k interpolation accuracy and got this: output_1727723047

MrSVCD commented 22 hours ago

Can you make the USB audio send the I/Q signal to the computer? Or is that what it is doing?

mryndzionek commented 22 hours ago

Can you make the USB audio send the I/Q signal to the computer? Or is that what it is doing?

No, it currently sends the demodulated and processed audio. Essentially the same signal as on headphones. For IQ a different USB class would be needed. Most likely some ethernet class, like RNDIS etc.

penfold42 commented 17 hours ago

+1 on the IQ over USB idea.

Could it send the IQ data as stereo audio ? I think SDR++ can use audio as a source.

Looking at rx_dsp::process_block we could just short circuit most of the for(uint16_t idx=0; idx<adc_block_size/decimation_rate; idx++) {} loop and stuff the IQ samples into the usb buffer. We then add a UI setting for

USB stream
----------
audio   IQ
mryndzionek commented 12 hours ago

+1 on the IQ over USB idea.

Could it send the IQ data as stereo audio ? I think SDR++ can use audio as a source.

Looking at rx_dsp::process_block we could just short circuit most of the for(uint16_t idx=0; idx<adc_block_size/decimation_rate; idx++) {} loop and stuff the IQ samples into the usb buffer. We then add a UI setting for

USB stream
----------
audio   IQ

Yeah, so the main issue is to have an interface easily recognizable by all of the SDR apps.

MrSVCD commented 10 hours ago

IQ over stereo audio is a common way to send that signal to sdr programs, at least in my experience. Elektors SDR shield for arduino does this as well as the first version of this type of receiver I saw. The adc is the sound card in your computer in those cases.

mryndzionek commented 10 hours ago

IQ over stereo audio is a common way to send that signal to sdr programs, at least in my experience. Elektors SDR shield for arduino does this as well as the first version of this type of receiver I saw. The adc is the sound card in your computer in those cases.

Okay, fair enough. I mostly have experience with "pure" USB SDRs.

penfold42 commented 9 hours ago

FYI When SDR++ outputs the IF of the VFO as audio. Left channel is I branch, right is Q branch.

dawsonjon commented 27 minutes ago

The weatherfax results look very impressive!

I guess that supplying IQ data would allow compatibility with sound-card based SDR applications like quisk and the like so would probably be a nice to have.

I think that outputting demodulated audio is quite important for things like fldigi, wsjtx, freedv, qsstv etc. It also opens the door for an even more minimal "headless" radio that can be connected to a smart-phone or laptop and wouldn't really need its own UI. I'm really very excited about getting this working.

I think we probably do need to think about adding CDC support either using stdio, or via the tinyUSB API. Aside from adding a cat capability, we also need some way to upload memory contents via USB. My plan was to extend the ts-480 protocol to add additional commands to read a d write channels to flash. We could then remove the memory upload option from the config menu.