Skip to content

fix(encoder): only convert RGB input where the driver sites chroma as the stream implies - #49

Merged
hgaiser merged 3 commits into
hgaiser:mainfrom
porkloin:rgb-chroma-location
Nov 4, 2026
Merged

hgaiser merged 3 commits into
hgaiser:mainfrom
porkloin:rgb-chroma-location

Conversation

@porkloin

@porkloin porkloin commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

Small follow-up to #46, came out of @repparw's review on hgaiser/moonshine#254.

With 4:2:0 there's one color sample per 2×2 block, and the decoder has to guess where in the block it was taken. Our streams don't say, so decoders should fall back to the usual default (left pixel, between the two rows). session_info asks the driver for that default, but if a driver can't do it, it quietly picks a different spot, and colors could end up half a pixel off from where the decoder draws them.

This makes it refuse RGB input in that case instead, so callers just use the shader converter like before (Moonshine already falls back when the RGB encoder can't be built).

… the stream implies

Streams don't signal a chroma location, so players assume type 0 (cosited left, midway vertically). A driver that couldn't site chroma there got the other offset, shifting color edges without the stream saying so. Now it gets no RGB input, and callers convert instead. RADV supports type 0.
@hgaiser

hgaiser commented Nov 4, 2026

Copy link
Copy Markdown
Owner

Awesome, good find! This lead to some other smaller issues, namely that we never signal that we set the type of the chroma location so players presumably default to type 0, but also that the shader didn't actually convert using type 0 (using 3x2 with 1, 2, 1 as weights over the columns), but type 1 (average of a 2x2 block).

I pushed those fixes on top of your PR, PSNR went up quite a bit in my tests:

┌──────────────┬────────────┬────────────┬───────────────┐
│    Frame     │ Old shader │ New shader │ ffmpeg type 0 │
├──────────────┼────────────┼────────────┼───────────────┤
│ testsrc2     │ 30.44      │ 31.14      │ 31.56         │
├──────────────┼────────────┼────────────┼───────────────┤
│ smptehdbars  │ 36.65      │ 37.54      │ 37.86         │
├──────────────┼────────────┼────────────┼───────────────┤
│ mandelbrot   │ 28.32      │ 28.73      │ 28.58         │
├──────────────┼────────────┼────────────┼───────────────┤
│ rgbtestsrc   │ 42.17      │ 42.16      │ 40.96         │
├──────────────┼────────────┼────────────┼───────────────┤
│ testsrc      │ 31.62      │ 32.57      │ 32.74         │
├──────────────┼────────────┼────────────┼───────────────┤
│ screenshot 1 │ 44.80      │ 45.46      │ 45.21         │
├──────────────┼────────────┼────────────┼───────────────┤
│ screenshot 2 │ 40.88      │ 42.15      │ 41.91         │
├──────────────┼────────────┼────────────┼───────────────┤
│ screenshot 3 │ 39.17      │ 40.42      │ 40.00         │
└──────────────┴────────────┴────────────┴───────────────┘

Pretty much on par with ffmpeg, or even slightly better.

@hgaiser
hgaiser merged commit bcf755a into hgaiser:main Nov 4, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants