Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ATV Demodulator: arbitrary sample rate support #594

Closed
Vort opened this issue Aug 11, 2020 · 8 comments
Closed

ATV Demodulator: arbitrary sample rate support #594

Vort opened this issue Aug 11, 2020 · 8 comments
Milestone

Comments

@Vort
Copy link
Contributor

Vort commented Aug 11, 2020

Present state of ATV Demodulator allows enabling of arbitrary sample rate support with minimal changes to the code.
Here is the hack (which is not intended for merging), which shows needed changes: Vort@d089307.
With this change, hacktv_dec2.sdriq decodes properly.

Despite ease of enabling, much work is needed to do this properly.
Lots of existing code expects sample rate to be limited.
There are ATVDemodSettings::getBaseValues code, which calculate correct amounts of points per line; m_interpolator, which converts samplerates; lots of documentation about selecting proper sample rate.
All these things should be changed or at least reconsidered.

And since it is architectural changes, they are better to be done by @f4exb.

@f4exb
Copy link
Owner

f4exb commented Aug 11, 2020

Yes it has quite a few side effects but much towards simplification. I can do that part.

@Vort
Copy link
Contributor Author

Vort commented Aug 11, 2020

Important improvement is reduced CPU usage in case of "not round" (don't know how to call it) sample rate.
For example, NTSC signal can be decoded easier (because of no forced filtering).

@f4exb
Copy link
Owner

f4exb commented Aug 11, 2020

Yes my plan is to remove the rational down-sampler and its software FIR filter. Moreover this did not yield good results for non integer ratios (so pretty useless outside of filtering). With the bandwidths usually implied the hardware filtering is enough to get rid of unwanted out of band signals. Some devices like the LimeSDR also have a FIR filter implemented in hardware. I will keep the FFT filter as an option for corner cases but this is indeed an option.

@Vort
Copy link
Contributor Author

Vort commented Aug 11, 2020

I completely agree with this (remove of decimator(?) and keeping FFT).
Also in some cases filtering can be done with Local channel sink + LocalInput.

@Vort
Copy link
Contributor Author

Vort commented Aug 11, 2020

I have found a little bug in my hackfix:
If branch should be executed in both cases (with or without HSync):

if (m_settings.m_hSync)

With disabled HSync m_hSyncShift just will be zero.

@f4exb f4exb added this to the v4.15.1 milestone Aug 19, 2020
@Vort
Copy link
Contributor Author

Vort commented Aug 21, 2020

Overall I like recent changes.
Filter adjustment now more smooth.

But I think that some corrections are needed for applySettings function:

m_settings changes
diff --git a/plugins/channelrx/demodatv/atvdemodsink.cpp b/plugins/channelrx/demodatv/atvdemodsink.cpp
index 6d031d327..0f51d1a92 100644
--- a/plugins/channelrx/demodatv/atvdemodsink.cpp
+++ b/plugins/channelrx/demodatv/atvdemodsink.cpp
@@ -515,21 +515,22 @@ void ATVDemodSink::applySettings(const ATVDemodSettings& settings, bool force)
         unsigned int samplesPerLineNom;
         ATVDemodSettings::getBaseValues(m_channelSampleRate, settings.m_nbLines * settings.m_fps, samplesPerLineNom);
         m_samplesPerLine = samplesPerLineNom;
-		m_samplesPerLineFrac = (float)m_channelSampleRate / (m_settings.m_nbLines * m_settings.m_fps) - m_samplesPerLine;
-		m_ampAverage.resize(m_samplesPerLine * m_settings.m_nbLines * 2); // AGC average in two full images
+		m_samplesPerLineFrac = (float)m_channelSampleRate / (settings.m_nbLines * settings.m_fps) - m_samplesPerLine;
+		m_ampAverage.resize(m_samplesPerLine * settings.m_nbLines * 2); // AGC average in two full images
 
         qDebug() << "ATVDemodSink::applySettings:"
                 << " m_channelSampleRate: " << m_channelSampleRate
                 << " m_samplesPerLine:" << m_samplesPerLine
                 << " m_samplesPerLineFrac:" << m_samplesPerLineFrac;
 
-        applyStandard(m_channelSampleRate, settings.m_atvStd, ATVDemodSettings::getNominalLineTime(settings.m_nbLines, settings.m_fps));
+        applyStandard(m_channelSampleRate, settings.m_atvStd,
+            ATVDemodSettings::getNominalLineTime(settings.m_nbLines, settings.m_fps));
 
         if (m_registeredTVScreen)
         {
             m_registeredTVScreen->resizeTVScreen(
                 m_samplesPerLine - m_numberSamplesPerLineSignals,
-                m_settings.m_nbLines - m_numberOfBlackLines
+                settings.m_nbLines - m_numberOfBlackLines
             );
 			m_tvScreenBuffer = m_registeredTVScreen->getBackBuffer();
         }

I have uploaded ntsc.sdriq, which decodes good enough concerning missing 29.97 FPS support, high noise and low samplerate. Good Synch value for decoding is 350 mV.

@f4exb
Copy link
Owner

f4exb commented Aug 21, 2020

Thanks for spotting this. I had noticed the issue but wasn't able to spot it although it is pretty obvious when you see it. A second pair of eyes is always useful.

@f4exb
Copy link
Owner

f4exb commented Aug 25, 2020

Implemented in v4.15.1

@f4exb f4exb closed this as completed Aug 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants