f4exb / sdrangel

SDR Rx/Tx software for Airspy, Airspy HF+, BladeRF, HackRF, LimeSDR, PlutoSDR, RTL-SDR, SDRplay and FunCube
GNU General Public License v3.0
2.85k stars 432 forks source link

AM Analog TV rendering issues (horizontal line shift) #459

Closed Vort closed 4 years ago

Vort commented 4 years ago

I have recently found a sample of TV signal: Принимаем и декодируем аналоговое ТВ с помощью SDR и Python. And converted it to .sdriq format.

I have tried to decode it with both 4.12.3 and 4.11.6, but had no luck with getting a stable result. It may be that I am setting wrong parameters to ATV Demodulator, but, most likely, it is just glitchy. Fixing of this problem would be a good task for someone who want to improve SDRangel. Here is the file, which can be loaded with FileInput plugin: atv_sample.sdriq (941 MiB).

And here is the screenshot of current decoding with 4.12.3: atv

Related: #21

f4exb commented 4 years ago

Decoding AM analog TV does not work well because it is difficult to scale the AM signal that has not constant power. It is pretty obvious by looking at the scope that there are amplitude jumps. For now I don't know how to fix it missing a good algorithm.

I see they use the GNU radio AM demod maybe something to be inspected.

Vort commented 4 years ago

It is pretty obvious by looking at the scope that there are amplitude jumps

What jumps? At 8.66 seconds? They can not affect decoding before that time.

At other points of time signal looks good enough. Here is my simple (crappy) demodulator in GRC: atv_grc And here is result waveform: atv_jump atv_no_jump

f4exb commented 4 years ago

I meant the scope tab of the ATV demod. In fact a lot of things do not seem to be quite right there starting with amplitude detection.Basically this should be 2 * sqrt (I² + Q²) - 1 for a [-1:+1] full scale and this is not what is coded. I tried the original GRC (adding the throttle as you did) and the result is roughly between -0.49 and -0.46 with the standard GNU radio AM demod.

f4exb commented 4 years ago

The above commits fix the AM demodulator inside the ATV demod. It uses the standard formula for complex envelope detection as stated above and probes the last 5 seconds for range detection. It is still not completely immune to false range detection if there are more than 5 seconds of dark images but this is unlikely for most transmissions. Note that you will have to wait 5 seconds once the reception is started to get a valid video signal. It starts with the assumption of a full range signal.

You have to adjust the Synch and Black levels to get proper horizontal synchronization and correct luminance range. I could get a good image with the recording provided here. It is a bit tricky to maintain horizontal synch on the first part (the interview).

Vort commented 4 years ago

You have to adjust the Synch and Black levels to get proper horizontal synchronization and correct luminance range.

I thought that correct amplitude for HSync is somewhere in the range of 50 .. 200. But it is not possible for me to make this recording work with, for example, Synch = 150. I was able to make synchronization work partially with Synch = 450. Why is that happening? Is my assumption about low level of Synch (at least, lower than Black) is wrong? Or horizontal synchroniztion algorithm in sdrangel is broken as well as AGC was?

f4exb commented 4 years ago

I don't know AM is troublesome.

Vort commented 4 years ago

I have tried to decode default hacktv test signal. hacktv -o baseband.bin -f 551250000 -m i -g 47 test Because it is high quality and have constant amplitudes.

Result is the same: With Synch = 100 (which should be perfect for such signal) decoded image still have glitches:

