Dsp freeze fix #1004

vladisslav2011 · 2021-11-21T01:39:06Z

Fix #1003.
Switching input devices works even on the fly without DSP freeze/open errors.
Changing device options like direct sampling/bias tee works on the fly too.

Workaround gnuradio blocks state corruption after device change by recreating dc_corr (fixes white waterfall and no sound) and resetting the demod (fixes corrupted sound). Pure hack. It is better to find a way to prevent corruption than fix it this way...

boushley

👋 I'm new here, but this looks like a clean PR after a couple formatting changes.

boushley · 2021-11-24T03:43:43Z

src/applications/gqrx/receiver.cpp

+        qDebug() << "  new: " << device.c_str();
+#endif
+        return;
+    }else{


Looks like the rest of the code puts parentheses on their own lines. Should we update this to match?

Fixed in 060e0af.

boushley · 2021-11-24T03:44:34Z

src/applications/gqrx/receiver.cpp

@@ -194,7 +205,7 @@ void receiver::set_input_device(const std::string device)
    {
        tb->stop();
        tb->wait();
-    }
+    };


Does the semicolon here have a purpose? C++ is not my strong suit but I don't think this semicolon is adding any value, right?

Fixed in 060e0af.

boushley · 2021-11-24T03:45:29Z

src/applications/gqrx/receiver.cpp

@@ -206,7 +217,13 @@ void receiver::set_input_device(const std::string device)
        tb->disconnect(src, 0, iq_swap, 0);
    }

-    src.reset();
+    //temporarily connect dummy source to ensure that previous device is closed


Thanks for the helpful comment explaining the purpose of the dummy device connect 👍

Workaround gnuradio blocks state corruption after device change by recreating dc_corr (fixes white waterfall and no sound) and resetting the demod (fixes corrupted sound). It looks like sometimes demodulators are not happy with bad samles too. We should recreate the graph, temporarily switching out current demodulator to prevent bad behavior. Now in case of demodulator internal state corruption, it can be resolved by switching from nbrx to wbfm and back from GUI or vise versa.

Workaround gnuradio blocks state corruption after device change by recreating dc_corr (fixes white waterfall and no sound) and resetting the demod (fixes corrupted sound). Pure hack. It is better to find a way to prevent corruption than fix it this way...

argilo · 2021-12-15T04:15:51Z

The underlying bug leading to #1003 (and #757) was already fixed in GNU Radio 3.8.1+: gnuradio/gnuradio#3184

As a result, I think any changes to receiver::set_input_device should be confined to an #if GNURADIO_VERSION < 0x030801 block.

argilo · 2021-12-15T04:41:23Z

Or perhaps #if GNURADIO_VERSION < 0x030802, since Ubuntu 20.04 shipped with 3.8.1.0-rc1, a release-candidate version which did not include the fix.

Workaround gnuradio blocks state corruption after device change by recreating dc_corr (fixes white waterfall and no sound) and resetting the demod (fixes corrupted sound). It looks like sometimes demodulators are not happy with bad samles too. We should recreate the graph, temporarily switching out current demodulator to prevent bad behavior. Now in case of demodulator internal state corruption, it can be resolved by switching from nbrx to wbfm and back from GUI or vise versa.

Partially reverting 8b4c2ef and fixing DSP restart failure in MainWindow::on_actionIoConfig_triggered allows to select different device/change sample rate/bandwidth/or other device options without DSP freeze and device open failure. Work around GNU Radio bug #3184 Resetting shared_ptr is not enough to close the device. We have to switch to some other dummy osmosdr source, start the top_block, then stop it and disconnect dummy source to really close the device. This may fix some hackrf glitches (at least switching bias tee on the fly) too. Fix white waterfall and no sound after playing back bad IQ file. Workaround gnuradio blocks state corruption after device change by recreating dc_corr (fixes white waterfall and no sound) and resetting the demod (fixes corrupted sound). It looks like sometimes demodulators are not happy with bad samles too. We should recreate the graph, temporarily switching out current demodulator to prevent bad behavior.

vladisslav2011 · 2021-12-15T22:13:30Z

Thanks for suggestion.
I've added GNU Radio version check and provided better fix for white waterfall/no sound after playing back bad IQ file.

Partially reverting 8b4c2ef and fixing DSP restart failure in MainWindow::on_actionIoConfig_triggered allows to select different device/change sample rate/bandwidth/or other device options without DSP freeze and device open failure. Work around GNU Radio bug #3184 Resetting shared_ptr is not enough to close the device. We have to switch to some other dummy osmosdr source, start the top_block, then stop it and disconnect dummy source to really close the device. This may fix some hackrf glitches (at least switching bias tee on the fly) too. Fix white waterfall and no sound after playing back bad IQ file. Workaround gnuradio blocks state corruption after device change by recreating dc_corr (fixes white waterfall and no sound) and resetting the demod (fixes corrupted sound). It looks like sometimes demodulators are not happy with bad samles too. We should recreate the graph, temporarily switching out current demodulator to prevent bad behavior.

argilo · 2021-12-15T22:32:38Z

and provided better fix for white waterfall/no sound after playing back bad IQ file

This change seems unrelated to the first one. Could you please split it off into its own pull request so that it can be reviewed separately?

