YUV implemented incorrectly

[opencv-pushbot]

Transferred from http://code.opencv.org/issues/4227

|| William Mantzel on 2015-03-05 00:56
|| Priority: Normal
|| Affected: branch '2.4' (2.4-dev)
|| Category: imgproc, video
|| Tracker: Bug
|| Difficulty: Medium
|| PR: 
|| Platform: x64 / Linux

YUV implemented incorrectly

The BT.601 BGR to YUV conversion is incorrectly implemented. The U channel is proportional to (R-Y) instead of (B-Y) and vice versa for V, and the coefficients for the Y channel are accidentally swapped, likely as a result of trying to account for RGB/BGR variations. 

At present, what is implemented is:
Y = .114R + .587G + .299R
U = .492(R-Y)
V = .877(B-Y)

When it should be:
Y = .299R + .587G + .114B
U = .492(B-Y)
V = .877(R-Y)

The inverse transform mirrors this bug, which is why it has stayed hidden for a while.

History

Steven Puttemans on 2015-03-05 08:12

Wikipedia (http://nl.wikipedia.org/wiki/YUV) says

<pre>
Y  = + 0,299R + 0,587G + 0,114B
U  = 0,492(B - Y)
   = - 0,147R - 0,289G + 0,436B
V  = 0,877(R - Y)
   = + 0,615R - 0,515G - 0,100B
</pre>

So indeed some stuff got swapped. Will supply a PR in a moment!
-   Assignee changed from William Mantzel to Steven Puttemans
-   Status changed from New to Open

Steven Puttemans on 2015-03-05 09:45

Fixing this seems more difficult than expected -_- I cannot find my way around the tegra optimizations.
Could you supply the fix maybe?
-   Assignee changed from Steven Puttemans to William Mantzel

Max Khardin on 2015-03-13 08:30

Steven Puttemans wrote:
> Fixing this seems more difficult than expected -_- I cannot find my way around the tegra optimizations.
> Could you supply the fix maybe?

Could you explain more - what are your particular problems with tegra optimizations? As far as I can see, all that we have in cvtColor for TEGRA_OPTIMIZATION and bgr2yuv is a call to tegra::cvtRGB2YCrCb that does not use any info on coefficients supplied by us. 

Or am I missing something?

Ilya Lavrenov on 2015-03-16 10:19

I don't understand where have you found a mistaken implementation?
https://github.com/Itseez/opencv/blob/2.4/modules/imgproc/src/color.cpp#L786 uses the same as in WIKI.

Steven Puttemans on 2015-03-16 13:46

Max Khardin wrote:
> Steven Puttemans wrote:
> > Fixing this seems more difficult than expected -_- I cannot find my way around the tegra optimizations.
> > Could you supply the fix maybe?
> 
> Could you explain more - what are your particular problems with tegra optimizations? As far as I can see, all that we have in cvtColor for TEGRA_OPTIMIZATION and bgr2yuv is a call to tegra::cvtRGB2YCrCb that does not use any info on coefficients supplied by us. 
> 
> Or am I missing something?

For example that was not clear to me at all :D

Max Khardin on 2015-03-19 18:05

Ilya Lavrenov wrote:
> I don't understand where have you found a mistaken implementation?
> https://github.com/Itseez/opencv/blob/2.4/modules/imgproc/src/color.cpp#L786 uses the same as in WIKI.

I think ticket is about this line: https://github.com/Itseez/opencv/blob/2.4/modules/imgproc/src/color.cpp#L3835

Nisarg Thakkar on 2015-04-03 03:37

I think this has been implemented correctly. OpenCV uses YCrCb whereas Matlab uses YCbCr. I guess this might be causing the confusion.

Artem Sanakoev on 2015-04-27 08:06

Possible workaround while bug is not fixed to convert from BGR to YUV:

<pre>
  cv::Mat source = cv::imread(filename, CV_LOAD_IMAGE_COLOR);
  cv::Mat yuvSource;    
  cvtColor(source, yuvSource, cv::COLOR_BGR2RGB); // rearranges B and R in appropriate order
  cvtColor(yuvSource, yuvSource, cv::COLOR_BGR2YUV);
</pre>

Vadim Pisarevsky on 2015-04-27 11:38

-   Category set to imgproc, video

Tim van Oosterhout on 2015-06-05 13:22

This issue is still present in version 3.0.0
An alternative workaround is to simply convert from RGB instead of BGR:

<pre>
cvtColor(yuvSource, yuvSource, cv::COLOR_RGB2YUV);
</pre>

[ceefour]

👍

[aleksey-nikolaev]

Version 3.1.0 Bug still alive!
Transformations wiki YCbCr. Look - YCbCr, Not YCrCb!
Let the term U is equivalent to Cb, and the term V is equivalent to Cr. To verify this it is enough to compare equations from wiki YUV and wiki YCbCr

BUG 1 BGR2YUV RGB2YUV

color.cpp:

            int Y = CV_DESCALE(src[0]*C0 + src[1]*C1 + src[2]*C2, yuv_shift);
            int Cr = CV_DESCALE((src[bidx^2] - Y)*C3 + delta, yuv_shift);
            int Cb = CV_DESCALE((src[bidx] - Y)*C4 + delta, yuv_shift);

This code must work for CV_BGR2YUV but not for CV_RGB2YUV because opencv did not swap multipliers for C0 and C2! Oh, no I miss this. OpenCV swap coefficients C0 and C2 for RGB source. Wrong Y for both source. BUG 1

BUG 2 BGR2YUV RGB2YUV

And color.cpp code wrong even for BGR. Let's compare with the simple formula from the Wiki (SDTV with BT.601)

Y-> 0.299 R + 0.587 G + 0.114 B
U-> 0.492 (B - Y)
V-> 0.877 (R - Y)

C4 == 14369 / 2^14 ~ 0.877
src[bidx] - is a color component B for both source,
BUG 2 B must be multiplied by C3

BUG 3 YUV2RGB YUV2BGR

color.cpp:

            dst[i] = saturate_cast<uchar>(Y);
            dst[i+1] = saturate_cast<uchar>(Cr);
            dst[i+2] = saturate_cast<uchar>(Cb);

WTF!!!! opencv use YVU byte order!!!! No it use wrong equivalence! Why I think so?

Let take a look color.cpp:

            uchar Y = src[i];
            uchar Cr = src[i+1]; // equivalent V?
            uchar Cb = src[i+2]; // equivalent U?

            int b = Y + CV_DESCALE((Cb - delta)*C3, yuv_shift);
            int g = Y + CV_DESCALE((Cb - delta)*C2 + (Cr - delta)*C1, yuv_shift);
            int r = Y + CV_DESCALE((Cr - delta)*C0, yuv_shift);

C0 > C3 and |C2| > |C1|
and this

R->1.14 V+Y,
G->-0.395 U-0.581 V+Y,
B->2.033 U+Y

Right side comparing give: opencv_Cr is equivalent to U, opencv_Cb is equivalent to V
OKAY, opencv use right YUV byte order but wrong equivalence. warning

But left side comparing give: int b = R - i.e. r and b are swaped! BUG 3

[aleksey-nikolaev]

#7396

System information (version)
OpenCV = 3.1.0
Detailed description
Сould you fix this very old bug?

Explain please which byte order used in opencv for YUV and YCrCb. Not now, but as it should be in truth.

@StevenPuttemans

There is no reason in opening up the same issue again, if it is already open. It will simply get closed down. Adding the info at the other issue, like you did, shall help fastening the process of getting the fix in. But providing a fix yourself through a PR will get you there even faster!

The reason was that I was expecting any answer, but nobody did not even changed the labels. Do you think that YUV implemented incorrectly is not a bug?
I can not fix this bug myself, because I have no idea what byte order should be for YUV and YCrCb in opencv and bug is bigger than I describe. Maybe code of other RGB2YCrCb_f or other template<> struct are also has this bug?

[StevenPuttemans]

Please keep in mind that no label change for 7 days does not mean it got ignored ... tons of developers do this in their spare time, and thus you have to be patient.

As to my opinion, I am unsure if YUV is broken in OpenCV, because I have been using it exhaustively in OpenCV in many projects and have not met unexpected behavior compared to my Matlab tests ... but we need a core dev to give his opinion first.

So again, patience is the key word here.