3.7MiB gif animation of glitches: ![test](https://user-images.githubusercontent.com/1242858/86034680-c215b880-ba43-11ea-8b63-9d4be297b258.gif)
Waveform: ![image](https://user-images.githubusercontent.com/1242858/86035348-df975200-ba44-11ea-9ae1-a5ec9076ad32.png)

Since with 9c05329bb3dc12b828f065e7aa7d00ed84b3de1f this issue is marked as fixed, what should be done next?

  1. Sync problems can be fixed in this issue as well as scaling problems.
  2. Or another issue with HSync should be opened instead?
f4exb commented 4 years ago

This is not a final solution (will there be?). It is not robust enough for a real signal like with your first example. With a test signal that spans the entire amplitude range at a calibrated level this is pretty easy to make it work. But it should be able to face more adverse situations.

As you probably understood the sync level is the horizontal sync trigger when the signal goes below this value and pulse length is determined by either the "H pulse" length or when the signal goes above the black level whichever is the shortest.

Vort commented 4 years ago

But it should be able to face more adverse situations.

It should work with real signals + test signals. Now it fails both of them.

As you probably understood the sync level is the horizontal sync trigger when the signal goes below this value and pulse length is determined by either the "H pulse" length or when the signal goes above the black level whichever is the shortest.

Yes, it should work like that. But it does not.

Synch is this level, right? pal ~50-150 and located below black level?

f4exb commented 4 years ago

The present code is missing an AGC and also we could use sqrt(I²+Q²) directly for the amplitude thus dealing with positive numbers only which is simpler. With AGC and min max scaling over just two complete images it works in most situations. Of course as said an uniform image at lower than 100% luminance will finally appear at 100% luminance with black level shifted up. I think there is not much to do to avoid that. At least we could hope the transmitter is smart enough to send some white somewhere on each image.

With this in place the horizontal sync works perfectly on calibrated signals. My description of "H pulse" control was not quite right and in fact it is critcal. This controls the number of samples displayed as a time duration that are counted as sync samples after pulse start detection (signal down past sync level). This is a variation from the nominal pulse time and the number of samples appears in the tooltip. It should be less than nominal time so make sure you move the slider somewhere on the left.

Vort commented 4 years ago

It is great that you are improving scaling algoritms. But it will not fix horizontal sync problems. HSync have its own problems, not related to signal levels.

With this in place the horizontal sync works perfectly on calibrated signals.

Let's use the same signal: hacktv_dec.sdriq.7z.zip (sorry for such mysterious packing, hope you will be able to unpack it) It's a 20 seconds of HackTV test signal, downsampled to 4 MS/s.

Screenshot of hacktv_dec.sdriq decoding ![htv](https://user-images.githubusercontent.com/1242858/86088944-ea3b0100-baaf-11ea-922f-18c140e17418.png)

I don't see perfect result here. Even if I change Synch, Black, H Pulse - it does not matter. Picture jumps randomly in horizontal direction. You can freeze its position by changing of sliders values. But the same effect can be achieved just by disabling of HSync.

f4exb commented 4 years ago

You have to set the "H pulse" below its nominal value of 4.7 us. Here 1.95 us works fine but its true that there might be some initialization issues because it is not correct right away. It is stable but shifted: 2020-06-30-083818_743x894_scrot

However after switching the "inv" back and forth the image eventually stabilizes in its right position: 2020-06-30-083836_743x977_scrot

Now you can notice that because the horizontal pulse is detected at a shorter time than its nominal value some of the sync appears in the image there could be an improvement here to shift right the line by the amount of sync left when sending it to the TV screen renderer. If I remember right the detection of pulse end (going above black level) is not very reliable on real signals therefore it is better not to count on it. However if the "H pulse" is at the nominal time or higher then normal pulse end detection takes place and it is not reliable.

Vort commented 4 years ago

some of the sync appears in the image

Porch, not sync. That is minor problem.

I think it should be fixed after "shift problem" fix. I wonder how that shift is possible at all. There are lots of sync pulses, they are detected correctly. But that information is not propagated further and image becomes randomly positioned.

I have tried to understand the logic of ATVDemodSink::processClassic(), but had no luck so far. Suppose that m_horizontalSynchroDetected is calculated correctly. But what is next? m_avgColIndex should be average? But it is replaced every time...

f4exb commented 4 years ago

Implemented in v4.14.14

Vort commented 4 years ago

HSync still fails for atv_sample.sdriq.

f4exb commented 4 years ago

It is not perfect but neither is the signal so it is good enough for me. It is time to move on stg else more useful.

On Mon, Jul 6, 2020, 10:07 Vort notifications@github.com wrote:

HSync still fails for atv_sample.sdriq.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/f4exb/sdrangel/issues/459#issuecomment-654083386, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABPHZP2LFFWIITWX5B75EB3R2GA4HANCNFSM4KAJN2WQ .

Vort commented 4 years ago

neither is the signal

It fails also with perfect signals. Here is the same signal as in hacktv_dec.sdriq, but with different samplerate: hacktv_dec2.zip. I can make more of them if you really want to make this plugin useful.

sdrangel v4.14.14 decoding: htvdm3

is time to move on stg else more useful.

Making ATV demod work not just accidently is useful thing I think.

f4exb commented 4 years ago

I think this latter issue is superseded by #560

Vort commented 4 years ago

560 is for simplification of debugging.

But anyway, I will open separate reports about non-standard samplerates, HSync problems etc. if I will be able to collect needed informaton. So this report may stay closed, more specific description of problems are needed.

f4exb commented 4 years ago

The "A" of ATV means "Amateur" or Amateur Radio and is open to all sorts of experiments with non standard sample rates, fps, modulations whatever. So if this is an issue with you I will split the plugins in ATV and Analog TV with Analog TV dedicated only to standards that are or have been commercial standards.

Vort commented 4 years ago

What this plugin should do and what should not is a broad and interesting topic. Splitting almost for sure will lead to code duplication, and this is a source of problems. So I think, after most annoying bugs are gone, first of all, signal requirements should be clarified. It is good also to make a collection of signals, which represent different sources with unique properties. And decide which properties makes signals so bad that they should not be supported by plugin.

But for now it is enough to fix bugs, which affects both official and unofficial modes.

Concerning sample rates, right now ATV Demod glitches if sample rate can't be divided by 15625. hacktv_dec2.sdriq have 5333333 S/s. 5333333 / 15625 = 341.333, which is a problem for plugin. But I think it is low priority bug, because more likely user will encounter HSync problems, so they should be addressed first.

f4exb commented 4 years ago

Yes the sample rate should be an integer multiple of number of lines times number of frames per second. This is mandatory and documented: https://github.com/f4exb/sdrangel/tree/master/plugins/channelrx/demodatv#4-tv-sample-rate

Now if this plugin becomes too controversial I may also remove it and keep it for my private use. Because honestly there are many more things to fix that are more important than analog TV that becomes obsolete. For example DATV does not work properly on QO-100 signals because it is not sensitive enough.

Vort commented 4 years ago

Yes the sample rate should be an integer multiple of number of lines times number of frames per second. This is mandatory and documented:

When bug is documented, it becomes a feature :) So it just have even lower priority.

Now if this plugin becomes too controversial I may also remove it and keep it for my private use.

It is not controversial, it just have bugs. Which should be fixed. But they should not be fixed ASAP. They may be documented with bug report and wait for people willing to fix them. That is what open source for - collective work. If you close report just because you don't have time to fix the problem, there will be less chances that someone decides to start work on it.

analog TV that becomes obsolete

It is very important for historical purposes. And for demonstration of SDR advantages. If people was able to decode such signal almost perfectly 50 years ago, then now it should be possible to make even better decoding.

tl;dr: no rush required; small sequential fixes will make this plugin great

f4exb commented 4 years ago

When bug is documented, it becomes a feature :)

I totally disagree on this point. It is not a bug it is by design and indeed introduces a limitation. You may not like it and I will not fight if you find a better solution that does not have this constraint but this is not something that was introduced inadvertently i.e. a "bug".

It is very important for historical purposes.

Sure. And talking about historical stuff today only amateur radio uses CW so similarly analog TV is now more in the realm of amateur radio and with amateur radio also comes experimentation. Therefore it is very important not to be limited to commercial or once commercial standards. The possibility to choose different lines per image, FPS, synchronization schemes than commercial is very important and should be kept. This is a major feature and definitely not a "bug".

I totally agree on fixing bugs and if you or s.o. else can contribute this is great. But the two above points are not bugs and for the first one a better design should be found and for the second it is in fact a key feature. A better design means partial or full rewrite and by now you may have had a closer look at the code and I think it needs a full rewrite because it has had so many patches along the way that it becomes almost unreadable. A better strategy is therefore to rewrite it from the ground up and with a new design.

As a side note analog TV is not entirely historical the FPV (First Person View) on drones use analog TV to transmit images not digital. They even use color so to be complete a good SDR analog receiver should also implement color possibly as an option for power users with power hardware but it should be there. Another issue is open for that that has been there for ages so when talking about not ASAP...

If people was able to decode such signal almost perfectly 50 years ago ...

They used analog circuits and in the particular case of SDR and analog TV it is difficult to mimic these circuits particularly at high sampling rates with commodity hardware. In my opinion to overcome limitations one should look at a best match with these analog circuits (and there might be compromises) starting with the schematic of an old TV receiver. As stated before it should also implement color and for color it is very important to look at what analog circuits do.

To summarize this is not just fixing bugs here and there but this plugin should be rewritten from the ground up.

Vort commented 4 years ago

I totally disagree on this point.

Ok, bug limitation should be fixed removed. At some time in future.

Therefore it is very important not to be limited to commercial or once commercial standards. The possibility to choose different lines per image, FPS, synchronization schemes than commercial is very important and should be kept.

I see the final result as description of various modes as set of variables. Like line count, FPS, interleave, sync pulse width etc. If both amateur and commercial modes can be described in such manner (and I almost sure they can), then there are no problems. (maybe it is already implemented in such way, did not digged deep enough)

As a side note analog TV is not entirely historical the FPV (First Person View) on drones use analog TV to transmit images not digital.

It is also possible to use it for playing old games: Capturing PAL video with an SDR (and a few dead-ends)

f4exb commented 4 years ago

This is an interesting link I may come back to it later.

For now I realized something. This pertains to the first sample file described in the issue (the one with the interview). The sample rate is 9 MS/s. While testing the FileInput plugin with this file I noticed in the console log that there were regular packet drop messages from the SampleSinkFIFO hence the sync cannot be stable because the stream just misses samples. It is also noticeable on the scope window because the waveform is not fluid. The second file (hacktv) at 4 MS/s does not cause the same issue and sync is stable. My conclusion is that at least on my hardware 9 MS/s is too much (I see the file read QThread going 100%) and this has nothing to do with an issue in the ATV plugin horizontal sync detection. I have tried a few things on the FileInput code but did not find a way to cope with the high sample rate but this is another issue and may be simply a limitation.

Vort commented 4 years ago

While testing the FileInput plugin with this file I noticed in the console log that there were regular packet drop messages from the SampleSinkFIFO hence the sync cannot be stable because the stream just misses samples.

I have found this too. But in my case large amount of CPU usage was because of filtering active. And without filters chroma and sound will destroy decoding.

But it does not mean that ATV works good :) If I shift signal to 0 Hz and apply decimation/filtering in GRC and then load result in sdrangel, glitches will still show up: First, decoder do not like normal black level of 300mV. Scope shows that black is actually at 300. Second, signal have tiny sample rate incosistency. I do not know how this defect is called and why it is happening. But I almost sure that it is common problem and plugin should tolerate slight variations in timing. It is observed as slow creep of image to the right. Then sync mechanisms of plugin resets picture to normal position and that process repeats. That happens even at CPU load of 3%, so no dropping.

