pothosware / SoapyNetSDR

SoapySDR Support Module for NetSDR protocol
https://github.com/pothosware/SoapyNetSDR/wiki
MIT License
3 stars 2 forks source link

NetSDR rate-test anomaly #7

Open gvanem opened 3 years ago

gvanem commented 3 years ago

Using this command to do a rate-test of my Afedri-SDR LAN receiver:

  SoapySDRUtil.exe --args=driver=netsdr --rate=2E6 --direction=RX

gives:

######################################################
##     Soapy SDR -- the SDR abstraction library     ##
######################################################

Found hostAddr 10.0.0.50, port 50000.
Using Afedri AFEDRI-SDR SN 31351F39 BOOT 1 FW 106 HW 5
getNativeStreamFormat
Stream format: CF32
Num channels: 1
Element size: 8 bytes
Begin RX rate test at 2 Msps
getStreamMTU
Starting stream loop, press Ctrl+C to exit...
activateStream + start 00000002 0 0 0
0 Msps  0 MBps
-deactivateStream
closeStream

The report on the transfer rate is always 0 MBps and no report on dropped packets or anything.

Looking at the code in NetSDR_Streaming.cpp, the call to ::select() looks funny:

int ret = ::select((int)(_udp+1), NULL, &fds, NULL, &tv);

Should it not test for a readable socket?

Tracing the rate-test, I off-course see a huge amount of calls to the above ::select(). But the ::recvfrom() never gets called!?

I'm on Win-10 using a x86 version of Soapy.

zuckschwerdt commented 3 years ago

Looking at the commit a60291a it was indeed meant to test the readfds, but somehow arguments got mixed up?

gvanem commented 3 years ago

... but somehow arguments got mixed up?

Correcting that, rebuilding and running again, there is still no reported rate. And no calls to recvfrom().

gvanem commented 3 years ago

By just changing this function into:

size_t SoapyNetSDR::getStreamMTU (SoapySDR::Stream *stream) const
{
  return 1;
}

the rate-test works:

getStreamMTU
Starting stream loop, press Ctrl+C to exit...
activateStream + start 00000002 0 0 0
0.589214 Msps   4.71371 MBps
0.598596 Msps   4.78877 MBps
0.60989 Msps    4.87912 MBps
0.614733 Msps   4.91787 MBps
0.617771 Msps   4.94217 MBps
0.618627 Msps   4.94902 MBps
\deactivateStream
closeStream

But what does it really do?

guruofquality commented 3 years ago

getStreamMTU is a hint for applications to be efficient. The rate tester app happens to use it

    /*!
     * Get the stream's maximum transmission unit (MTU) in number of elements.
     * The MTU specifies the maximum payload transfer in a stream operation.
     * This value can be used as a stream buffer allocation size that can
     * best optimize throughput given the underlying stream implementation.
     *
     * \param stream the opaque pointer to a stream handle
     * \return the MTU in number of stream elements (never zero)
     */

This MTU value of 0 should be replaced with however many samples per network packet are the default.

gvanem commented 3 years ago

Something like return (((1500 - 20 - 8) / _datasize) + 1);? With this I still there are a ridiculous amount of calls to select() for each recvfrom() (around 34).

And I see my Afedri never sends UDP-packets bigger than 1444 on this LAN.

zuckschwerdt commented 3 years ago

That 1440+4 seems to be the magic number. The buffer in processUPD() looks wrong (but too big isn't a problem), it should be 4 (header) + 720 (elements) * 2 (element size). Just a guess though. MTU would be 720 then. Not too sure how big the elements are though.

Did you see the many select() calls with the fix from above or without? Select on the UDP writefds would surely always return 1.

gvanem commented 3 years ago

Did you see the many select() calls with the fix from above or without?

With. A return (1); gives me 230 calls to select() for each recvfrom().

zuckschwerdt commented 3 years ago

Sorry, I meant the fix to select() on readfds, instead of writefds. That should be fixed too. But yes, reading with an (element) MTU of 1 will touch the buffer 720 times, only then do another recv(), likely.

zuckschwerdt commented 3 years ago

I'd say we shouldn't even select() if (numElems <= datacount). It's a rare special case, but we should not impose a timeout on too small reads. Do you want to PR those three changes, or should I just apply those?

zuckschwerdt commented 3 years ago

Ah, selecting on readfds we will return with SOAPY_SDR_TIMEOUT for small reads ;) It just works now because select on writefds always comes back with 1. Basically the select is now just a no-op. Needs to be fixed but also skipped if there is still enough data in the datasave buffer.

gvanem commented 3 years ago

, or should I just apply those?

👍

guruofquality commented 3 years ago

