[libcamera-devel] [PATCH] libcamera: color_space: Rename Jpeg to Sycc
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sun Aug 21 22:28:00 CEST 2022
Hi Hans,
On Fri, Aug 19, 2022 at 11:12:56AM +0200, Hans Verkuil wrote:
> On 8/17/22 23:18, Laurent Pinchart wrote:
> > The JPEG color space is badly name, as the JPEG specification (ITU-T
> > T.81) doesn't define any particular color space:
> >
> > The interchange format does not specify a complete coded image
> > representation. Application-dependent information, e.g. colour
> > space, is outside the scope of this Specification.
> >
> > The JFIF specification (ITU-T T.871) is clearer as it requires ITU-R
> > BT.601 YCbCr encoding and a full quantization range:
> >
> > The interpretations of Y, CB, and CR are derived from the E'Y, E'Cb,
> > and E'Cr signals defined in the 625-line specification of Rec. ITU-R
> > BT.601, but these signals are normalized so as to permit the usage of
> > the full range of 256 levels of the 8-bit binary encoding of the Y
> > component.
> >
> > It however doesn't specify color primaries or a transfer function
> > explicitly. It only mentions the latter when describing the conversion
> > from YCbCr to RGB:
> >
> > The inverse relationship for computing full scale 8-bit per colour
> > channel gamma pre-corrected RGB values (following Rec. ITU-R BT.601
> > gamma pre-correction and colour primary specifications) from YCbCr
> > colours (with 256 levels per component) can be computed as follows:
> > [...]
> >
> > Given that ITU-R BT.601-5 (1995) didn't specify color primaries or a
> > transfer function, and that the later ITU-R BT.601-7 (2011) version
> > specifies color primaries for the 625-line variant that do not match
> > sRGB, the JPEG color space in libcamera is badly named.
> >
> > Rename the color space to sYCC, as its definition matches the sYCC
> > standard, and indicate that it is typically used to encode JPEG images.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >
> > Hans, following our discussion on IRC, I double-check the JFIF standard,
> > and my findings are described in the commit message above. It seems that
> > naming the color space "JPEG" in V4L2 is actually incorrect, as JFIF
> > expects the 626-line ITU-R BT.601 chromaticities for the primary colors,
> > which are slightly different than sRGB (the 525-line chromaticities
> > match SMPTE 170M and SMPTE 240M, but not sRGB either). It seems that we
> > are missing a V4L2_COLORSPACE_* value to describe the 625-line ITU-R
> > BT.601 chromaticities, is that an oversight ?
>
> We have that colorspace: V4L2_COLORSPACE_470_SYSTEM_BG, however modern
Indeed, my bad.
> SDTV transmissions use V4L2_COLORSPACE_SMPTE170M these days.
>
> The differences between the two are next to impossible to see.
>
> However, I think you misinterpreted the JFIF standard: it does not specify
> that the BT 601 chromaticities/transfer function should be used, it only
> uses the YCbCr-RGB conversion that BT.601 describes.
>
> Note 3 in section 7 of JFIF is relevant here:
>
> "NOTE 3 − As this Recommendation | International Standard is based on the prior informally-circulated JFIF version 1.02
> specification that was produced in 1992, which referenced Rec. ITU-R BT.601 (formerly CCIR 601), it references that
> specification for definition of the E' Y, E' , and E' signals that correspond to the YCBCR values specified herein. However,
> since the development of the prior JFIF version 1.02 specification, additional industry specifications have been developed, Rec.
> ITU-R BT.601 has been updated, and common industry practice has emerged which often follows the sYCC specification in IEC
> 61966-2-1/Amd.1. The difference between the use of the colour interpretation specification in this Recommendation |
> International Standard and that of the sYCC specification may be considered negligible in practice. Moreover, as previously
> noted, the colour space specification herein can provide only a basic level of colour fidelity. The use of supplemental metadata
> such as an ICC profile (e.g., as specified in ISO 15076-1) may be necessary to provide a more accurate colour characterization."
>
> In other words, it recommends that sYCC is followed as the default.
>
> Note that I believe that the V4L2_COLORSPACE_JPEG define predates the sYCC standard,
> so there was a good reason why SYCC wasn't used. :-)
That's possible, I'll stop short of researching this :-)
> I wonder if it would make sense to make a V4L2_COLORSPACE_SYCC as well. Probably
> more effort than it is worth, though.
For V4L2 I think it's likely more effort than it's worth, yes. For
libcamera we'll use sYCC.
> > ---
> > include/libcamera/color_space.h | 2 +-
> > src/libcamera/color_space.cpp | 28 +++++++++----------
> > .../pipeline/raspberrypi/raspberrypi.cpp | 10 +++----
> > src/libcamera/v4l2_device.cpp | 4 +--
> > src/py/libcamera/py_main.cpp | 2 +-
> > 5 files changed, 23 insertions(+), 23 deletions(-)
> >
> > diff --git a/include/libcamera/color_space.h b/include/libcamera/color_space.h
> > index 0d39fbc02220..8030a264c66f 100644
> > --- a/include/libcamera/color_space.h
> > +++ b/include/libcamera/color_space.h
> > @@ -46,8 +46,8 @@ public:
> > }
> >
> > static const ColorSpace Raw;
> > - static const ColorSpace Jpeg;
> > static const ColorSpace Srgb;
> > + static const ColorSpace Sycc;
> > static const ColorSpace Smpte170m;
> > static const ColorSpace Rec709;
> > static const ColorSpace Rec2020;
> > diff --git a/src/libcamera/color_space.cpp b/src/libcamera/color_space.cpp
> > index caf397607b10..6ace40dcb0fb 100644
> > --- a/src/libcamera/color_space.cpp
> > +++ b/src/libcamera/color_space.cpp
> > @@ -121,8 +121,8 @@ namespace libcamera {
> > * \brief Assemble and return a readable string representation of the
> > * ColorSpace
> > *
> > - * If the color space matches a standard ColorSpace (such as ColorSpace::Jpeg)
> > - * then the short name of the color space ("JPEG") is returned. Otherwise
> > + * If the color space matches a standard ColorSpace (such as ColorSpace::Sycc)
> > + * then the short name of the color space ("sYCC") is returned. Otherwise
> > * the four constituent parts of the ColorSpace are assembled into a longer
> > * string.
> > *
> > @@ -134,8 +134,8 @@ std::string ColorSpace::toString() const
> >
> > static const std::array<std::pair<ColorSpace, const char *>, 6> colorSpaceNames = { {
> > { ColorSpace::Raw, "RAW" },
> > - { ColorSpace::Jpeg, "JPEG" },
> > { ColorSpace::Srgb, "sRGB" },
> > + { ColorSpace::Sycc, "sYCC" },
> > { ColorSpace::Smpte170m, "SMPTE170M" },
> > { ColorSpace::Rec709, "Rec709" },
> > { ColorSpace::Rec2020, "Rec2020" },
> > @@ -242,17 +242,6 @@ const ColorSpace ColorSpace::Raw = {
> > Range::Full
> > };
> >
> > -/**
> > - * \brief A constant representing the JPEG color space used for
> > - * encoding JPEG images
> > - */
> > -const ColorSpace ColorSpace::Jpeg = {
> > - Primaries::Rec709,
> > - TransferFunction::Srgb,
> > - YcbcrEncoding::Rec601,
> > - Range::Full
> > -};
> > -
> > /**
> > * \brief A constant representing the sRGB color space
> > *
> > @@ -266,6 +255,17 @@ const ColorSpace ColorSpace::Srgb = {
> > Range::Limited
> > };
> >
> > +/**
> > + * \brief A constant representing the sYCC color space, typically used for
> > + * encoding JPEG images
> > + */
> > +const ColorSpace ColorSpace::Sycc = {
> > + Primaries::Rec709,
> > + TransferFunction::Srgb,
> > + YcbcrEncoding::Rec601,
> > + Range::Full
> > +};
> > +
> > /**
> > * \brief A constant representing the SMPTE170M color space
> > */
> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > index e895584d4fbc..b4094898ca6c 100644
> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > @@ -593,11 +593,11 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,
> > fmts = data->isp_[Isp::Output0].dev()->formats();
> > pixelFormat = formats::NV12;
> > /*
> > - * Still image codecs usually expect the JPEG color space.
> > + * Still image codecs usually expect the sYCC color space.
> > * Even RGB codecs will be fine as the RGB we get with the
> > - * JPEG color space is the same as sRGB.
> > + * sYCC color space is the same as sRGB.
> > */
> > - colorSpace = ColorSpace::Jpeg;
> > + colorSpace = ColorSpace::Sycc;
> > /* Return the largest sensor resolution. */
> > size = sensorSize;
> > bufferCount = 1;
> > @@ -628,7 +628,7 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,
> > case StreamRole::Viewfinder:
> > fmts = data->isp_[Isp::Output0].dev()->formats();
> > pixelFormat = formats::ARGB8888;
> > - colorSpace = ColorSpace::Jpeg;
> > + colorSpace = ColorSpace::Sycc;
> > size = { 800, 600 };
> > bufferCount = 4;
> > outCount++;
> > @@ -835,7 +835,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
> > format.size = maxSize;
> > format.fourcc = dev->toV4L2PixelFormat(formats::YUV420);
> > /* No one asked for output, so the color space doesn't matter. */
> > - format.colorSpace = ColorSpace::Jpeg;
> > + format.colorSpace = ColorSpace::Sycc;
> > ret = dev->setFormat(&format);
> > if (ret) {
> > LOG(RPI, Error)
> > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> > index 3fc8438f6579..b22a981ff1a6 100644
> > --- a/src/libcamera/v4l2_device.cpp
> > +++ b/src/libcamera/v4l2_device.cpp
> > @@ -745,8 +745,8 @@ void V4L2Device::eventAvailable()
> >
> > static const std::map<uint32_t, ColorSpace> v4l2ToColorSpace = {
> > { V4L2_COLORSPACE_RAW, ColorSpace::Raw },
> > - { V4L2_COLORSPACE_JPEG, ColorSpace::Jpeg },
> > { V4L2_COLORSPACE_SRGB, ColorSpace::Srgb },
> > + { V4L2_COLORSPACE_JPEG, ColorSpace::Sycc },
> > { V4L2_COLORSPACE_SMPTE170M, ColorSpace::Smpte170m },
> > { V4L2_COLORSPACE_REC709, ColorSpace::Rec709 },
> > { V4L2_COLORSPACE_BT2020, ColorSpace::Rec2020 },
> > @@ -771,8 +771,8 @@ 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::Jpeg, V4L2_COLORSPACE_JPEG },
> > { ColorSpace::Srgb, V4L2_COLORSPACE_SRGB },
> > + { ColorSpace::Sycc, V4L2_COLORSPACE_JPEG },
> > { ColorSpace::Smpte170m, V4L2_COLORSPACE_SMPTE170M },
> > { ColorSpace::Rec709, V4L2_COLORSPACE_REC709 },
> > { ColorSpace::Rec2020, V4L2_COLORSPACE_BT2020 },
> > diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp
> > index 505cc3dc1a0d..2a58e34a7cdb 100644
> > --- a/src/py/libcamera/py_main.cpp
> > +++ b/src/py/libcamera/py_main.cpp
> > @@ -602,8 +602,8 @@ PYBIND11_MODULE(_libcamera, m)
> > .def_readwrite("ycbcrEncoding", &ColorSpace::ycbcrEncoding)
> > .def_readwrite("range", &ColorSpace::range)
> > .def_static("Raw", []() { return ColorSpace::Raw; })
> > - .def_static("Jpeg", []() { return ColorSpace::Jpeg; })
> > .def_static("Srgb", []() { return ColorSpace::Srgb; })
> > + .def_static("Sycc", []() { return ColorSpace::Sycc; })
> > .def_static("Smpte170m", []() { return ColorSpace::Smpte170m; })
> > .def_static("Rec709", []() { return ColorSpace::Rec709; })
> > .def_static("Rec2020", []() { return ColorSpace::Rec2020; });
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list