f4exb commented 4 years ago

I can decode the 4 MS/s hacktv sample file right away since fixes for #560 have been put in place.

There is a fundamental issue with adapting analog TV to digital. Not sure my implementation is the best but there is definitely an issue:

In pure analog TV the electron beam of the CRT can fall anywhere on the screen where the deflectors lead it to. This is not the same story with digital. You have a frame of pixels with a definite number of rows and columns. This is where the sample rate constraint comes from. For the image to look acceptable you have to align the samples to the frame boundaries else if there is only one sample difference from one line to another it will be noticeable on the screen. Anti aliasing does not help here because this is not the case of a curve approximated by a staircase.

In my implementation the screen size is calculated as a best fit to the sample rate, number of lines and frames per second but in practice this really works only if the sample rate is an exact match. Then a sample counter runs and is adjusted at each frame change to follow the horizontal sync. There is a sort of amortization so that the image does not always flicker horizontally however it does fluctuate a bit but this is always better than line shear.

About levels: the horizontal sync is triggered when the video signal goes below the horizontal sync level (nominally 100 mV or 0.1 in amplitude). The black level is used for two things: first as the actual video black level then to reset the horizontal sync latch.

Vort commented 4 years ago

I have decided to make 2x decimated version of atv_sample.sdriq. Hope that my transformations did not altered signal shape too much. It was interesting for me to make them manually, so I wrote a program for it:

