§ ¶0.5 != 128/255
I cringe whenever I see people implement YCbCr to RGB conversion like this:
y = 1.164 * (y - 16.0 / 256.0);
r = y + 1.596 * (cr - 0.5);
g = y - 0.813 * (cr - 0.5) - 0.391 * (cb - 0.5);
b = y + 2.018 * (cb - 0.5);
What's wrong with this, you say? Too slow? No, that's not the problem. The problem is that the bias constants are wrong. The minor error is the 16/256 luma bias, which should be 16/255. That's a 0.02% error over the full range, so we can grudgingly let that slide. What isn't as excusable are the 0.5 chroma bias constants. If you're working with 8-bit channels, the chroma center is placed at 128, which when converted to float is 128/255 rather than exactly one-half. This is an error of 0.5/255, which would also be barely excusable, except for one problem. The coefficients for converting chroma red and chroma blue are greater than 1 in magnitude, so they'll actually amplify the error. If you're converting from 8-bit YCbCr to 8-bit RGB, this basically guarantees that you'll frequently be off by one in one or more components. Even more fun is that the green channel errors won't coincide with the red and blue errors and will be in the opposite direction, which then leads to ugliness like blacks that aren't black and have color to them.
In other words, please use 16/255 and 128/255 when those are actually where your luma and chroma values are based.
(I have to confess that the reason this came to mind is that I found this issue in some prototype code of mine when I started actually hacking it into shape. Needless to say, the production code doesn't have this problem.)
You might be thinking that it's a bit weird to be doing a conversion on 8-bit components with floating point, and you'd be correct. The place where this frequently comes up is in 3D pixel shaders. A pixel shader is a natural place to do color conversion, and is also where you'd frequently encounter 8-bit components converted to floating point. Unlike a CPU-based converter, however, there is basically no extra cost to using the correct constants. The only time you'd be better off using 0.5 in a shader instead of 128/255 is if you're on a GeForce 3, in which case (a) you're already skating on thin ice precision-wise and (b) the hardware is 9-bit signed fixed point so you're going to get 128/255 anyway. Otherwise, it's kind of sloppy to be displaying 720p video on-screen with a shader that doesn't get colors right just because someone was too lazy to type in the right constants.
So that people blindly copying-and-pasting don't inadvertently use integer division, maybe you should explicitly recommend 16/255.0 and 128/255.0 instead?
James - 02 09 08 - 04:41
Interesting topic, since I always struggle to get colour matched uniformly across various sources. This got me to look at the MPC source for "16-235 -> 0-255" shader... and I don't think it's doing the right thing. Firstly half the #defines are useless, and secondly it seems to compute only the "y=..." part of your equation (255.0/219.0 is the 1.164 in your formula). So is this shader ineffectual/wrong/broken? This is a question to anyone here who has experience with this stuff, not necessarily Phaeron.
sampler s0 : register(s0);
float4 p0 : register(c0);
float4 p1 : register(c1);
#define width (p0)
#define height (p0)
#define counter (p0)
#define clock (p0)
#define one_over_width (p1)
#define one_over_height (p1)
#define PI acos(-1)
#define Const_1 (16.0/255.0)
#define Const_2 (255.0/219.0)
float4 main(float2 tex : TEXCOORD0) : COLOR
return( ( tex2D( s0, tex ) - Const_1 ) * Const_2 );
Keije - 02 09 08 - 17:07
That's a bit unnecessary, don't you think? Some people may not notice if the constants are off by 1/255, but they'll definitely realize something's wrong if their constants are *zero* and the entire screen turns green.
Yes, that shader isn't quite correct. The extraneous #defines are not unusual because they're just the standard definitions for the constants provided by MPC, but the formula does need tweaking. First, by amping RGB, it is also affecting saturation as well as luma. Second, the range for chroma isn't quite the same as for luma: it's 16-240 instead of 16-235. It's close enough that there's not much difference and it's just about the same, but it's also not hard to correct. The luma axis can be extracted via dot product and then subtracted from the original color to recover chroma. The scale for chroma commutes with the YCbCrRGB transform and no chroma offset is necessary, so we can then just scale the chroma in RGB space and re-add the corrected luma.
Anyway, try this instead. If your video card is slow, try replacing all instances of float/float2/float3/float4 with half/half2/half3/half4.
sampler s0 : register(s0);
float4 main(float2 tex : TEXCOORD0) : COLOR
float4 px = tex2D(s0, tex);
float y = dot(px.rgb, float3(0.2567882f, 0.5041294f, 0.0979059f));
float3 c = px.rgb - y;
y = (y - (16.0f/255.0f)) * (255.0f / 219.0f);
c *= 255.0f / 224.0f;
return float4(c+y, px.a);
Phaeron - 03 09 08 - 01:51
Well the shader works fine and speed is not an issue, but I can't find any visual difference between it and the MPC's original.
At any rate, what shader language is this? HLSL, since the player is DirectShow based? MPC's site appears to contain no specs for this.
Keije - 03 09 08 - 09:32
It's nice to see the top post on this blog being about YUV, seeing as vdub still defaults to making pointless RGB conversions in transcodes...
as - 03 09 08 - 10:56
That shader is not in MPC, at least not in my version. There is something similar called "procamp" that does color space conversion and can change the scale if one part of it is uncommented.
Gabest - 03 09 08 - 10:58
Well, I am actually somewhat suprised I actually got it right (in the MPlayer OpenGL code). Except of course for the GeForce3-compatible code, that is beyond good and evil correctness-wise. On the plus side there is probably no faster way to get at least an ok image (though I admit I only looked at OpenGL features)...
Reimar - 03 09 08 - 14:56
@Gabest, I should've been more specific, I'm using the MPC Home Cinema (one of the builds from the development thread over at Doom9). Note that not all builds appear to include every shader that is actually available.
And as a follow-up on the shader comparison, I watched some of my caps from actual TV (as opposed to dvd I tried before) and there is a minute difference between original and Avery's version. Can't really tell which one is "better", but since Avery's version is mathematically correct, I'll stick to that.
Keije - 03 09 08 - 16:01
Excuse me, that's a lie and has not been true for a long time. VirtualDub has been able to do pure transcoding in YCbCr for a long time via fast recompress mode, can do YCbCr-to-YCbCr conversions without roundtripping through RGB since 1.6.x, and video filters have been able to handle YCbCr since 1.8.x.
Phaeron - 04 09 08 - 01:48
Maybe this stuff has something to do w/ a MPC-HC bug when using custom-EVR as renderer. :/ The video is too bright then... and the "16-235 -> 0-255" shader has to be applied to correct the brightness. It doesn't happen w/ EVR though...
I will give the devs a link to this post.
Avi - 06 09 08 - 11:18
Huh? what are you talking about, Avi? All movies (anything from DVD and TV) is in 16-235 range, everything needs conversion to 0-255 for playback on a PC.
Unless you mean that you used Vdub to make the conversion and it didn't work and you still had to use MPC's shader?
Keije - 07 09 08 - 11:27
Some video renderers (overlay mixer, Haali's renderer) does the TV->PC range expansion in the renderer by default. VMR9 usually doesn't. I dunno about EVR but I don't think it does.
TheFluff - 07 09 08 - 12:23
The problem with the VMR conversion is that it depends on the *video driver*. I have an NVIDIA-based laptop here that does it wrong (0-255), and an ATI-based laptop that does it correctly (16-235). This means that on the NVIDIA based laptop, the video levels differ depending on whether hardware acceleration is available -- if I remote into it at 24-bit, thus turning off hardware acceleration, I get 16-235.
Phaeron - 07 09 08 - 16:08
ffdshow also does range extension, so let's hope there's no one out there watching a video decoded by ffdshow, rendered with Haali's, and shaded with MPC's 16-235 -> 0->255 shader :/
Emanuel - 17 09 08 - 05:51