argilo · 2021-12-15T22:38:09Z

src/applications/gqrx/receiver.cpp

    std::string error = "";

    if (device.empty())
        return;
+    if (input_devstr.compare(device) == 0)
+    {
+#ifndef QT_NO_DEBUG_OUTPUT


Why is #ifndef QT_NO_DEBUG_OUTPUT needed? Doesn't setting that disable qDebug() automatically?

I don't know where did I took this from. No useful information in case, the input device did not change. So, reverted to previous state and added a check for changed device string as it was done before, but removed later for some unknown purpose.
Why should we always reopen a device even if it did not change? If we have to work around some buggy device, then it is better to check for it's specific device string and reopen if it matches.
Moved this to #1021.

Partially reverting 8b4c2ef and fixing DSP restart failure in MainWindow::on_actionIoConfig_triggered allows to select different device/change sample rate/bandwidth/or other device options without DSP freeze and device open failure. Work around GNU Radio bug #3184 Resetting shared_ptr is not enough to close the device. We have to switch to some other dummy osmosdr source, start the top_block, then stop it and disconnect dummy source to really close the device. This may fix some hackrf glitches (at least switching bias tee on the fly) too.

vladisslav2011 · 2021-12-15T23:48:30Z

Could you please split it off into its own pull request so that it can be reviewed separately?

OK. I've opened 2 new issues #1018 and #1019, and corresponding pull requests #1020 and #1021

argilo · 2021-12-16T04:20:33Z

Thanks for splitting those out!

At a glance the change looks good. I'll merge once I've verified that things work well with an older GNU Radio version.

Partially reverting 8b4c2ef and fixing DSP restart failure in MainWindow::on_actionIoConfig_triggered allows to select different device/change sample rate/bandwidth/or other device options without DSP freeze and device open failure. Work around GNU Radio bug #3184 Resetting shared_ptr is not enough to close the device. We have to switch to some other dummy osmosdr source, start the top_block, then stop it and disconnect dummy source to really close the device. This may fix some hackrf glitches (at least switching bias tee on the fly) too.

argilo

Tested on Ubuntu 18.04, and this works well. I'll merge once CI passes.

boushley reviewed Nov 24, 2021

View reviewed changes

vladisslav2011 mentioned this pull request Dec 2, 2021

Iq tool fixes #1008

Closed

vladisslav2011 force-pushed the dsp_freeze_fix branch from 060e0af to 9ab8b29 Compare December 2, 2021 20:01

vladisslav2011 force-pushed the dsp_freeze_fix branch from 804f5b8 to 4e61d1b Compare December 15, 2021 22:11

vladisslav2011 force-pushed the dsp_freeze_fix branch from 4e61d1b to 1a9c247 Compare December 15, 2021 22:16

argilo reviewed Dec 15, 2021

View reviewed changes

vladisslav2011 force-pushed the dsp_freeze_fix branch from 1a9c247 to 368a7f1 Compare December 15, 2021 23:27

vladisslav2011 and others added 2 commits December 16, 2021 12:16

Update news

0c21c59

argilo force-pushed the dsp_freeze_fix branch from 368a7f1 to 0c21c59 Compare December 16, 2021 17:26

argilo approved these changes Dec 16, 2021

View reviewed changes

argilo merged commit c061525 into gqrx-sdr:master Dec 16, 2021

vladisslav2011 deleted the dsp_freeze_fix branch December 16, 2021 19:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dsp freeze fix #1004

Dsp freeze fix #1004

vladisslav2011 commented Nov 21, 2021

boushley left a comment

boushley Nov 24, 2021

vladisslav2011 Nov 26, 2021

boushley Nov 24, 2021

vladisslav2011 Nov 26, 2021

boushley Nov 24, 2021

argilo commented Dec 15, 2021 •

edited

Loading

argilo commented Dec 15, 2021

vladisslav2011 commented Dec 15, 2021

argilo commented Dec 15, 2021

argilo Dec 15, 2021

vladisslav2011 Dec 15, 2021

vladisslav2011 commented Dec 15, 2021

argilo commented Dec 16, 2021

argilo left a comment

Dsp freeze fix #1004

Dsp freeze fix #1004

Conversation

vladisslav2011 commented Nov 21, 2021

boushley left a comment

Choose a reason for hiding this comment

boushley Nov 24, 2021

Choose a reason for hiding this comment

vladisslav2011 Nov 26, 2021

Choose a reason for hiding this comment

boushley Nov 24, 2021

Choose a reason for hiding this comment

vladisslav2011 Nov 26, 2021

Choose a reason for hiding this comment

boushley Nov 24, 2021

Choose a reason for hiding this comment

argilo commented Dec 15, 2021 • edited Loading

argilo commented Dec 15, 2021

vladisslav2011 commented Dec 15, 2021

argilo commented Dec 15, 2021

argilo Dec 15, 2021

Choose a reason for hiding this comment

vladisslav2011 Dec 15, 2021

Choose a reason for hiding this comment

vladisslav2011 commented Dec 15, 2021

argilo commented Dec 16, 2021

argilo left a comment

Choose a reason for hiding this comment

argilo commented Dec 15, 2021 •

edited

Loading