Source code ```csharp using System; using System.Collections.Generic; using System.IO; namespace FreqShift { class Program { static double Sinc(double x) { if (x == 0.0) return 1.0; return Math.Sin(x) / x; } static double[] MakeRaisedCosine(double bitDuration, int tapCount, double beta) { double[] raisedCosine = new double[tapCount]; for (int i = 0; i < tapCount; i++) { int t = i - tapCount / 2; if (Math.Abs(t) != bitDuration / (2.0 * beta)) { raisedCosine[i] = Sinc(Math.PI * t / bitDuration) * Math.Cos(Math.PI * beta * t / bitDuration) / (1.0 - Math.Pow(2.0 * beta * t / bitDuration, 2)); } else raisedCosine[i] = Math.PI * Sinc(Math.PI / (2.0 * beta)) / 4.0; } return raisedCosine; } static double Dot(double[] a, double[] b) { double sum = 0.0; for (int i = 0; i < a.Length; i++) sum += a[i] * b[i]; return sum; } static void Main(string[] args) { var sampleLen = 4; var headerLen = 32; var sampleRate = 9000000; var decimation = 2; var tapCount = 150; var cutoffFreq = 1500000.0; var rc = MakeRaisedCosine(sampleRate / 2 / cutoffFreq, tapCount, 0.6); var sfs = File.OpenRead("atv_sample.sdriq"); var sampleCount = (sfs.Length - headerLen) / sampleLen; var br = new BinaryReader(sfs); br.BaseStream.Seek(headerLen, SeekOrigin.Begin); var ib = new Queue(new double[tapCount]); var qb = new Queue(new double[tapCount]); var dfs = File.Create("atv_sample_dec.sdriq"); var bw = new BinaryWriter(dfs); bw.Write(4500000); bw.Write((long)554151000); bw.Write((long)1577259226); bw.Write(16); bw.Write(0); bw.Write((uint)0x5DE211C2); var shiftFrequency = 2901770.76; var x = 2 * Math.PI * shiftFrequency / sampleRate; for (int t = 0; t < sampleCount; t++) { if (t % 4194304 == 0) Console.Write('.'); var i = br.ReadInt16(); var q = br.ReadInt16(); var phase = x * t; var cos = Math.Cos(phase); var sin = Math.Sin(phase); ib.Enqueue(i * cos - q * sin); qb.Enqueue(q * cos + i * sin); ib.Dequeue(); qb.Dequeue(); if (t % decimation == 0) { bw.Write((short)Dot(ib.ToArray(), rc)); bw.Write((short)Dot(qb.ToArray(), rc)); } } dfs.Close(); Console.WriteLine("Done"); } } } ```

