@oyvindln I need to test this with the tapes that had issues with before its ready to merge.

@oyvindln This is ready for you to review / test when you get a chance. I still may do some clean up and add a few things though before it's ready to merge. Let me know if you want me to change something or have a question.

I will try look into it in more time sometime during this week

For 525 v 625 line blanking signals - see e.g this https://martin.hinner.info/vga/pal.html
I couldn't quite make out from a quick glance whether it would require any changes for 625 line (PAL - though to be pedantic PAL colour can be used for 525-line video too).

The code will also need to cope with non-standard blanking signals like various types of "240p/288p" (or 400-line format) right now that requires the --fallback_vsync parameter to enable the function that does the fallback line0 detection as noted in the livestream, or the signal being damaged. Or at least have some fallback to the old "fallback" line0 code or something so that can still be decoded (EIAJ and Type-C pretty much requires this to work as the former tends to not have proper vsync and the latter doesn't record the vsync area on tape so relies on it in vhs-decode even though it on real hardware would rely on servo feedback.)

Okay - tested a bit. Seemed to work well on PAL and NTSC samples of most typical formats. Only sample I had an issue on was the PAL-N sample that was recently linked on discord. (I don't think it has anything to do with it being PAL-N specifically, it also had some non-standard vsync that required the fallback-vsync parameter when using the old code.)

It crashes when trying with any of the open reel format as prev_first_hsync_loc gets sets to None which causes the call to sync.get_first_hsync_loc to fail. If tweak it so it gets set to -1 if it's None if ends up part-working on the EIAJ sample (once I tweak the IRE0 parameter for the sample, the current defaults seem to be way off from the samples I have - need to look into whether the params for them are wrong or if there are just multiple standards..) while on the TYPEC sample it crashes with a different error. Might be easiest to test with the latter first as those are very clean - there are some samples on the drive (also quite interested if this PR helps any for the TYPEB samples once those issues are ironed out.)

cvbs also needs to be sorted as it tries to call the function with wrong params but you are already aware of that it looks like.

@oyvindln I looked around the shared drive, linked on the discord, but couldn't find the TYPEC samples. Could you share the link for that?

@eshaz Most samples were migrated you want this IA repository for Type-C and Type-B samples.

Seems to now be working better with the open reel formats as well, good work. Still having a bit of issues with not always getting the right starting line on them so the fields sometimes jump one frame down though, also encountered one frame on EIAJ where there was a line 1/3 way down that looked like it got skipped causing the rest of the field to get shifted one line down.

(I did these changes to the params to make the EIAJ samples from the gdrive to decode properly, need to look into the actual specs on what the defaults should be SysParams_PAL_EIAJ["hz_ire"] = 1400000 / (100 + -SysParams_PAL_EIAJ["vsync_ire"]) SysParams_PAL_EIAJ["ire0"] = 3.0e6 + ( SysParams_PAL_EIAJ["hz_ire"] * -SysParams_PAL_EIAJ["vsync_ire"] ))

@oyvindln Could you send me the file name / frame number where the sync issue occurred on the EIAJ sample? I'll take a look.

I'm currently testing with some more problem VHS tapes, and fixing a few edge case issues.

I tried decoding the Type C sample, but the starting line jumped around +- 1 line. Also the hsync was unstable, but I don't think the changes here would affect that. We might need to keep the fallback vsync option for these formats, if that is working better.

@oyvindln I was able to get Type C and B to decode ok. I think there still needs to be some tuning on the format parameters though. On this sample, I can visibly see the ending vsync pulses in the picture and the debug plot, but it's not being identified properly in the validpulses structure. This causes the picture to drift since the vsync transitions are improperly identified, probably somewhere in here: https://github.com/eshaz/vhs-decode/blob/d48efb085a7a53bb11561f8e58506ae4e95d1fbf/vhsdecode/field.py#L466

I tried a few of the EIAJ samples and I can't get any pulses to be found at all. Maybe I don't have the right sample downloaded or the format parameters are wrong, but it's failing before it gets to these changes.

VHS is really solid now, and there are no more field order issues, skipped lines, or dropped frames. This change may also help with gaps in recording that used to be skipped causing sync issues with audio. I changed it so that once the first vsync is detected, it doesn't any drop fields, and just tries to decode everything.

Type C (and EIAJ) has the fallback_vsync option active by default - since it doesn't have a proper vsync recorded on the tape it had to have enabled that to decode previously since the normal vsync code wouldn't find anything. (EIAJ very often also have non-standard vsync on the recordings due to being used with primitive sync generators hence why it was enabled by default there.)

Type C does need some filter tweaking still yeah. Same with other formats, type B is worse as the code doesn't deal with the head switching on it well yet (at least not with the old code.) That's outside the scope of this PR though, as long as it doesn't act any worse than it did before it's fine.

For the EIAJ samples on the drive you need to change the values for EIAJ PAL as noted in my previous post - they don't decode with the default ones. I think the current defaults are based on a different spec than what those samples used, or they are just incorrect - will have to look over specs and service manuals to find out which. May have to add multiple EIAJ options as these samples are clearly not using the same specs as the ones that the values that are in there now are based on at least.

Will have to re-check what EIAJ sample it was I had the issue on.

I tried a few of the EIAJ samples on the archive.org link with and without your parameter changes, and different cxadc options, but nothing worked. I haven't tried them all yet though, so maybe I just picked a few bad sample files?

I added some logic in this latest commit to sync the start location with the hsync pulses so there shouldn't be any more skipped lines, unless the video is severely damaged.

As far as this PR goes. I think the major changes to this new sync code are complete and now it's just a matter of testing and handling any edge cases that pop up. Is there anything you'd like for me to change before it's ready to merge?

For testing, I'm going to transfer around 80 or so camcorder / commercial VHS tapes for my uncle who is a train enthusiast. I'm also testing this with the large number of commercial VHS that I'm uploading to youtube and archive.org.

Okay, I'll look over to see if it's fine, and if there are just some minor things we can always work them out after merging.

I think the fallback_vsync param needs to be enabled by default on typec/eiaj/405-line still (haven't tested 405-line properly yet but that was already slightly borked.) - it has to be turned on for the NTSC typec samples to decode. Otherwise they seem to decode fine. The EIAJ sample (e502-ueit2.u8), not sure if it's one on the drive or from some other download also seems to work fine now provided I change the params as noted in the post.

The PAL typec sample etienne_1PC-tape1_PAL_CXWhiteModified_8bit_40mhz.flac which I think is on the drive meanwhile still seems to have issues with jumping up and down one pixel but that was the only one I've noticed a regression on now.

I reenabled fallback vsync to default on those formats, and also incorporated it into the new sync code. This should help if the fallback vsync fails for some reason, the guessed next location will be used.

I found the Type C file on the archive.org link: https://archive.org/download/Analog-Video-Tape-FM-RF-Reference-Archive/FM%20RF%20Samples/SMPTE/SMPTE_Type_C/etienne_1PC-tape1_PAL_CXWhiteModified_8bit_40mhz.typec. It no longer skips with these most recent changes. The problem was due to the field order being detected incorrectly because of the miss-detected vsync pulses. Now when fallback vsync is enabled, it will just use the inverse of the previous field like it was before unless the sync logic is completely sure about the field order.

I couldn't find the e502-ueit2.u8 file on archive.org or in the Google drive (pinned in the vhs-decode discord channel). If it's decoding fine though, I don't think I need it.

Not a big deal for my use-case, but it looks like the current code has no way to disable fallback vsync for the formats it is defaulted for.

Thanks for all the work! looks good now