Message ID | 20220823114551.27796-1-laurent.pinchart@ideasonboard.com |
---|---|
State | Accepted |
Headers | show |
Series |
|
Related | show |
On Tue, Aug 23, 2022 at 02:45:51PM +0300, Laurent Pinchart via libcamera-devel 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. This is > confirmed by ITU-T T.871: > > 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'Cb, and E'Cr 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. > > 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@ideasonboard.com> > Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > --- > Changes since v1: > > - Rename more instances of JPEG to sYCC > - Reword the references to the V4L2 documentation > --- > include/libcamera/color_space.h | 2 +- > src/libcamera/color_space.cpp | 43 +++++++++++-------- > .../pipeline/raspberrypi/raspberrypi.cpp | 10 ++--- > src/libcamera/v4l2_device.cpp | 4 +- > src/py/libcamera/py_main.cpp | 2 +- > 5 files changed, 34 insertions(+), 27 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..f0d6109ac4fb 100644 > --- a/src/libcamera/color_space.cpp > +++ b/src/libcamera/color_space.cpp > @@ -29,21 +29,28 @@ namespace libcamera { > * (sometimes also referred to as the quantisation) of the color space. > * > * Certain combinations of these fields form well-known standard color > - * spaces such as "JPEG" or "REC709". > + * spaces such as "sRGB" or "Rec709". > * > * In the strictest sense a "color space" formally only refers to the > * color primaries and white point. Here, however, the ColorSpace class > * adopts the common broader usage that includes the transfer function, > * Y'CbCr encoding method and quantisation. > * > - * For more information on the specific color spaces described here, please > - * see: > + * More information on color spaces is available in the V4L2 documentation, see > + * in particular > * > * - <a href="https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/colorspaces-details.html#col-srgb">sRGB</a> > * - <a href="https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/colorspaces-details.html#col-jpeg">JPEG</a> > * - <a href="https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/colorspaces-details.html#col-smpte-170m">SMPTE 170M</a> > * - <a href="https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/colorspaces-details.html#col-rec709">Rec.709</a> > * - <a href="https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/colorspaces-details.html#col-bt2020">Rec.2020</a> > + * > + * 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. > + * > + * \todo Define the color space fully in the libcamera API to avoid referencing > + * V4L2 > */ > > /** > @@ -121,8 +128,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 +141,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,21 +249,10 @@ 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 > * > - * This is identical to the JPEG color space except that the Y'CbCr > + * This is identical to the sYCC color space except that the Y'CbCr > * range is limited rather than full. > */ > const ColorSpace ColorSpace::Srgb = { > @@ -266,6 +262,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 7e613a3f2a5a..75947889f7b1 100644 > --- a/src/py/libcamera/py_main.cpp > +++ b/src/py/libcamera/py_main.cpp > @@ -463,8 +463,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; });
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..f0d6109ac4fb 100644 --- a/src/libcamera/color_space.cpp +++ b/src/libcamera/color_space.cpp @@ -29,21 +29,28 @@ namespace libcamera { * (sometimes also referred to as the quantisation) of the color space. * * Certain combinations of these fields form well-known standard color - * spaces such as "JPEG" or "REC709". + * spaces such as "sRGB" or "Rec709". * * In the strictest sense a "color space" formally only refers to the * color primaries and white point. Here, however, the ColorSpace class * adopts the common broader usage that includes the transfer function, * Y'CbCr encoding method and quantisation. * - * For more information on the specific color spaces described here, please - * see: + * More information on color spaces is available in the V4L2 documentation, see + * in particular * * - <a href="https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/colorspaces-details.html#col-srgb">sRGB</a> * - <a href="https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/colorspaces-details.html#col-jpeg">JPEG</a> * - <a href="https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/colorspaces-details.html#col-smpte-170m">SMPTE 170M</a> * - <a href="https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/colorspaces-details.html#col-rec709">Rec.709</a> * - <a href="https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/colorspaces-details.html#col-bt2020">Rec.2020</a> + * + * 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. + * + * \todo Define the color space fully in the libcamera API to avoid referencing + * V4L2 */ /** @@ -121,8 +128,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 +141,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,21 +249,10 @@ 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 * - * This is identical to the JPEG color space except that the Y'CbCr + * This is identical to the sYCC color space except that the Y'CbCr * range is limited rather than full. */ const ColorSpace ColorSpace::Srgb = { @@ -266,6 +262,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 7e613a3f2a5a..75947889f7b1 100644 --- a/src/py/libcamera/py_main.cpp +++ b/src/py/libcamera/py_main.cpp @@ -463,8 +463,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; });