It is slow, but this does not matter since it should be executed only once.

The goal was to minimize overshoot and ringing while preserving as much information as possible. I have uploaded result to DropMeFiles: atv_sample_dec.sdriq, 470 MiB, 4.5 MS/s. But it is possible to recreate it on your own PC (with Mono, maybe). I also can upload it to different file sharing service if needed.

After such transformation was done, it became obvious for me that signal have glitch at 8.66 seconds:

Glitch screenshot ![atv_sample_glitch](https://user-images.githubusercontent.com/1242858/87244690-a8af3c00-c447-11ea-91cd-be58ef115c2a.png)

Not surprising considering slightly wrong samplerate. Some samples were doomed to be dropped or duplicated. Good news is that it is the first and last glitch (as far as I can see). Just enough to test reliability improvements for ATV plugin :)

I hope that your PC will have enough power to process this file without additional drops.

(I will answer to your last message later)

f4exb commented 4 years ago

I could download the file OK. I do not understand what is wrong with the sample rate and how does this connect to the "glitch"? The file is a succession of samples and basically you choose the sample rate and this will give you a representation of the time. The result may be incorrect from the original signal time line if the sample rate is not equal to the original but if you consider just IQ then this is not an issue. To be sure... is what you show the IQ samples? If there is any discontinuity then this could be an issue for the sync but that is expected. If this is IQ then this discontinuity looks like a phase jump and can be just an issue in the receiver that recorded this. In theory AM should be insensitive to phase but this is theory. Maybe there is also a jump in amplitude that we do not notice on the screenshot. When playing the file in the ATV demod I do notice a hiccup in the sync at this offset: ~8s.

I do not have the exact same view in Audacity and at around the same time there is indeed an issue with amplitude not just phase:

