Iq tool fixes #1008

vladisslav2011 · 2021-12-01T02:52:41Z

Some small IQ tool improvements.
Things done:

Buffered file writer. Makes it possible to record 20Msps and more to modern laggy HDDs without increasing DSP delay and skipping.
Fix Loading a second raw file causes a crash #977
Add more formats (std::complex,std::complex). May close IQ recording informats other than F32 #881.
GUI improvements. Block device selection and IQ tool controls while playing back/recording IQ file. Helps to prevent accidental file corruption/crash.
To be done later:
Buffered player. May help with skipping/lag while reading IQ file from slow device (under IO load or just heavy fragmented)
Remote control commands to start/stop IQ recording. Looks easy. Should I do it before switching to another issue?

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. But this still does not fix the failure when device string changes for the same device. Changing "hackrf=0" to "hackrf=0,bias=1" still results in freeze. It looks like src destructor does not get called at time of osmosdr::source::make call despite of explicit src.reset. Strange...

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.

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...

Try to parce center frequncy from a filename and apply it to waterfall. Store current tuned frequency to a class member before switchng to IQ file and restore it after playback ends. My be fixes gqrx-sdr#977 too.

Remove CIqTool::sampleRateFromFileName as it's not used anymore.

1. New IQ recorder with heavy buffering (to overcome modern harddrive unpredictable latencies) 2. Ability to play/record not only gr_complex IQ ssamples, but integer too. int16_t (short) format is implemented and working. Implementation of int8_t (char) format is pending as my gnuradio installation lacks gnuradio::blocks::complex_to_interleaved_char.h file... 3. File names are parced to find the sample format (last field). TODO: 1. Update GUI to be able to select recording format/max time to buffer. 2. Calculate min buffer size from sampling frequency to hold 1 second of samples or less. 160 Mb per buffer (Hackrf @ 20MSps) may be too much. 3. Do not try to waste all available RAM for buffering if the destination devise is too slow to handle selected sample rate. Just fail stop recording (no exceprion thorowing in this case). 4. Fix GUI freeze while flushing recorder buffers. 5. Implement buffered file reader too

…nto iq_center_freq_fix

When file list grows long and the user tries to scroll the list to a new postion, the list continues to return it's scroll to the last selected item. That's really annoying. Return the list to last seen position after refreshing it's contents.

Add GUI to set IQ recording sample format and file writer buffer count.

libvolk-based conversion to/from integer formats. Slightly increased noise in 8-bit format with hackrf and no soch degradation when recording from RTL-SDR dongle... Connecting iq recorder after DC-blocker did not change anything. Further investigation required...

1. Switch over to old IQ recorder connection point and eliminate some common code. 2. Restore center frequency and offset when playback stops. 3. Reselect file before starting playback. Fixes incorrect sample rate when playback is started, stopped, devices switched, dsp started, stopped and then started playback of the same IQ file.

Apply correct scaling to sample values.

1. Add GUI labels for buffer usage/file size monitoring 2. Stop recording in case of failure instead of crashing

...while playing back IQ file.

..while recording is in progress. Changing IO devices and loading settings does not look like good thing to do while recording an IQ file.

Switching IO devices while playing back IQ file results in bad things. Disable it.

CMakeLists.txt

argilo · 2021-12-01T13:14:07Z

src/applications/gqrx/file_resources.cpp

@@ -32,7 +32,7 @@ std::string receiver::get_random_file(void)
    static std::string path;
    if (path.empty())
    {
-        path = "/dev/urandom";
+        path = "/dev/zero";


The name of this method should be changed to reflect its new purpose. Also, line 45 should be changed to fill the file with zeroes.

These changes should go into their own pull request, since they're unrelated to I/Q recording & playback.

Thanks.
Reverted this change in a8e9e89.

This has been done in #1001

argilo · 2021-12-01T13:16:24Z

src/interfaces/file_sink.h

+};
+
+
+#endif


Please end files with a newline.

Done in 2db4382.
I'll check other changed files too.

argilo · 2021-12-01T13:19:15Z

src/applications/gqrx/mainwindow.cpp

+    //connect(ui->freqCtrl, SIGNAL(newFrequency(qint64)), remote, SLOT(setNewFrequency(qint64)));
+    //connect(ui->freqCtrl, SIGNAL(newFrequency(qint64)), uiDockAudio, SLOT(setRxFrequency(qint64)));
+    //connect(ui->freqCtrl, SIGNAL(newFrequency(qint64)), uiDockRxOpt, SLOT(setRxFreq(qint64)));


