Iq tool center freq fix #1009

vladisslav2011 · 2021-12-02T05:40:40Z

Parse the center frequency from IQ file name
Save center frequency and offset before starting IQ file playback
Restore center frequency and offset after finishing IQ file playback

argilo · 2021-12-02T13:28:26Z

Thanks for splitting this off into its own pull request. It's much easier to review this way. I'll have a look at this tonight.

argilo · 2021-12-02T13:40:51Z

src/qtgui/iq_tool.cpp

@@ -91,7 +91,8 @@ void CIqTool::on_listWidget_currentTextChanged(const QString &currentText)
    QFileInfo info(*recdir, current_file);

    // Get duration of selected recording and update label
-    sample_rate = sampleRateFromFileName(currentText);
+    //sample_rate = sampleRateFromFileName(currentText);


This can be removed.

argilo · 2021-12-02T13:41:21Z

src/qtgui/iq_tool.cpp

@@ -91,7 +91,8 @@ void CIqTool::on_listWidget_currentTextChanged(const QString &currentText)
    QFileInfo info(*recdir, current_file);

    // Get duration of selected recording and update label


This comment looks like it should be above line 96 instead.

argilo · 2021-12-02T13:41:46Z

src/qtgui/iq_tool.cpp

@@ -126,7 +127,7 @@ void CIqTool::on_playButton_clicked(bool checked)
            ui->listWidget->setEnabled(false);
            ui->recButton->setEnabled(false);
            emit startPlayback(recdir->absoluteFilePath(current_file),
-                               (float)sample_rate);
+                               (float)sample_rate,center_freq);


Suggested change

(float)sample_rate,center_freq);

(float)sample_rate, center_freq);

argilo · 2021-12-02T13:42:04Z

src/applications/gqrx/mainwindow.cpp

@@ -1658,6 +1663,7 @@ void MainWindow::stopIqPlayback()

    // restore frequency, gain, etc...
    uiDockInputCtl->readSettings(m_settings);
+    on_plotter_newDemodFreq(backupFreq,backupOffset);


Suggested change

on_plotter_newDemodFreq(backupFreq,backupOffset);

on_plotter_newDemodFreq(backupFreq, backupOffset);

argilo · 2021-12-02T13:42:36Z

src/qtgui/iq_tool.cpp


    QStringList list = filename.split('_');

    if (list.size() < 5)
-        return sample_rate;
+        return ;


Suggested change

return ;

return;

argilo · 2021-12-02T13:43:00Z

src/applications/gqrx/mainwindow.cpp


    auto sri = (int)samprate;
-    auto devstr = QString("file='%1',rate=%2,throttle=true,repeat=false")
-            .arg(filename).arg(sri);
+    auto cf  = (long long) center_freq;


Suggested change

auto cf = (long long) center_freq;

auto cf = (long long) center_freq;

argilo · 2021-12-02T13:44:20Z

So far the code looks good, I just suggested a few formatting improvements.

I'll test this out tonight.

vladisslav2011 · 2021-12-02T18:18:47Z

Thank you for a review.
Sorry for sending a dirty PR. I forgot to check code formatting and remove unused code. Again...
Should I fix code formatting by editing commits followed by push --force, or by adding a new commit?

argilo · 2021-12-02T18:38:08Z

Should I fix code formatting by editing commits followed by push --force, or by adding a new commit?

I'd suggest to force-push a revised commit. That will keep the commit history clean once this is merged.

vladisslav2011 · 2021-12-02T19:16:26Z

Done.

argilo · 2021-12-03T04:30:17Z

I noticed a bug. When switching from receive (in this example, with an RTL-SDR) to I/Q playback, the plotter frequency axis updates, but the frequency controls do not. Here's what I see when I start playback of an I/Q file with a center frequency around 390 MHz:

The frequency control still shows 929 MHz, and the indicated offset is 0 Hz. If I click to retune, then the correct frequencies are displayed.

vladisslav2011 · 2021-12-03T06:36:47Z

Thank you for reporting this bug.
Fixed in d7b7c94.

argilo · 2021-12-15T03:03:43Z

src/applications/gqrx/mainwindow.cpp

+    backupFreq = ui->freqCtrl->getFrequency();
+    backupOffset = (qint64) rx->get_filter_offset();


Is it necessary to add these variables? I expect storeSession() already saves them in the input/frequency and receiver/offset settings, the same way that the sample rate is saved.

Using the "Device dialog" or the "Save settings" button may overwrite settings during IQ playback as both are calling storeSession(). But using any of these buttons breaks playback too...
OK. Ill switch to reading center/offset from stored settings and add disabling "Save settings" button during playback to #1011.

The suggestion to use storeSession() was just for consistency with the current design. I agree that the current design is not ideal, so maybe we should think about changing it (in a future PR, perhaps).

argilo · 2021-12-15T03:24:05Z

Another minor issue I noticed is that when the I/Q file's center frequency falls outside the hardware limits of the currently selected device, then the frequency is not set correctly. (It's constrained to be within the hardware device's limits.) It looks like this happens because the hardware frequency range is not updated. This change appears to fix the problem:

