| Message ID | 20220824162425.71087-4-umang.jain@ideasonboard.com |
|---|---|
| State | Superseded |
| Delegated to: | Umang Jain |
| Headers | show |
| Series |
|
| Related | show |
Hi Umang, Thank you for the patch. On Wed, Aug 24, 2022 at 09:54:22PM +0530, Umang Jain via libcamera-devel wrote: > Rectify the ColorSpace::Srgb to denote that it does not use > any Y'Cbcr encoding and uses full range. > > The kernel on the other hand, recommends to use Rec601 as the encoding > for V4L2_COLORSPACE_SRGB. It is not very explicit but it can be > inferred that the kernel assumes V4L2_COLORSPACE_SRGB is a YUV-encoded > one. However, when the data is in RGB, no encoding is required (and > this is denoted by YcbcrEncoding::None in libcamera). > > Hence, to be clear on the libcamera colorspace API, rectify the > ColorSpace::Srgb preset to use YcbcrEncoding::None. > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > --- > src/libcamera/color_space.cpp | 14 +++++++------- > src/libcamera/v4l2_device.cpp | 7 +++++-- > 2 files changed, 12 insertions(+), 9 deletions(-) > > diff --git a/src/libcamera/color_space.cpp b/src/libcamera/color_space.cpp > index f0d6109a..cb47acf9 100644 > --- a/src/libcamera/color_space.cpp > +++ b/src/libcamera/color_space.cpp > @@ -47,7 +47,10 @@ namespace libcamera { > * > * Note that there is no guarantee of a 1:1 mapping between color space names > * and definitions in libcamera and V4L2. A notable difference is that the sYCC > - * libcamera color space is called JPEG in V4L2 due to historical reasons. > + * libcamera color space is called JPEG in V4L2 due to historical reasons. On > + * a similar note, the sRGB colorspace defined in the kernel assumes a Y'CbCr > + * encoding to it which is not true. Hence the ColorSpace::sRGB is defined > + * differently in libcamera (with no Y'CbCr encoding and full range). * and definitions in libcamera and V4L2. Two notable differences are * * - The sRGB libcamera color space is defined for RGB formats only with no * Y'CbCr encoding and a full quantization range, while the V4L2 SRGB color * space has a Y'CbCr encoding and a limited quantization range. * - The sYCC libcamera color space is called JPEG in V4L2 due to historical * reasons. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > * > * \todo Define the color space fully in the libcamera API to avoid referencing > * V4L2 > @@ -250,16 +253,13 @@ const ColorSpace ColorSpace::Raw = { > }; > > /** > - * \brief A constant representing the sRGB color space > - * > - * This is identical to the sYCC color space except that the Y'CbCr > - * range is limited rather than full. > + * \brief A constant representing the sRGB color space (RGB formats only) > */ > const ColorSpace ColorSpace::Srgb = { > Primaries::Rec709, > TransferFunction::Srgb, > - YcbcrEncoding::Rec601, > - Range::Limited > + YcbcrEncoding::None, > + Range::Full > }; > > /** > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp > index 1fb08b9d..a9e34ee1 100644 > --- a/src/libcamera/v4l2_device.cpp > +++ b/src/libcamera/v4l2_device.cpp > @@ -746,7 +746,11 @@ void V4L2Device::eventAvailable() > > static const std::map<uint32_t, ColorSpace> v4l2ToColorSpace = { > { V4L2_COLORSPACE_RAW, ColorSpace::Raw }, > - { V4L2_COLORSPACE_SRGB, ColorSpace::Srgb }, > + { V4L2_COLORSPACE_SRGB, { > + ColorSpace::Primaries::Rec709, > + ColorSpace::TransferFunction::Srgb, > + ColorSpace::YcbcrEncoding::Rec601, > + ColorSpace::Range::Limited } }, > { V4L2_COLORSPACE_JPEG, ColorSpace::Sycc }, > { V4L2_COLORSPACE_SMPTE170M, ColorSpace::Smpte170m }, > { V4L2_COLORSPACE_REC709, ColorSpace::Rec709 }, > @@ -772,7 +776,6 @@ static const std::map<uint32_t, ColorSpace::Range> v4l2ToRange = { > > static const std::vector<std::pair<ColorSpace, v4l2_colorspace>> colorSpaceToV4l2 = { > { ColorSpace::Raw, V4L2_COLORSPACE_RAW }, > - { ColorSpace::Srgb, V4L2_COLORSPACE_SRGB }, > { ColorSpace::Sycc, V4L2_COLORSPACE_JPEG }, > { ColorSpace::Smpte170m, V4L2_COLORSPACE_SMPTE170M }, > { ColorSpace::Rec709, V4L2_COLORSPACE_REC709 },
diff --git a/src/libcamera/color_space.cpp b/src/libcamera/color_space.cpp index f0d6109a..cb47acf9 100644 --- a/src/libcamera/color_space.cpp +++ b/src/libcamera/color_space.cpp @@ -47,7 +47,10 @@ namespace libcamera { * * Note that there is no guarantee of a 1:1 mapping between color space names * and definitions in libcamera and V4L2. A notable difference is that the sYCC - * libcamera color space is called JPEG in V4L2 due to historical reasons. + * libcamera color space is called JPEG in V4L2 due to historical reasons. On + * a similar note, the sRGB colorspace defined in the kernel assumes a Y'CbCr + * encoding to it which is not true. Hence the ColorSpace::sRGB is defined + * differently in libcamera (with no Y'CbCr encoding and full range). * * \todo Define the color space fully in the libcamera API to avoid referencing * V4L2 @@ -250,16 +253,13 @@ const ColorSpace ColorSpace::Raw = { }; /** - * \brief A constant representing the sRGB color space - * - * This is identical to the sYCC color space except that the Y'CbCr - * range is limited rather than full. + * \brief A constant representing the sRGB color space (RGB formats only) */ const ColorSpace ColorSpace::Srgb = { Primaries::Rec709, TransferFunction::Srgb, - YcbcrEncoding::Rec601, - Range::Limited + YcbcrEncoding::None, + Range::Full }; /** diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp index 1fb08b9d..a9e34ee1 100644 --- a/src/libcamera/v4l2_device.cpp +++ b/src/libcamera/v4l2_device.cpp @@ -746,7 +746,11 @@ void V4L2Device::eventAvailable() static const std::map<uint32_t, ColorSpace> v4l2ToColorSpace = { { V4L2_COLORSPACE_RAW, ColorSpace::Raw }, - { V4L2_COLORSPACE_SRGB, ColorSpace::Srgb }, + { V4L2_COLORSPACE_SRGB, { + ColorSpace::Primaries::Rec709, + ColorSpace::TransferFunction::Srgb, + ColorSpace::YcbcrEncoding::Rec601, + ColorSpace::Range::Limited } }, { V4L2_COLORSPACE_JPEG, ColorSpace::Sycc }, { V4L2_COLORSPACE_SMPTE170M, ColorSpace::Smpte170m }, { V4L2_COLORSPACE_REC709, ColorSpace::Rec709 }, @@ -772,7 +776,6 @@ static const std::map<uint32_t, ColorSpace::Range> v4l2ToRange = { static const std::vector<std::pair<ColorSpace, v4l2_colorspace>> colorSpaceToV4l2 = { { ColorSpace::Raw, V4L2_COLORSPACE_RAW }, - { ColorSpace::Srgb, V4L2_COLORSPACE_SRGB }, { ColorSpace::Sycc, V4L2_COLORSPACE_JPEG }, { ColorSpace::Smpte170m, V4L2_COLORSPACE_SMPTE170M }, { ColorSpace::Rec709, V4L2_COLORSPACE_REC709 },
Rectify the ColorSpace::Srgb to denote that it does not use any Y'Cbcr encoding and uses full range. The kernel on the other hand, recommends to use Rec601 as the encoding for V4L2_COLORSPACE_SRGB. It is not very explicit but it can be inferred that the kernel assumes V4L2_COLORSPACE_SRGB is a YUV-encoded one. However, when the data is in RGB, no encoding is required (and this is denoted by YcbcrEncoding::None in libcamera). Hence, to be clear on the libcamera colorspace API, rectify the ColorSpace::Srgb preset to use YcbcrEncoding::None. Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> --- src/libcamera/color_space.cpp | 14 +++++++------- src/libcamera/v4l2_device.cpp | 7 +++++-- 2 files changed, 12 insertions(+), 9 deletions(-)