Please delete instead of commenting out code.

Sorry for forgetting to do it.
Removed in 0c09ce8.

argilo · 2021-12-01T13:22:24Z

src/dsp/format_converter.h

+}
+
+    /*!
+     * \brief Convert stream of one format to a stream of another format.


Can't you just use GNU Radio's built-in type conversion blocks for this?

I've did it before and switched to my own implementation later because:

GNU Radio's built-in type conversion blocks are not using libvolk (for some unknown reason), so performance is not great.

My GNU Radio installation (distribution supplied, yes, I'm lazy to compile it myself) lacks interleaved_char_to_complex and complex_to_interleaved_char blocks.

Using GNU Radio blocks requires to chain a multiply_const block for scaling. Libvolk functions do both conversion and scaling at one call.

As with the File Sink, I don't think Grqx is the right place to do this. I don't want the added maintenance cost of maintaining a parallel implementation.

GNU Radio's built-in type conversion blocks are not using libvolk

I'm sure GNU Radio would appreciate a patch to fix this.

My GNU Radio installation lacks interleaved_char_to_complex and complex_to_interleaved_char blocks.

What version are you using, and what version were these blocks added in? If needed, we could use a cascade of blocks in Gqrx to support older versions of GNU Radio.

Using GNU Radio blocks requires to chain a multiply_const block for scaling.

Some of the type conversion blocks have built-in scaling. If others lack it, then I'd suggest contributing a patch to GNU Radio.

I'm sure GNU Radio would appreciate a patch to fix this.

This is already done in GNU Radio 3.9.1 but setting minimum required GNU Radio version to 3.9.1 would break compilation on many major distributions.

If needed, we could use a cascade of blocks in Gqrx to support older versions of GNU Radio.

I've written before, that I've did it this way first. But then I have found, that complex_to_interleaved_char block is not shipped by my base distribution (Ubuntu 18.04LTS). So I have to either build recent GNU Radio from source on all machines, where I use gqrx (different distributions, different architectures), or pull the block from GNU Radio source code, or replace all conversion blocks with small template class, that may be easily extended to support more formats later (it is possible as we do not have to support python binding).
Cascade blocks are bad from performance point of view: more unneeded buffers, more threads.
Sometimes all major distributions will adopt GNU Radio 3.9.1 and we'll be able to get rid of this small header.
As an option, I can convert added blocks to implement modern GNU Radio interface (inherit or just typedef would be enough?) and put the implementation into "compat" folder, that would not get compiled/included if we have found GNU Radio 3.9.1. This would reduce maintenance time later. But this would add more #ifdefs and grow CMakelists.txt...

My suggestion would be to have two different versions of the flow graph, depending on the GNU Radio version. Gqrx already does that in a few places, for example:

gqrx/src/dsp/rx_rds.cpp

Lines 85 to 103 in 75d108f

#if GNURADIO_VERSION < 0x030800

d_bpf = gr::filter::fir_filter_ccf::make(1, d_rrcf);

d_agc = gr::analog::agc_cc::make(2e-3, 0.585 * 1.25, 53 * 1.25);

d_sync = gr::digital::clock_recovery_mm_cc::make(8, 0.25 * 0.175 * 0.175, 0.5, 0.175, 0.005);

d_koin = gr::blocks::keep_one_in_n::make(sizeof(unsigned char), 2);

#else

d_rrcf_manchester = std::vector<float>(n_taps-8);

for (int n = 0; n < n_taps-8; n++) {

d_rrcf_manchester[n] = d_rrcf[n] - d_rrcf[n+8];

}

d_bpf = gr::filter::fir_filter_ccf::make(1, d_rrcf_manchester);

d_agc = gr::analog::agc_cc::make(2e-3, 0.585, 53);

d_sync = gr::digital::symbol_sync_cc::make(gr::digital::TED_ZERO_CROSSING, 16, 0.01, 1, 1, 0.1, 1, p_c);

#endif

I would suggest to use a cascade of blocks for type conversion in older versions, and the new IChar blocks (starting in 3.8?).

As a side note, supporting GNU Radio 3.7 is already becoming a burden, so I'd like to drop support for it soon. Users on old distributions can always use the AppImage releases.

Cascades of blocks are very bad from performance point of view. I think, we should implement ability to remove all unused blocks (like dc_corr, iq_swap, iq_balance optimize_cc/fix_cc inside of osmosdr_source, etc) like it is already done to dc_corr.

GNU Radio does not support conversion from unsigned types, like uint8_t (rtl-sdr command line tool default format)

Conversion from int16/uint16/int8/uint8 may be done with lookup tables, that may be faster, than libvolk functions.

Even gr-sigmf author had implemented type converters himself instead of using GNU Radio blocks...

I think, it may be better to switch to GNU Radio 3.9 class prototypes while maintaining compatibility with 3.8/3.7. This can be done by importing some 3.9 classes into our namespace and providing equivalents, constructed from 3.8/3.7 blocks or converted from 3.9 blocks in separate files (switched by cmake?). This would clean most of the main tree from ifdefs and improve code readability while allowing people to maintain compatibility with older versions.
I'd prefer to switch to GNU Radio 3.9, but it requires installing updated boost, libvolk, building GNU Radio, patching and building at least gr-osmosdr and soapysdr with all it's dependencies. Maybe, I'll setup chroots to test compatibility with 3.8/3.9.

Appimage is not an option to me, as I want to build from source, use as much system libraries, as possible, fix bugs (some of which may be specific to my systems) and add features (some of which may be useful only to me). And Appimage version would crash on some systems - thanks to soapy SDR default plugin search strategy....

argilo · 2021-12-01T13:27:13Z

src/interfaces/file_sink.h

+    * \brief Write stream to file without blocking.
+    * \ingroup file_operators_blk
+    */
+class BLOCKS_API file_sink : virtual public gr::sync_block


