Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

color.rgb2ycbcr and color.ycbcr2rgb are not mutually inverse #5972

Closed
hookxs opened this issue Oct 21, 2021 · 5 comments
Closed

color.rgb2ycbcr and color.ycbcr2rgb are not mutually inverse #5972

hookxs opened this issue Oct 21, 2021 · 5 comments
Labels
🫂 Support User help and QA

Comments

@hookxs
Copy link

hookxs commented Oct 21, 2021

Description

The colorspace conversion functions color.rgb2ycbcr and color.ycbcr2rgb are not mutually inverse. Maybe there is some assumption of the data type and/or input range which is not mentioned in the docs, I don't know. The rgb2ycbcr seems to work fine for both 0-255 uint8 images as well as 0-1 float images, the inverse ycbcr2rgb does not seem to work for either uint8 or float.

Way to reproduce

# rgb is a numpy uint8 HWC image with range 0-255
ycc = skimage.color.rgb2ycbcr(rgb)
rgb2 = skimage.color.ycbcr2rgb(ycc)
print(np.max(np.abs(rgb-rgb2))) # usually returns something close to 255, should be close to 0
@hookxs
Copy link
Author

hookxs commented Oct 21, 2021

I suspect there is a bug in ycbcr2rgb. The first thing the function does is subtract 16 or 128 and then it does the conversion which depends on the dtype. If one passes uint8 as input, the initial subtraction is nonsense. If one passes float ranged to 0-1 it is also a nonsense. If one passes float ranged to 255 the initial subtraction works but then the conversion treats the input as 0-1 ranged float which it isn't. Or I may be wrong and don't understand what's going on but someone more knowledgeable please look into it:-)

@rfezzani
Copy link
Member

Well, most skimage.color functions converts uint8 images to float using img_as_float internally.
Let's do some tests:

import numpy as np
from skimage import img_as_float
from skimage import color

### First create test data
# Create a uint8 image
rgb_u8 = np.round(np.random.rand(10, 10, 3) * 255).astype('uint8')

# Convert it to float
rgb = img_as_float(rgb_u8)  # This image is the target of the loop RGB -> YCbCr -> RGB

### Perform RGB -> YCbCr -> RGB loop

# Convert the both images to YCbCr
ybr_u8 = color.rgb2ycbcr(rgb_u8)
ybr = color.rgb2ycbcr(rgb)

# Convert back both images to RGB
rgb2_u8 = color.ycbcr2rgb(ybr_u8)
rgb2 = color.ycbcr2rgb(ybr)

### Validate the loop

assert np.allclose(rgb, rgb2)
assert np.allclose(rgb, rgb2_u8)

In your example rgb2 is the float version of your input rgb (ie img_as_float(rgb)). You can check it using

print(np.max(np.abs(rgb - skimage.img_as_float(rgb2))))

@rfezzani rfezzani added the 🫂 Support User help and QA label Oct 21, 2021
@mkcor
Copy link
Member

mkcor commented Oct 21, 2021

Dear @hookxs,

Thanks for reaching out!

I just checked and functions color.rgb2ycbcr and color.ycbcr2rgb are mutually inverse. When comparing images of different data types, you must rescale them before comparing them numerically, or rescale all input images beforehand, as done in the CI test:

img_rgb = img_as_float(self.img_rgb)[::16, ::16]

Please try running:

import matplotlib.pyplot as plt
import numpy as np
from skimage import color, data, exposure

img = data.colorwheel()
img.dtype  # dtype('uint8')
img.max()  # 255

ycc = color.rgb2ycbcr(img)
ycc.dtype  # dtype('float64')
ycc.min()  # 16.0
ycc.max()  # 240.0

rgb = color.ycbcr2rgb(ycc)
rgb.dtype  # dtype('float64') in range 0 to 1

img_rescaled = exposure.rescale_intensity(img, out_range='float')
np.testing.assert_array_almost_equal(rgb, img_rescaled)

With or witout rescaling, both img and rgb look like this:
rgb

Hope this helps!

@rfezzani
Copy link
Member

If you don't mind @hookxs, I will convert your issue to discussion 😉

@scikit-image scikit-image locked and limited conversation to collaborators Oct 21, 2021
@scikit-image scikit-image unlocked this conversation Apr 8, 2022
@grlee77
Copy link
Contributor

grlee77 commented Apr 8, 2022

comments moved back from the deleted Discussion

@hookxs on Oct 22, 2021
Ok, thank you @rfezzani and @mkcor for your quick reaction and explanation. I get it now. I think the confusing part is that the intermediate representation in YCbCr (eg the ybr_u8 image in @rfezzani's post) is a float but in the 0-255 range, which is unusual, at least as far as I understand. And this is the only combination that works with ycbcr2rgb. My use case is that I read the image directly from disk as an uint8, in which case it has the correct range (and img_as_float would be wrong to use here) but still needs to be converted to float for ycbcr2rgb to work, otherwise it returns nonsense. In my opinion this should be clarified in the ycbcr2rgb docs ("The function accepts only float dtype but in the same range as returned by the rgb2ycbcr, ie slightly less than 0-255") or the function should handle uint8 input (because YCbCr is intended to be used in 8bits so uint8 is not anything exotic here).
Anyway, thanks for your work.

@grlee77 on Oct 22, 2021
I agree that ycbcr2rgb should work with uint8 inputs in YCbCr space. In that case we should convert to float with ycbcr = ycbcr.astype(float) instead of using img_as_float so range is preserved. I will submit a fix for this.

@grlee77 on Oct 22, 2021
Another complicating factor here is that there are multiple YCbCr definitions. The one implemented in scikit-image is this one:
https://en.wikipedia.org/wiki/YCbCr#ITU-R_BT.601_conversion

while the one implemented by OpenCV's cvtColor function is this one:
https://en.wikipedia.org/wiki/YCbCr#JPEG_conversion

@grlee77 on Oct 22, 2021
@hookxs, I opened a PR at #5980 to allow uint8 input to ycbcr2rgb. Let me know if it looks good for your use case.

@hookxs on Oct 25, 2021
Thank you @grlee77 , that looks great.
Also, I did a bit of experimentation and for my particular case the "JPEG" flavor of YCbCr that you also mentioned performs better than the one implemented in skimage (which has the limited range of values). So it might be worth considering that such transform is also included in skimage, but that is of course a separate discussion.
Anyway, amazing feedback by the community, thanks:-)

@rfezzani on Oct 25, 2021
Please see #1185

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🫂 Support User help and QA
Projects
None yet
Development

No branches or pull requests

4 participants