Properly darken chroma along with luma when enabling scanlines

[nyanpasu64]

Fixes an issue where scanlines were only applied to luma and not chroma, causing blue (and somewhat red) areas of the image failing to apply scanlines.

Note to self

``` crt monitor/gbs-c scanlines, deinterlacing.md # 2023-10-30 chroma fix ```

Background

Scanlines in 240p content are implemented by turning on the deinterlacer (but not RAM accesses?) to weave the current field with a black "previous field", then setting the deinterlacer into a mode where you manually control (using MADPT_Y_MI_OFFSET and MADPT_UV_MI_OFFSET) how much to interpolate between scanlines vs. using the previous black field.

Previously, only luma deinterlacing was enabled, and chroma was wholly interpolated between scanlines. This caused increased color saturation (and brightness if some RGB channels become negative and get clipped to zero) in scanline gaps.

Enabling chroma scanlines

This PR enables chroma deinterlacing (which was largely implemented but commented out). The commented-out code set MADPT_UV_MI_OFFSET (chroma blending/brightness) to a fixed value of 64, which caused chroma to remain partly blended between scanlines even when scanlines were fully set to black, by clicking "scanline intensity" until MADPT_Y_MI_OFFSET (luma blending/brightness) is set to 0.

• While doing slow-motion camera testing (PXL_20231030_115142583.mp4), I did not find any change in latency in 240p Test Suite between scanlines on and off (either way, after pressing a button to navigate a menu, there's 1 frame of no response, and the next frame had proper luma and chroma).
  • I did notice that the first frame's colors appeared to be slightly desaturated compared to the second, so I performed more testing:
• When switching between a white and blue screen (PXL_20231030_115938855.mp4), proper scanlines also appeared to appear immediately on the first blue frame, without blue light appearing in the gaps of the image.

Tuning chroma scanlines

I want the RGB values in scanline gaps to be linearly interpolated between black and the average of the adjacent scanlines. To do this, we must interpolate Y and UV by the same amount between black and the average of the adjacent scanlines. This can be achieved by setting MADPT_UV_MI_OFFSET = MADPT_Y_MI_OFFSET (which is itself written with GBS::MADPT_Y_MI_OFFSET::write(uopt->scanlineStrength)).

To verify I took pictures of my VGA CRT in 480p (which can resolve output scanlines) using a macro camera, to check that scanlines appeared correctly:

• If MADPT_UV_MI_OFFSET is always set to 0, then with bright scanlines (MADPT_Y_MI_OFFSET = 50) green areas of the image have gray scanline gaps with all RGB phosphors illuminated (when they should be green), because chroma is zero but luma is nonzero:

• If MADPT_UV_MI_OFFSET is always set to 64 (like in the commented-out code), then with dark scanlines (MADPT_Y_MI_OFFSET = 0) blue areas of the image have blue scanline gaps (when they should be black), because luma is zero but chroma is nonzero (red/green are negative and clipped to zero):

• This PR sets MADPT_UV_MI_OFFSET equal to MADPT_Y_MI_OFFSET = uopt->scanlineStrength. I verified that scanline gaps (red, green, and blue) are always the same color (but dimmer) than scanlines when brightness (the intensity button) is set to 50, and black when brightness is set to 0.

Questions

• [ ] Is it safe to "Print deinterlacing registers in 'd' register dumps"? It was helpful for this PR, and I don't know why it was disabled to begin with (to reduce the dump length slightly?). Also I don't think register dumps are something a normal user ever does.
• [ ] I renamed the printed message Scanline Strength: {MADPT_Y_MI_OFFSET} (which never matched the "intensity" button) to Scanline Brightness.
  • I prefer Scanline Brightnessbecause higher values correspond to higher brightness. The old name Scanline Strength implied that higher values produce stronger scanlines (when they actually produce weaker scanlines and gaps). Is this an acceptable name? (I tried Scanline Bloom which is more technically correct, but found the name too confusing for myself.)
• [ ] I'm not sure all the code I uncommented is "good". In particular, the final line GBS::MADPT_MO_ADP_UV_EN::write(1); // 2_16 5 (try to do this some other way?) sounds concerning, is this badly designed? It seems to work?

Fixes #458.

[nyanpasu64]

Try to make it look good, test it on some other displays and other source consoles, and it'll be fine? A lag test setup will be required to tell if the changes add any more latency.

I brought out a Samsung T24C550ND LCD monitor/TV for testing (since I didn't want to play on my vertically mounted VGA-compatible LCD monitor), and plugged the GBS-C into the monitor over VGA. Unfortunately it misinterprets GBS-C's 1280x960 output as 1600x900, so I set the GBS-C to output in 1080p mode (at the cost of horizontal resolution 😦).

Appearance, SNES testing

Super Mario World on a 1:1:1 Super Famicom looked good on the monitor (as good as GBS-C's emulated scanlines will get), and the scanlines on a 1080p display/signal were not noticeably worse than 960p on a CRT. The blue water on the Yoshi's Island overworld map had well-defined dark scanline gaps (as opposed to the vaguely differentiated sea of blue before this PR). I found that on this LCD, setting scanline brightness to 20 (rather than my earlier 30) made the image look better (though that may sacrifice too much brightness on a CRT with an inherently darker gamma curve). I did not feel noticeable input lag, but did not test the lag added by the monitor.

When I booted SNES 240p Test Suite and opened color bars, I noticed that the image background was a noisy dark green, and that with scanlines enabled, the darkened scanlines had a dark purple background instead. This was not noticeable unless I was looking closely at the display, and could be caused by the GBS-C and monitor's AC coupling and DC offsets. I did not notice this issue during actual gameplay.

Oscilloscope RGB testing

When performing oscilloscope captures with the GBS-C set to 480p output, I found that even on Scanline Brightness: 0 where scanline gap lines should be pitch-black, the output voltage within scanline gaps was around 6% of the full-scale voltage (using the blanking voltage as a reference). This occurred in both white, green, and blue images. Disabling "line filter" did not fix this problem.

On colored images, there was a small amount of crosstalk between output channels; a green bright line would induce a small amount of blue signal (while a blue bright line would induce less of a green signal). Darkened output scanlines would induce proportionally less signal to other color channels, or no signal at all if I set scanline brightness to 0.

Oscilloscope captures:

Green output scanline leakage, and low crosstalk to blue:  Blue output scanline leakage, and near-zero crosstalk to green: 

In summary, oscilloscope testing reveals that the scanlines aren't perfect, but it's "close enough" that I don't think my code has obvious flaws. Also I don't think I can reasonably improve it further, and don't want to try. I guess you get what you pay for...

---

While testing various parameters, I noticed that the scanline gaps were a lot brighter than I expected at higher scanline brightness values. I found that scanline brightness 50 (out of a 7-bit number or 127) produced a darkened output scanline RGB level (BY-AY) of 510 mV, compared to a bright scanline RGB level of 710 mV. This means that darkened output scanlines are 0.718 times as bright as regular output scanlines, while the blending factor programmed into the TV5725 is only 50/127 = 0.394. I'm not sure what's going on (what formula is used to interpolate between black/blended lines), but don't want to look deeper ATM.

Signal latency testing

Earlier I had already ran an button-to-photon lag test on Wii -> GBS-C -> CRT, using a slow-motion camera recording of my GC controller and the monitor.

This time I decided to run a signal-level lag test of the GBS-C alone:

• On the SNES, open 240p Test Suite to "White & RGB Screen". On the controller, press R until I see a green screen.
• On the oscilloscope, hook the probes to the GBS-C (1=input green, 2=input blue, 3=output green, 4=output blue).
  • Set the scope to trigger on channel 2 (input blue), and single trigger (capture one screen of data, then stop).
• On the SNES controller, press R to switch from green screen to blue (and have the scope capture data when blue is shown).

I found that the GBS-C's output switched cleanly from green to blue output, a quarter frame after the input switched from green to blue. By zooming into the signal capture, I found no visible errors (like using chroma levels from the previous frame) in the blue scanlines after the switch.

This implies that the GBS-C in chroma scanline mode has no latency, beyond the GBS-C's natural frame-buffering (which I've configured to a quarter-frame using frame sync).

• Oddly, it appears that if you open SNES 240p Test Suite's White/RGB Screen and press L/R to switch screen colors, the signal changes near the bottom of a video frame, rather than during vblank between frames. I observed this both in this oscilloscope capture, and visually on-screen.

Interestingly I found that with clockgen framesync enabled, the output signal actually drifts back and forth by around a quarter input-scanline or 12 microseconds. This is only visible when zooming way into the signal. The phase error appears to reach a positive or negative peak every 1.6 seconds, or lockInterval = 100 * 16.70; // every 100 frames (perhaps with some long-term drift/noise by a few microseconds). In any case quarter-scanline accuracy is more than good enough (assuming the monitor doesn't lose sync, which I haven't seen this LCD do so far), and I recall the error being even smaller when locking onto a Wii.

Summary

I could probably merge the PR now? I'm still hoping ArcheyChen can test the PR, though if he isn't ready to test the fix I may still merge it since it "works on my machine" and I don't want to change it further.

[ramapcsx2]

Man, I love that crazy test you built there, huge fan :D

The latency is actually my main worry, and it's good that it seems like it will not be an issue. If you read the register descriptions/programmer's guide a bit between the lines, it seems like adding Chroma would require more fully buffered fields (half frames, so 16.66ms / 2). It's possible though that we don't automatically get the added latency, if the registers aren't set up "correctly", and since we don't use this for actual deinterlacing, that might be what's going on.

I'm okay with merging this in any case. Love the scope test xD

[ramapcsx2]

Little side note: The SNES 240p test suite doesn't have to be switching the shown image in Vblank. There's no reason to assume anything like that, in any kind of test software, unless it was made specifically to avoid this :p

[nyanpasu64]

Sidenote: in television terminology, for interlaced signals, individual images (eg. in 480i, 240-line for 16.6ms) are called fields, and frames instead last for two vertical periods (eg. 33.3ms) and include both an odd and even field.

In 240p we call 16.7ms images "frames" for convenience (matching 480p), even though 240p isn't a standard TV signal and gets misinterpreted as 480i by some capture cards and TVs (which will weave the previous frame/field with the current, if you're lucky it's motion-adaptive "deinterlacing" and will only weave in 30hz flicker). It sucks how when a game switches from 240p to 480i, the 16.6ms vertical period changes names from a frame to a field, but I didn't make the rules.

---

In luma deinterlacing, I found that the previous frame buffers are only used to compare the current frame to, and weave old frame data in non-moving sections of the image. I'd speculate (but am not confident) that a similar thing is happening in chroma deinterlacing.