pothosware / SoapyPlutoSDR

Soapy SDR plugin for PlutoSDR
https://github.com/pothosware/SoapyPlutoSDR/wiki
GNU Lesser General Public License v2.1
58 stars 22 forks source link

MTU adaptation w.r.t sample rate #20

Closed vsonnier closed 5 years ago

vsonnier commented 5 years ago

The dynamically adapts the getStreamMTU w.r.t current sample rate. This is used by CubicSDR to determine how much samples are fetched at a time. The current value = 64K was too big for < 4MHz sample rates, so roughly make MTU smaller when sample rate is smaller as well, to prevent cracking sound.

Also restored buffer size= 64K * 2 which seems to be an ideal size, this one staying constant for all sample rates.

zuckschwerdt commented 5 years ago

Shouldn't the MTU in the driver reflect the buffer size to minimize number of readStream calls needed? If CubicSDR has requirements for readStream the best place to handle that is CubicSDR then?

I take it that the driver MTU is an indication how many samples you can expect to get from each readStream call. You may read fewer if your application demands that, but you can't expect to get more.

vsonnier commented 5 years ago

@zuckschwerdt Turns out the crackling sound was related not really of the MTU size, but apparently not being a power of 2. (try 65535 for instance.) I restored Buffer_size = MTU size = 64K which seems to be OK for all sample rates.

The PR is still usefull because it make it possible to adjust MTU with buffer size if we happen to change it in the future. If that happen, the only event on which it can be changed safely is indeed on sample rate change: CubicSDR effectively request the MTU on sample rate changes and recomputes it's internal buffers accordingly.

guruofquality commented 5 years ago

Shouldn't the MTU in the driver reflect the buffer size to minimize number of readStream calls needed? If CubicSDR has requirements for readStream the best place to handle that is CubicSDR then?

MTU is there to reflect the actual size of the buffer allocations or DMA transfer limits or whatever of the underlying hardware. This lets an application allocate buffers that match in size as hopefully a performance optimization.

Using MTU length multiple buffers on receive could mean less calls to readStream, less fragmentation of buffers, like holding remaining samples to be read out on a subsequent readStream. For writeStream it means better utilization of the buffers and less calls to writeStream either by avoiding flushing out partial buffers, or holding buffers to be entirely filled (depends on underlying hardware and driver).

That said, the application using whatever number of elements it wants should be fine, nothing should break, its just suboptimal.

Secondly, as an optimization, readStream or writeStream could produce or consume the entire submitted buffer, ie multiple MTUs worth of read or writes. Its an optimization, it tends to be a bit specific to the driver. So you are just moving the for loop from the application to the driver, but it means less calls to read/writeStream(). This may not mean much in C or C++ in terms of performance, but it could clean up an application a little, but its kind of nice for python to have less "context" switching in and out of the interpreter.

I take it that the driver MTU is an indication how many samples you can expect to get from each readStream call. You may read fewer if your application demands that, but you can't expect to get more.

Not quite, so readStream is allowed to return any number between 0 and numElems as passed in by the caller. Also there is a flag that the caller can pass in to stop from getting more than one MTU worth (if its respected by the driver) SOAPY_SDR_ONE_PACKET

vsonnier commented 5 years ago

After some bit of experiment with Cubic, which always fetch MTU at a time, it appears that it is worth adapting the buffer_size / MTU to "optimize" the number of readStream calls according to sample rate.

zuckschwerdt commented 5 years ago

I guess this this is indeed a good strategy. (maybe this needs revision if we someday figure out how the different transports (native, USB, network) behave.) The PlutoSDR is capable of up to 60Msps (only when running native I'd say). I still need to try that, but the mapping should perhaps use an "else if >8M" case.

vsonnier commented 5 years ago

Well, it covers 0-10Mhz of bandwidth, which is what the module expose now.

zuckschwerdt commented 5 years ago

Well, before I got:

# rx_sdr -f 434M -s 20M -n 20000000 -d driver=plutosdr test.cu8
[...]
Sampling at 20000000 S/s.
Tuned to 434000000 Hz.
[INFO] Using format CS16.
[INFO] Auto setting Buffer Size: 65536
Reading samples in sync mode...
[INFO] Has direct RX copy: 1
User cancel, exiting...

and with the change I get:

# rx_sdr -f 434M -s 20M -n 20000000 -d driver=plutosdr test2.cu8
[...]
[INFO] Auto setting Buffer Size: 3064522060
[INFO] Set MTU Size: 3064522060
Reading samples in sync mode...
[INFO] Has direct RX copy: 1
WARNING: sync read failed. -1
Segmentation fault
vsonnier commented 5 years ago
# rx_sdr -f 434M -s 20M -n 20000000 -d driver=plutosdr test.cu8
[...]
Sampling at 20000000 S/s.
....

Ah ah of course nothing prevent us to request something beyond the listed sample rates, here 20Msps. So let's be smart and compute a size whatever sample_rate the program asks for. This solution works quite well for me.

vsonnier commented 5 years ago

I think the present state is good enough to publish, let's add further improvements later when we got time.