diff --git a/src/applications/gqrx/mainwindow.cpp b/src/applications/gqrx/mainwindow.cpp
index 34a7d5eb6b..a78237d736 100644
--- a/src/applications/gqrx/mainwindow.cpp
+++ b/src/applications/gqrx/mainwindow.cpp
@@ -1609,6 +1609,7 @@ void MainWindow::startIqPlayback(const QString& filename, float samprate, double
     qDebug() << __func__ << ":" << devstr;
 
     rx->set_input_device(devstr.toStdString());
+    updateHWFrequencyRange(false);
 
     // sample rate
     auto actual_rate = rx->set_input_rate(samprate);

argilo · 2021-12-15T03:26:58Z

Thanks again for the pull request, and sorry for the long delay in reviewing this. I've suggested a couple further improvements. When you make those changes, could you also squash your commits into a single one? That will help keep the commit history clean.

vladisslav2011 · 2021-12-15T20:58:09Z

This change appears to fix the problem

Thanks. Fix confirmed. Included into 489af53.

When you make those changes, could you also squash your commits into a single one?

No problem. Done.
I'll rebase subsequent PRs on top of this and squash commits into one too.

argilo · 2021-12-17T21:09:44Z

Another bug:

If CIqTool::parseFileName fails to parse the filename for any reason (for instance, if the filename is foo.raw), it leaves center_freq uninitialized, which leads to undefined behaviour. Its value needs to be set, perhaps similarly to how sample_rate is set:

gqrx/src/qtgui/iq_tool.cpp

Line 48 in 49de5f3

sample_rate = 192000;

gqrx/src/applications/gqrx/mainwindow.cpp

Line 617 in 49de5f3

iq_tool->setSampleRate((qint64)actual_rate);

vladisslav2011 · 2021-12-18T20:04:30Z

Thanks for more testing.
Hmmm. If we want to support arbitrary-named files in the IQ Player, then it's up to user to supply correct sample rate, center frequency (and sample format). So, we have to provide some GUI elements.
Another issue was spotted while testing this PR: Incorrect frequency calculation. on_plotter_newDemodFreq wants current tuned frequency and offset, not center frequency and offset.
I've fixed both and force pushed a new version.

vladisslav2011 · 2021-12-18T20:06:05Z

I forgot to remove debug prints.
Pushed a clean version.

argilo · 2021-12-18T20:15:29Z

If we want to support arbitrary-named files in the IQ Player, then it's up to user to supply correct sample rate, center frequency (and sample format). So, we have to provide some GUI elements.

That might be a useful feature, but for now I think it's fine to default to the current sample rate and center frequency, and assume complex floating point samples.

vladisslav2011 · 2021-12-18T20:17:48Z

OK. I'll split this feature into separate PR. Wait some minutes.

vladisslav2011 · 2021-12-18T21:05:54Z

New GUI controls are moved to #1026

argilo · 2021-12-23T04:09:28Z

Another bug:

Open the I/Q tool
Click on a file, but don't play it yet
Close the I/Q tool
Open the I/Q tool again
Press the Play button

In this case, the center frequency will not be adjusted.

argilo · 2021-12-23T04:20:05Z

Also, in that same case the wrong file duration will be displayed if the sample rate of the file differs from the sample rate of the currently selected device. I guess both bugs are triggered by the additional code that was added to MainWindow::on_actionIqTool_triggered.

vladisslav2011 · 2021-12-23T13:16:48Z

In this case, the center frequency will not be adjusted.

Confirmed. It looks, like getting default sample rate/center frequency from current selected device does not work as expected.

I have found another bug. Wrong center frequency is set when the current offset does not fit into bandwidth of selected IQ file:

Set sampling rate to ~2 Msps and record IQ file.
Select wideband device (LimeSDR/HackRF), set sampling rate to some large value. 16 Msps for example.
Set offset to +7 MHz for example
Select IQ file, recorded at step 1 and hit play.

I'll fix both bugs and reopen this PR.
I think, we should not declare support of playing back arbitrary named IQ files until #1026 gets merged.

src/applications/gqrx/mainwindow.cpp

Try to parce center frequncy from a filename and apply it to waterfall. If parcing the file fails, default to last sample rate/center frequency. Restore current tuned frequency and offset after playback ends. My be fixes gqrx-sdr#977 too. Remove unused function Update uiDockRxOpt widgets correctly

argilo · 2021-12-31T04:02:27Z

The code looks good now, and the change appears to be working well. I'll merge once CI passes.

This was referenced Dec 2, 2021

Iq tool fixes #1008

Closed

Iq tool center freq lock #1010

Open

argilo reviewed Dec 2, 2021

View reviewed changes

vladisslav2011 force-pushed the iq_tool_center_freq_fix branch from 9dc5ecf to 796a5a8 Compare December 2, 2021 19:13

argilo reviewed Dec 15, 2021

View reviewed changes

vladisslav2011 force-pushed the iq_tool_center_freq_fix branch from c57412c to 489af53 Compare December 15, 2021 20:53

vladisslav2011 force-pushed the iq_tool_center_freq_fix branch from 489af53 to 08a99c3 Compare December 18, 2021 20:01

vladisslav2011 force-pushed the iq_tool_center_freq_fix branch from 08a99c3 to 2d24568 Compare December 18, 2021 20:04

vladisslav2011 marked this pull request as draft December 18, 2021 20:18

vladisslav2011 force-pushed the iq_tool_center_freq_fix branch from 2d24568 to 510c45a Compare December 18, 2021 20:39

vladisslav2011 force-pushed the iq_tool_center_freq_fix branch from 510c45a to 4059516 Compare December 18, 2021 20:57

vladisslav2011 marked this pull request as ready for review December 18, 2021 21:05

vladisslav2011 marked this pull request as draft December 23, 2021 13:16

vladisslav2011 force-pushed the iq_tool_center_freq_fix branch from 4059516 to 7877be3 Compare December 23, 2021 13:21

vladisslav2011 marked this pull request as ready for review December 23, 2021 13:31

argilo reviewed Dec 30, 2021

View reviewed changes

src/applications/gqrx/mainwindow.cpp Outdated Show resolved Hide resolved

vladisslav2011 force-pushed the iq_tool_center_freq_fix branch from 7877be3 to ee3f480 Compare December 31, 2021 00:02

vladisslav2011 and others added 2 commits December 30, 2021 22:34

Update news

bae1256

argilo force-pushed the iq_tool_center_freq_fix branch from ee3f480 to bae1256 Compare December 31, 2021 04:01

argilo merged commit 4eaeb16 into gqrx-sdr:master Dec 31, 2021

vladisslav2011 deleted the iq_tool_center_freq_fix branch December 31, 2021 11:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Iq tool center freq fix #1009

Iq tool center freq fix #1009

vladisslav2011 commented Dec 2, 2021

argilo commented Dec 2, 2021

argilo Dec 2, 2021

argilo Dec 2, 2021

argilo Dec 2, 2021

argilo Dec 2, 2021

argilo Dec 2, 2021

argilo Dec 2, 2021

argilo commented Dec 2, 2021

vladisslav2011 commented Dec 2, 2021

argilo commented Dec 2, 2021 •

edited

Loading

vladisslav2011 commented Dec 2, 2021

argilo commented Dec 3, 2021

vladisslav2011 commented Dec 3, 2021

argilo Dec 15, 2021 •

edited

Loading

vladisslav2011 Dec 15, 2021

argilo Dec 15, 2021

argilo commented Dec 15, 2021

argilo commented Dec 15, 2021

vladisslav2011 commented Dec 15, 2021

argilo commented Dec 17, 2021 •

edited

Loading

vladisslav2011 commented Dec 18, 2021

vladisslav2011 commented Dec 18, 2021

argilo commented Dec 18, 2021

vladisslav2011 commented Dec 18, 2021

vladisslav2011 commented Dec 18, 2021 •

edited

Loading

argilo commented Dec 23, 2021

argilo commented Dec 23, 2021

vladisslav2011 commented Dec 23, 2021

argilo commented Dec 31, 2021

		@@ -91,7 +91,8 @@ void CIqTool::on_listWidget_currentTextChanged(const QString &currentText)
		QFileInfo info(*recdir, current_file);

		// Get duration of selected recording and update label

	(float)sample_rate,center_freq);
	(float)sample_rate, center_freq);

	on_plotter_newDemodFreq(backupFreq,backupOffset);
	on_plotter_newDemodFreq(backupFreq, backupOffset);

	auto cf = (long long) center_freq;
	auto cf = (long long) center_freq;

		backupFreq = ui->freqCtrl->getFrequency();
		backupOffset = (qint64) rx->get_filter_offset();

