pothosware / SoapySDRPlay2

Soapy SDR plugin for SDRPlay
MIT License
50 stars 11 forks source link

Early return from readStream() causes problems when running over SoapyRemote and gr-osmosdr #68

Closed szpajder closed 4 years ago

szpajder commented 4 years ago

SoapySDRPlay::readStream() function performs an early check on whether the stream is currently active (https://github.com/pothosware/SoapySDRPlay/blob/master/Streaming.cpp#L288). If it's not, it returns 0. This is problematic when the device is accessed remotely using SoapyRemote. Here is what happens:

Now because readStream() is called in a loop and returns early without any waiting, the loop runs at full CPU speed, which in turn saturates the CPU core on the server and produces a storm of 24-byte UDP packets. This is especially nasty on slow network links (read: WiFi), because the client app usually calls SOAPY_REMOTE_SETUP_STREAM first and then continues with configuring the device before issuing SOAPY_REMOTE_ACTIVATE_STREAM call. Discovering device capabilities and setting it up requires 30-40 RPC calls to be sent over TCP (which translates to 30-40 network round trips). These calls get delayed due to increased jitter and retransmissions on the saturated link. As a result, GQRX startup takes about 4-5 seconds at best. In the worst case (eg. when the wireless network is being utilized by others) SoapyRemote setup times out and the program fails to start. Before the user presses the "start DSP" button, the stream is still not active, which causes continuous 100% CPU usage on the server. When in turn the stream is active, CPU usage is only about 20% (on an Odroid XU4 SBC). Stopping the stream causes the CPU usage to jump back to 100% of course.

So far I was using GQRX with GNUradio 3.7 and SoapyRemote. Despite abovementioned problems, this setup worked quite reliably, so I didn't bother to investigate. However after a recent upgrade to GNUradio 3.8 it failed completely - after "Start DSP" was pressed, the data was not being streamed - just empty waterfall and silence. It seems that the block scheduling logic has changed slightly in GNUradio 3.8 - ie. whenever a source block returns 0 samples, its status is set to BLKD_IN (https://github.com/gnuradio/gnuradio/blob/master/gnuradio-runtime/lib/tpb_thread_body.cc#L124) which in turn causes its next execution to get delayed by 250 milliseconds (https://github.com/gnuradio/gnuradio/blob/master/gnuradio-runtime/lib/tpb_thread_body.cc#L142). The initial flood of empty UDP packets causes the readStream() call in gr-osmosdr block to return 0 samples on each call (https://github.com/osmocom/gr-osmosdr/blob/master/lib/soapy/soapy_source_c.cc#L99) which triggers the abovementioned timer every time. This slows down the recipient considerably and causes a massive backlog on the network socket, from which the client app is unable to recover. Note the high Recv-Q value in the netstat output, meaning the app is not reading the data from the socket in a timely manner:

Proto Recv-Q   Send-Q Local Address           Foreign Address         State
udp   36159744      0 192.168.1.1:51480       192.168.1.1:41423       ESTABLISHED

After a few seconds the server stops the stream due to packets not being acknowledged by the client and the whole thing fails for good.

Long story short, I commented out this section in SoapySDRPlay::readStream():

    if (!streamActive)
    {
        return 0;
    }

It works nicely now and GQRX starts up and initializes the remote device in about one second. No empty UDP packets have been observed on the network.

The above change causes SoapySDRPlay::acquireReadBuffer() to be called on every readStream() call. Before the stream is activated by the client, it just waits briefly on _buf_cond and returns SOAPY_SDR_TIMEOUT. As this is an error condition, it gets propagated to the client, however the wait in acquireReadBuffer() causes the TX thread loop to be slowed down considerably, so the amount of UDP packets produced is much lower compared to what it is when readStream() returns 0. I have not tested this change with a device connected locally.

Before I have narrowed the problem down to the above code section, I also tried to prevent the flood by changing the logic in SoapyStreamEndpoint::releaseSend(), so that it does not send packets that contain only header. It seemed to work, however I have tested it just briefly and I'm not sure if it wouldn't cause any side effects.

guruofquality commented 4 years ago

I agree with that change, most the readStream calls tend to block on something with a timeout, even if the stream isnt activated. And that also seems to be just fine to do in sdrplay once that if statement is removed. And I think its correct behaviour. Can you make this change a pull request?