I think it would be best to contribute this improvement upstream to GNU Radio rather than having Gqrx maintain its own copy of the File Sink block.

Also, I'm not convinced that adding another layer of buffering on top of the standard library & operating system's buffering is a good idea, especially since it requires adding threads and incurring the risk of concurrency bugs.

I'll try to contribute this improvement upstream to GNU Radio (after some rework, adding and option switch buffering on/off, an option to allocate static buffer and maybe non-blocking file closing), but I don't think that it will be accepted fast enough... or accepted at all. But I want to use gqrx now and have perfect IQ recordings without skipping.
The standard library does not implement even double buffering and always end up with system write call, that may sometimes block for multiple seconds (on my modern SMR HDDs). Adding some application-controlled buffering always solves this problem at cost of (temporarily) increased memory usage. Writing data in large chunks improves performance and lowers CPU usage.
I have plans to implement buffered file_sourse too as playing back 10...20Msps IQ files, recorded from Hackrf, is problematic with current GNU Radio file_source.

system write call, that may sometimes block for multiple seconds

This sounds like a problem with your operating system. I'd expect write-back caching to solve this problem at the OS layer. Perhaps you need to adjust the cache size?

All machines, all operating systems (different versions of Linux) suffer from it more or less. And windows, of course, suffer from it too in a much worse way. I do not own a mac, so maybe mac lives in some different world where there is more RAM, than all running processes (chrome e.g.) could consume and where is always enough free RAM space for disk buffers/cache.

Some fine tuning may help, but, as write system call is blocking by design and can not be made non blocking (like send system call), there is absolutely no guarantee, that it will finish in predictable time. The only way to get predictable low blocking time is application level buffering with large enough buffers. Buffers should be preallocated, as sbrk system call may block or even fail and maybe marked with madvice as being used in near future.

After turning off swaps (took about 5 minutes) things became a little bit better. Much less skipping in normal conditions.

My test results (hackrf, 20Msps) are the following:

Current git master: gqrx starts dropping buffers after maximum 10 seconds of recording. The resulting file is unplayable. Constant crackling after 10 seconds.

iq_tool_fixes: gqrx finishes recording gracefully with error message after 1 minute. The resulting file does not have lost samples. Recording with 16 bits per component (32 bits per sample) proceeds till the end of disk. The resulting file is playable.

Is it better to have a short, but correct file and error message showing, that the target device is too slow or a large file full of garbage?

Is it better to have a short, but correct file and error message showing, that the target device is too slow or a large file full of garbage?

I can believe that some users might prefer the recording to continue, but with dropped samples.

In any case, I still believe that Gqrx is not the right place for this. If you believe this improvement to the File Sink is valuable, please contribute it to GNU Radio. Then Gqrx and all other applications can benefit from it. I do not want to maintain copies of GNU Radio's blocks inside Gqrx.

An alternate suggestion: try calling set_min_output_buffer on the block preceding the File Sink.