Iq tool center freq fix #1009

Iq tool center freq fix #1009

Conversation

vladisslav2011 commented Dec 2, 2021

argilo commented Dec 2, 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

argilo commented Dec 2, 2021

vladisslav2011 commented Dec 2, 2021

argilo commented Dec 2, 2021 • edited Loading

vladisslav2011 commented Dec 2, 2021

argilo commented Dec 3, 2021

vladisslav2011 commented Dec 3, 2021

argilo Dec 15, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

argilo commented Dec 15, 2021

argilo commented Dec 15, 2021

vladisslav2011 commented Dec 15, 2021

argilo commented Dec 17, 2021 • edited Loading

vladisslav2011 commented Dec 18, 2021

vladisslav2011 commented Dec 18, 2021

argilo commented Dec 18, 2021

vladisslav2011 commented Dec 18, 2021

vladisslav2011 commented Dec 18, 2021 • edited Loading

argilo commented Dec 23, 2021

argilo commented Dec 23, 2021

vladisslav2011 commented Dec 23, 2021

argilo commented Dec 31, 2021

argilo commented Dec 2, 2021 •

edited

Loading

argilo Dec 15, 2021 •

edited

Loading

argilo commented Dec 17, 2021 •

edited

Loading

vladisslav2011 commented Dec 18, 2021 •

edited

Loading