From 1034c79c303fae6e6eb89e88a09e8053ab119326 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arne=20J=C3=BCnemann?= Date: Mon, 23 Oct 2023 14:48:40 +0200 Subject: [PATCH] Fix FFT Overlap for spectrum view Movement of the buffer was double what should have been intended Added warning to std::copy uses that overlaping copy is undefined behaviour --- sdrbase/dsp/spectrumvis.cpp | 10 ++++++---- sdrgui/gui/glspectrumgui.cpp | 7 +++---- sdrgui/gui/glspectrumview.cpp | 12 +++++------- 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/sdrbase/dsp/spectrumvis.cpp b/sdrbase/dsp/spectrumvis.cpp index df4a1938e..53a384b75 100644 --- a/sdrbase/dsp/spectrumvis.cpp +++ b/sdrbase/dsp/spectrumvis.cpp @@ -327,7 +327,7 @@ void SpectrumVis::feed(const ComplexVector::const_iterator& cbegin, const Comple while (begin < end) { std::size_t todo = end - begin; - std::size_t samplesNeeded = m_refillSize - m_fftBufferFill; + std::size_t samplesNeeded = m_settings.m_fftSize - m_fftBufferFill; if (todo >= samplesNeeded) { @@ -338,6 +338,7 @@ void SpectrumVis::feed(const ComplexVector::const_iterator& cbegin, const Comple processFFT(positiveOnly); // advance buffer respecting the fft overlap factor + // undefined bahavior if the memory regions overlap, valid code for 50% overlap std::copy(m_fftBuffer.begin() + m_refillSize, m_fftBuffer.end(), m_fftBuffer.begin()); // start over @@ -377,7 +378,7 @@ void SpectrumVis::feed(const SampleVector::const_iterator& cbegin, const SampleV while (begin < end) { std::size_t todo = end - begin; - std::size_t samplesNeeded = m_refillSize - m_fftBufferFill; + std::size_t samplesNeeded = m_settings.m_fftSize - m_fftBufferFill; if (todo >= samplesNeeded) { @@ -391,6 +392,7 @@ void SpectrumVis::feed(const SampleVector::const_iterator& cbegin, const SampleV processFFT(positiveOnly); // advance buffer respecting the fft overlap factor + // undefined bahavior if the memory regions overlap, valid code for 50% overlap std::copy(m_fftBuffer.begin() + m_refillSize, m_fftBuffer.end(), m_fftBuffer.begin()); // start over @@ -890,8 +892,8 @@ void SpectrumVis::applySettings(const SpectrumSettings& settings, bool force) if ((fftSize != m_settings.m_fftSize) || (settings.m_fftOverlap != m_settings.m_fftOverlap) || force) { - m_overlapSize = settings.m_fftOverlap < 0 ? 0 : - settings.m_fftOverlap < fftSize/2 ? settings.m_fftOverlap : (fftSize/2) - 1; + m_overlapSize = settings.m_fftOverlap < 0 ? 0 : + settings.m_fftOverlap < fftSize ? settings.m_fftOverlap : (fftSize/2); m_refillSize = fftSize - m_overlapSize; m_fftBufferFill = m_overlapSize; } diff --git a/sdrgui/gui/glspectrumgui.cpp b/sdrgui/gui/glspectrumgui.cpp index b5e43661e..fe29f645f 100644 --- a/sdrgui/gui/glspectrumgui.cpp +++ b/sdrgui/gui/glspectrumgui.cpp @@ -864,8 +864,7 @@ void GLSpectrumGUI::setAveragingToolitp() { QString s; int averagingIndex = m_settings.m_averagingMode == SpectrumSettings::AvgModeNone ? 0 : m_settings.m_averagingIndex; - float halfSize = m_settings.m_fftSize / 2; - float overlapFactor = (halfSize - m_settings.m_fftOverlap) / halfSize; + float overlapFactor = (m_settings.m_fftSize - m_settings.m_fftOverlap) / m_settings.m_fftSize; float averagingTime = (m_settings.m_fftSize * (SpectrumSettings::getAveragingValue(averagingIndex, m_settings.m_averagingMode) == 0 ? 1 : SpectrumSettings::getAveragingValue(averagingIndex, m_settings.m_averagingMode))) / (float) m_glSpectrum->getSampleRate(); @@ -906,10 +905,10 @@ void GLSpectrumGUI::setFFTSize(int log2FFTSize) void GLSpectrumGUI::setMaximumOverlap() { int halfSize = m_settings.m_fftSize/2; - ui->fftOverlap->setMaximum((halfSize)-1); + ui->fftOverlap->setMaximum(halfSize); int value = ui->fftOverlap->value(); ui->fftOverlap->setValue(value); - ui->fftOverlap->setToolTip(tr("FFT overlap %1 %").arg((value/(float)halfSize)*100.0f)); + ui->fftOverlap->setToolTip(tr("FFT overlap %1 %").arg((value/(float)m_settings.m_fftSize)*100.0f)); if (m_glSpectrum) { m_glSpectrum->setFFTOverlap(value); diff --git a/sdrgui/gui/glspectrumview.cpp b/sdrgui/gui/glspectrumview.cpp index c5c47c0b2..3a41fa565 100644 --- a/sdrgui/gui/glspectrumview.cpp +++ b/sdrgui/gui/glspectrumview.cpp @@ -2733,10 +2733,9 @@ void GLSpectrumView::applyChanges() if (m_sampleRate > 0) { float timeScaleDiv = ((float)m_sampleRate / (float)m_timingRate); - float halfFFTSize = m_fftSize / 2; - if (halfFFTSize > m_fftOverlap) { - timeScaleDiv *= halfFFTSize / (halfFFTSize - m_fftOverlap); + if (m_fftSize > m_fftOverlap) { + timeScaleDiv *= m_fftSize / (m_fftSize - m_fftOverlap); } if (!m_invertedWaterfall) { @@ -2827,10 +2826,9 @@ void GLSpectrumView::applyChanges() if (m_sampleRate > 0) { float timeScaleDiv = ((float)m_sampleRate / (float)m_timingRate); - float halfFFTSize = m_fftSize / 2; - if (halfFFTSize > m_fftOverlap) { - timeScaleDiv *= halfFFTSize / (halfFFTSize - m_fftOverlap); + if (m_fftSize > m_fftOverlap) { + timeScaleDiv *= m_fftSize / (m_fftSize - m_fftOverlap); } if (!m_invertedWaterfall) { @@ -4452,7 +4450,7 @@ void GLSpectrumView::timeZoom(bool zoomInElseOut) return; } - if (zoomInElseOut && (m_fftOverlap == m_fftSize/2 - 1)) { + if (zoomInElseOut && (m_fftOverlap == m_fftSize/2)) { return; }