Amplitude jump ![2020-07-12-192311_1920x1056_scrot](https://user-images.githubusercontent.com/6192319/87252700-79172880-c475-11ea-89c9-f410e1cc12d1.png)

Edit: I have more of these. After a second thought this may just be the vertical sync as we are in inverse video this make indeed a jump up in amplitude but this is normal

Vort commented 4 years ago

For the image to look acceptable you have to align the samples to the frame boundaries else if there is only one sample difference from one line to another it will be noticeable on the screen.

Even half-pixel shifts are noticeable. So aligning to an integer number of samples will always produce problems.

In my implementation the screen size is calculated as a best fit to the sample rate, number of lines and frames per second

Now I see. So you are doing resampling with the help of OpenGL. That is fast, but may be not accurate.

I do not understand what is wrong with the sample rate and how does this connect to the "glitch"?

Ideally, line length should take exactly 64us. But if you measure it in first 8 seconds of atv_sample_dec.sdriq, then you will see that it takes 64.00005us instead. I suspect that it happens because receiver have slightly incorrect sampling frequency. Which means that at some time it will have extra samples or shortage of samples. This state can not be kept for a long time: at some time samples will be either dropped or created out of nowhere (filled with zeroes or some other crap). This is what may happened at 8.66s and what causes image creep.

I do not have the exact same view in Audacity

That is because Audacity have limitation: When you select raw input and set sample rate to 4500000, Audacity actually sets it to 100000. To overcome this, I save it into wav file as is, then put correct sample rate with hex editor (0x18 offset in my case) and open file again.

f4exb commented 4 years ago

So you are doing resampling with the help of OpenGL

Whether using OpenGL or not a frame on the screen has a definite integer number of rows and columns and there is no way to overcome this. What I was trying to say is trying to fit an analog signal into this is an issue and not an easy one to solve. So of course we have to take an integer number of samples at some point because there is no such thing as fractional samples.

if you measure it in first 8 seconds of atv_sample_dec.sdriq, then you will see that it takes 64.00005us instead.

Yes this is why the frame will move slowly laterally as we go along this is by design and to change this you may have to change the whole design if you find a better design ever. However this does not compromise the horizontal sync process.

This is what may happened at 8.66s and what causes image creep.

I don't think so as said earlier exact sample rate mismatch will only cause the image to move slowly. Now if you have an issue in the original IQ input this will defeat the algorithm that tries to process it whatever is the algorithm. I do not consider this as an issue.

f4exb commented 4 years ago

there is no such thing as fractional samples.

Second thought: a common way to mitigate this is interpolation however in this case you would have to interpolate by a fairly large number: 8, 16...? This leads to sample rates that commodity hardware may not be able to sustain.

GkvJeep commented 4 years ago

irrational rate https://liquidsdr.org/doc/msresamp/

f4exb commented 4 years ago

This exists in SDRangel too: https://github.com/f4exb/sdrangel/blob/master/sdrbase/dsp/interpolator.h

But here the issue is that we trust the sender to run at some sample rate but there is a clock mismatch between the sender clock and the computer clock. This clock skew that is bound to happen causes a slight mismatch which at some point results in sample excess or shortage as Vort mentioned. If you interpolate by a large number this proportionally reduces the effect of the excess or shortage but to be effective you need to interpolate by a large number not compatible with common processing power.

The Remote Input plugin has an adaptive FIFO since you cannot trust the sender and receiver clocks that reside on different machines to match. However I am not sure this would be efficient in this case.

Vort commented 4 years ago

Whether using OpenGL or not a frame on the screen has a definite integer number of rows and columns and there is no way to overcome this.

But you can fit N samples into M pixels with different ways. OpenGL resampling (scaling) is one of them.

Yes this is why the frame will move slowly laterally as we go along this is by design and to change this you may have to change the whole design if you find a better design ever.

Yes, design should be changed. I will think about better approach. But I don't promise that there will be useful result. Will see.

However this does not compromise the horizontal sync process.

I don't think so. But I need to completely understand what is going on inside plugin to be sure.

Now if you have an issue in the original IQ input this will defeat the algorithm that tries to process it whatever is the algorithm. I do not consider this as an issue.

It may fully recover with 1 frame and with 10 frames of delay. That's the difference.

Second thought: a common way to mitigate this is interpolation

As I said earlier - this is exactly what OpenGL now do in your code.

f4exb commented 4 years ago

OpenGL resampling (scaling) is one of them.

That's a way that indeed could be promising. So first you obtain an image that is not exactly a rectangle and may be slanted a bit and then you correct it with an OpenGL matrix operation. I suppose this is the idea.

Vort commented 4 years ago

I think more about dealing not with rectangles, but with 1px tall lines. Which can be not only shrinked or expanded, but also shifted with subpixel precision. That precise shift may be provided to OpenGL by HSync detection algorithm.

f4exb commented 4 years ago

I see... yes the good point is to use OpenGL that is by essence fractional since it uses floats. Good idea indeed. I think that with all this content this issue can be reopened with a different title...

Vort commented 4 years ago

Here is the demonstration of how subsample precision can work: 9bb3d8119da0c65d5bf2a5c4936616f1ad283558. For now subsample shift is applied only to whole rectangle, so hacktv_dec2.sdriq will still have glitches. But for atv_sample_dec.sdriq results are better: Image creep is almost fixed (most likely, it can be completely eliminated, I need to think about it more). Default black level of 300mV not causing any problems now.

However, despite results are look better, there are still many work to do there. So commit is not ready for merging. I hope that I will be able to improve it more.

f4exb commented 4 years ago

I tried the various samples and the results are indeed much better. I did not notice anything bad with hacktv_dec2.sdriq the re-sync after file loop is even faster. I have put a few comments in the commit mostly acknowledging your changes. As said what I like in the "rectangle" approach is that it would be even in performance I think.

Vort commented 4 years ago

Here is another variation: 11ce0b2fa88da1b9cd48cd5048d291cef66b2aa2 (diff to previous).

Code most likely still contains logical mistakes, so the focus here is to test how it behaves.

Vort commented 4 years ago

I have found and converted one more recording: nanjing.sdriq, 853 MiB, 10 MS/s. It is important, since it is a broadcasted test signal.

Screenshot of decoding: ![image](https://user-images.githubusercontent.com/1242858/87575661-e676c380-c6d8-11ea-9350-2a93727e3051.png)
f4exb commented 4 years ago

The 10 MS/s of Nanjing TV record is a bit fast for me but at least I can experience the fast horizontal sync. It recovers much faster than before from the missing samples.

Now it is possible to disable VSync

I suppose you mean that one can disable VSync and the horizontal sync still works because this is what I noticed. Of course in this case vertical sync is not maintained. Indeed this achieves de-coupling between horizontal and vertical sync and that may not have been the case before (didn't try this recently).

Vort commented 4 years ago

Most parts of algorithm now works correctly: https://github.com/Vort/sdrangel/commit/39ff90a5e2a5194cda079404f7acd45db43c8791. Next I need to add extra columns to TVScreen so subsample shifts can work without glitches and test non-625-line modes.

upd. Fixed some of the problems, which I introduced in decoding of non-625 line modes: https://github.com/Vort/sdrangel/commit/53ae9786289fefa2915ebb637039c9c643e1e7ca.

Vort commented 4 years ago

Here is how subsample shift calculations behaves for atv_sample_dec.sdriq: hsync

At x = 3300 and x = 8300 m_sampleIndex = shiftSamples; line triggers change of sample index by 1 sample.

Vort commented 4 years ago

TV screen now have additional (partially visible) columns: 1 to the left and 1 to the right: https://github.com/Vort/sdrangel/commit/ff8368470744d78b573c6977f6e94153a96b5504.

f4exb commented 4 years ago

Tried your latest version with the various samples and it looks all good to me. It seems the Nanjing TV one is even crisper than at your commit 11ce0b2 but this could be my perception.

Vort commented 4 years ago

It seems the Nanjing TV one is even crisper

There was a lot of bugs in previous versions of my algorithm. So yes, with latest commit picture should be more stable.

I think that latest version is good enough for review and squashing/splitting. Most likely, it should not be merged "as is", since intermediate commits contains many "noise" and mistakes.

f4exb commented 4 years ago

For me it is good to go for merging. I tried it successfully with different number of lines and frames and vertical sync schemes and it works in all cases so application of the various standard parameters works fine.

If you would like to squash some commits before you are welcome to do so. I will not do it myself since I don't really know which commits make sense to be squashed or not. You may want to keep some intermediate commits to document the development logic.

Edit: I just merged a PR which wouldn't cause a conflict (it is the cmake file of demoddatv) but you may want to rebase on master.