Few thoughts

zuckschwerdt commented 3 years ago

I'm on it but it might take a day or two. There are other piece perhaps wrong or at least obfuscated, e.g. float datasave[256 * 2 * sizeof(float)];

zuckschwerdt commented 3 years ago

Gisle, did you have a chance to test #8 ? Relevant changes are in ed31a70

gvanem commented 3 years ago

Looks okay with SoapySdrUtil --args=driver=netsdr --rate=2E6 --direction=RX. Number of select()== recvfrom() now.

gvanem commented 3 years ago

Update: tested this Soapy module with GQrx and my Afedri-radio. Previously it did not work at all. Now it does, but there is a lot of Audio Underrun (aU) from GnuRadio: GQrx-audio-underrun

But not sure who's fault that is.

zuckschwerdt commented 3 years ago

The module currently respects the timeoutUs parameter and will block until a packet is available or timeoutUs is exceeded. It then blocks again until the requested amount of data is acquired. We'd need to see what condition Audio Underrun in Gqrx exactly means to figure this out.

gvanem commented 3 years ago

We'd need to see what condition Audio Underrun in Gqrx exactly means to figure this out.

Maybe it's an issue with gr-audio + PortAudio in GnuRadio since that module is a bit in limbo-land at the moment. Seems my issue was solved. Hence closing. Many thanks for the effort.

gvanem commented 3 years ago

Still some issue with a:

just hanging at the end forever. A opposed to a;

which returns after doing it's job, whatever that is. Shouldn't there be some SoapyNetSDR::stop() somewhere?

gvanem commented 3 years ago

I think I've found the issue. NetSDR_Settings.cpp calls getFrequencyRange2() expecting my Afedri to return something sensible for this command. It does not:

  * 1.179542 sec: f:/gv/dx-radio/SoapySDR/SoapyNetSDR/NetSDR_Settings.cpp(315) (SoapyNetSDR::transaction+588):
    send (856, 0x010FF540, 5, 0) --> 5 bytes.
    0000: 05 40 20 00 00                                   .@ ..

  * 1.181656 sec: f:/gv/dx-radio/SoapySDR/SoapyNetSDR/NetSDR_Settings.cpp(321) (SoapyNetSDR::transaction+610):
    recv (856, 0x010FED20, 2, 0) --> 2 bytes.   << !! an empty reply
    0000: 02 00                                            

Thus the last recv() in transaction() is waiting for a payload that will never come.

To fix it, I simply patched like so:

--- a/NetSDR_Settings.cpp 2021-08-11 12:45:17
+++ b/NetSDR_Settings.cpp 2021-08-27 16:05:18
@@ -314,10 +314,11 @@
       return false;

     length -= 2; /* subtract header size */
-
-    nbytes = recv(_tcp, (char *)&data[2], length, 0); /* read payload */
-    if ( nbytes != length )
-      return false;
+    if (length > 0)  {
+       nbytes = recv(_tcp, (char *)&data[2], length, 0); /* read payload */
+       if ( nbytes != length )
+         return false;
+    }
     rx_bytes = 2 + length; /* header + payload */
   }
zuckschwerdt commented 3 years ago

Very nice, yes, recv with len 0 is not the right thing to do :) PR if you like, otherwise I'll need fake the author so you get credit ;)

gvanem commented 3 years ago

And BTW, the output of SoapySDRUtil.exe --probe="driver=netsdr" is now:

Probe device driver=netsdr
Using
getStreamFormats
getNativeStreamFormat

----------------------------------------------------
-- Device identification
----------------------------------------------------
  driver=netSDR
  hardware=NetSDR

----------------------------------------------------
-- Peripheral summary
----------------------------------------------------
  Channels: 1 Rx, 0 Tx
  Timestamps: NO

----------------------------------------------------
-- RX Channel 0
----------------------------------------------------
  Full-duplex: NO
  Supports AGC: NO
  Stream formats: CF32
  Native format: CF32 [full-scale=1]
  Antennas: RX
  Full gain range: [-30, 0] dB
    ATT gain range: [-30, 0] dB
  Full freq range:  MHz
    RF freq range:  MHz
  Sample rates: 0.02, 0.032, 0.04, 0.05, 0.08, ..., 0.625, 0.8, 1, 1.25, 2 MSps
  Filter bandwidths: 34 MHz

Does this look OK?

And the initial SoapySDRUtil.exe --args=driver=netsdr --rate=2E6 --direction=RX command works. Although the Netsdr Lost X packets message is a bit worrying for rates > 1 MBS/s. And the first lost-message is always positive for any rate.

gvanem commented 3 years ago

PR if you like,

I'll try.