happycube / ld-decode

Software defined LaserDisc decoder
GNU General Public License v3.0
294 stars 76 forks source link

lddecode: add --deemp_adjust_cb option for adjusting color burst deemph #867

Closed ripose-jp closed 1 week ago

ripose-jp commented 9 months ago

Allows color burst and 0.5 MHz region to have a different deemphasis adjust applied from the video burst region. This can potentially reduce rightwards smearing in the video region, without coming at the cost of introducing color banding that isn't present at high deemph adjust values.

For some background, I was having difficulty with rightwards ghosting and bright color smear pretty bad at default ld-decode settings. Eventually I discovered --deemp_adjust and this greatly reduced rightwards smearing and ghosting, but it came at the cost of introducing horizontal color banding not present at higher --deemph_adjust values. After examining the frames with banding in ld-analyse, I saw the color burst was slightly distorted for those lines. I thought I could fix it by decoding the color burst region with a higher deemph_adjust than the video burst region and found it worked perfectly.

Sorry for the white space changes in the files. My editor is configured to remove trailing white space on save. I can edit the commit without these changes if you like.

happycube commented 9 months ago

Interesting idea, but I'm a bit confused by the implementation. It looks like it's replacing one of the two deemphasis coefficients rather than setting up a second one.

happycube commented 9 months ago

If you can put a sample up and the decoding settings you used, I'll (hopefully) look at adding an optional second deemphasis filter this wekend.

ripose-jp commented 9 months ago

Interesting idea, but I'm a bit confused by the implementation. It looks like it's replacing one of the two deemphasis coefficients rather than setting up a second one.

I see why you think that, but it's misleading because the deemp_mult variable was originally tuple. It always just contained two copies of the same value though, so there wasn't any point to it. I repurposed it to contain two different deemp_adjust coefficients.

The relevant lines https://github.com/happycube/ld-decode/pull/867/files#diff-dcb5913cda7758165b049903bd7282a256b5fee738a1220d6def1cc0f8031305R293 https://github.com/happycube/ld-decode/pull/867/files#diff-d1a188176c18d35059aa87ff622e8b5307688b641c54ae91f5d962c09b16a6ccR566

If you can put a sample up and the decoding settings you used, I'll (hopefully) look at adding an optional second deemphasis filter this wekend.

I'll try to get that to you tomorrow.

ripose-jp commented 9 months ago

Here's a sample: https://litter.catbox.moe/tciwlf.ldf This file will automatically get deleted three days from now, so download it sooner than later.

At --deemp_adjust 1.0 there is minimal banding, but a lot of smear. The lowest --deemp_adjust I've been able to decode it at without issue is 0.62, though this creates a significant amount of color banding. At --deemp_adjust 0.62 --deemp_adjust_cb 1.0 there is a small amount of banding and no smear.

happycube commented 9 months ago

Yeah, I see the ringing there... this is a relatively low quality (albeit rot-free) disk (32dB SNR) with hints of crosstalk in the blue area.

Got an lddb link to the disk? This 'smells' like an 80's pressing.

ripose-jp commented 9 months ago

Sorry to be vague, but copyright and all that. I believe it's an early 90s pressing, though it is possible (not probable) that it could have been done in the late 80s. I see quite a bit of crosstalk noise in bright blues, reds, and yellows specifically. It could be a player issue in my case, but I'm not certain.

happycube commented 9 months ago

Take a look at branch chad-2023.10.07-deemp please - I cleaned up the deemphasis options and you now provide RF frequency in mhz for low-high values. In your case I think "--deemp_high 11" should be close to what you were looking at?

ripose-jp commented 9 months ago

--deemp_high 11 looks equivalent to --deemp_adjust 1.0 to me. I haven't played around with it too much yet, but I'm not certain how to get back to the result I was getting with --deemp_adjust 0.62 --deemp_adjust_cb 1.0 just yet.

happycube commented 9 months ago

I've added a --deemp_strength option that actually changes the level of deemphasis provided. (EDIT: i just fixed this, the first checkin set the default to 0 which really messes things up...)

I'd also look at boosting --deemp_low since I think the version you proposed changes both.

ripose-jp commented 8 months ago

Sorry for the late reply. Looking over the code on the chad-2023.10.07-deemp branch, I get the impression that variable de-emphasis is achieved through the --deemp_strength option since that constant is only applied to the video burst, right? If that's the case, I'm satisfied with the implementation and you can close this PR.