src->set_min_output_buffer was the first thing, I have tried. If it is set to ~2 seconds, it still gives choppy sound, interrupted every second. If it is set to more, than 2 seconds ... gqrx crashes with std::bad_alloc.
I've implement simple circular buffer based on std::vector (to prevent memory leaks) and got perfect results. No audio interruptions when the buffer is set to 6-7 seconds and acceptable memory usage. Seek performance is sometimes bad, as well as startup times.

I've added current file position readout to keep the slider in sync with actual file position. This prevents the slider runaway when the DSP is stopped, but file playback is not. This closes IQ Tool playback slider runaway when the DSP is stopped #1017. I do not see a way to make it work correctly using stock GNU Radio file_source...
As this PR is converted to draft (maybe just close it?), we can continue conversation here: Iq tool buffered #1016

argilo · 2021-12-01T13:34:28Z

@vladisslav2011 I'd suggest proposing changes in individual pull requests, instead of lumping many changes into one. This makes code review much easier, and avoids the situation where only some of the changes can be accepted.

...boost::lockfree::queue, etc. Remove unused function. Remove obsolete debug messages.

vladisslav2011 · 2021-12-01T17:30:42Z

Thank you for review. (It looks, like I'll have to maintain my own fork like LouDou LoL).
I can split this PR into at least 6 (+ buffered file_source later) parts:

Writing different IQ file formats using stock GNU Radio file_sink and related GUI changes (QComboBox to select sample format)
Parsing center frequency from file name
Playing different IQ file formats and parsing sample format from file name
Fixing issues with center frequency changes while playing IQ file
GUI changes to improve IQ tool usability (disabling unusable/bad behaving controls)
Buffered file writer and related GUI changes. May be made optional with cmake defines or with runtime switch to choose between GNU Radio file_sink and buffered file_sink. May be replace with some better buffering implementation when somebody makes it.
TBD Buffered file reader that would not require GUI changes (take buffer count from recording buffering configuration and calculate buffer size from sample rate to allocate 1s buffers)
It may be, I forgot something, that may be split out without breaking things and introducing more bugs. Suggest your variants, please.

argilo · 2021-12-01T18:39:12Z

I can split this PR into at least 6 (+ buffered file_source later) parts

I think that's a sensible breakdown. But as mentioned, I'm not yet convinced that 6 and 7 are necessary; even if they are they should go into GNU Radio instead.

Parsing center frequency from file name
Playing different IQ file formats and parsing sample format from file name

In the longer term, I think the path forward is SigMF (https://github.com/gnuradio/SigMF). It would be great if I/Q recording and playback in Gqrx supported SigMF. Any improvements we make now should leave room for SigMF to be added.

vladisslav2011 · 2021-12-01T22:19:17Z

SigMF is fine. I have looked at it's specs.
Implementing metadata readout in a player is easy.
Implementing writing metadata file in recorder should be easy too.
Reading/writing a container is a bit harder, but easy enough in sequental way. Seeking tar (maybe compressed tar) is harder, but still possible.

I think, it is better to fix bugs before adding features. So, I'll do 2,4,5 first, 1 and 3 then (maybe merged into 1 PR), 6,7 last. And only then I'll try to add SigMF support and look at other issues (squelch triggered wav file recording, CTSS/DCS detection and squelch triggering, multiple demodulators in slightly different, than LouDou did, way, CW skimmer, digital modes like psk32, ft8, etc., dsd as demodulator or support to send demodulated samples into stdin of subprocess and many other things, that I'll be happy to see supported by gqrx)
Take a look at
#1009
and
#1004
, please.
And #1010, please.

vladisslav2011 · 2021-12-31T01:22:03Z

Outdated. Closing.

vladisslav2011 added 19 commits November 21, 2021 03:11

Fix gqrx-sdr#962 and restore the frequency after playback

cd08e54

Try to parce center frequncy from a filename and apply it to waterfall. Store current tuned frequency to a class member before switchng to IQ file and restore it after playback ends. My be fixes gqrx-sdr#977 too.

remove unused function

9a8c78b

Remove CIqTool::sampleRateFromFileName as it's not used anymore.

Code formatting.

060e0af

Merge branch 'iq_center_freq_fix' of github.com:vladisslav2011/gqrx i…

a8ae093

…nto iq_center_freq_fix

Fix jumpy scrollbar of IQ player

217009a

When file list grows long and the user tries to scroll the list to a new postion, the list continues to return it's scroll to the last selected item. That's really annoying. Return the list to last seen position after refreshing it's contents.

Add GUI to set IQ recording params

eeb3a92

Add GUI to set IQ recording sample format and file writer buffer count.

Repair broken IQ recorder

a8885d2

Prevent overflow when recording IQ file.

3490c8d

Apply correct scaling to sample values.

Avoid possible deadlocks and memory leak

96f1f1a

iq recorder GUI improvements

96bb33b

1. Add GUI labels for buffer usage/file size monitoring 2. Stop recording in case of failure instead of crashing

IQ tool: prevent accidental change of hw freq

5ba28db

...while playing back IQ file.

IQ recorder: disable harmful buttons

20d84cc

..while recording is in progress. Changing IO devices and loading settings does not look like good thing to do while recording an IQ file.

IQ tool:disable IO device switching while playback

e07e24a

Switching IO devices while playing back IQ file results in bad things. Disable it.

argilo reviewed Dec 1, 2021

View reviewed changes

CMakeLists.txt Outdated Show resolved Hide resolved

argilo reviewed Dec 1, 2021

View reviewed changes

vladisslav2011 added 4 commits December 1, 2021 18:46

Switch to c++11 equivalents of boost::thread ...

2db4382

...boost::lockfree::queue, etc. Remove unused function. Remove obsolete debug messages.

Update CMakelists.txt to get rid of boost::thread

fce4a22

Revert unrelated changes in file_resources.cpp

a8e9e89

Remove commented out code

0c09ce8

vladisslav2011 mentioned this pull request Dec 14, 2021

Iq tool buffered #1016

Open

vladisslav2011 marked this pull request as draft December 14, 2021 11:25

vladisslav2011 closed this Dec 31, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Iq tool fixes #1008

Iq tool fixes #1008

vladisslav2011 commented Dec 1, 2021

argilo Dec 1, 2021

argilo Dec 1, 2021

vladisslav2011 Dec 1, 2021

LouDou Dec 4, 2021

argilo Dec 1, 2021

vladisslav2011 Dec 1, 2021

argilo Dec 1, 2021

vladisslav2011 Dec 1, 2021

argilo Dec 1, 2021

vladisslav2011 Dec 1, 2021

argilo Dec 1, 2021

vladisslav2011 Dec 1, 2021 •

edited

Loading

argilo Dec 1, 2021

vladisslav2011 Dec 14, 2021

argilo Dec 1, 2021

argilo Dec 1, 2021

vladisslav2011 Dec 1, 2021

argilo Dec 1, 2021

vladisslav2011 Dec 1, 2021

argilo Dec 2, 2021

argilo Dec 2, 2021

vladisslav2011 Dec 14, 2021

argilo commented Dec 1, 2021

vladisslav2011 commented Dec 1, 2021 •

edited

Loading

argilo commented Dec 1, 2021

vladisslav2011 commented Dec 1, 2021 •

edited

Loading

vladisslav2011 commented Dec 31, 2021

	#if GNURADIO_VERSION < 0x030800
	d_bpf = gr::filter::fir_filter_ccf::make(1, d_rrcf);

	d_agc = gr::analog::agc_cc::make(2e-3, 0.585 * 1.25, 53 * 1.25);

	d_sync = gr::digital::clock_recovery_mm_cc::make(8, 0.25 * 0.175 * 0.175, 0.5, 0.175, 0.005);

	d_koin = gr::blocks::keep_one_in_n::make(sizeof(unsigned char), 2);
	#else
	d_rrcf_manchester = std::vector<float>(n_taps-8);
	for (int n = 0; n < n_taps-8; n++) {
	d_rrcf_manchester[n] = d_rrcf[n] - d_rrcf[n+8];
	}
	d_bpf = gr::filter::fir_filter_ccf::make(1, d_rrcf_manchester);

	d_agc = gr::analog::agc_cc::make(2e-3, 0.585, 53);

	d_sync = gr::digital::symbol_sync_cc::make(gr::digital::TED_ZERO_CROSSING, 16, 0.01, 1, 1, 0.1, 1, p_c);
	#endif

Iq tool fixes #1008

Iq tool fixes #1008

Conversation

vladisslav2011 commented Dec 1, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vladisslav2011 Dec 1, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

argilo commented Dec 1, 2021

vladisslav2011 commented Dec 1, 2021 • edited Loading

argilo commented Dec 1, 2021

vladisslav2011 commented Dec 1, 2021 • edited Loading

vladisslav2011 commented Dec 31, 2021

vladisslav2011 Dec 1, 2021 •

edited

Loading

vladisslav2011 commented Dec 1, 2021 •

edited

Loading

vladisslav2011 commented Dec 1, 2021 •

edited

Loading