[libcamera-devel] [PATCH 2/3] libcamera: colorspace: Add a default colorspace
David Plowman
david.plowman at raspberrypi.com
Mon Jul 25 12:46:46 CEST 2022
Hi Umang
Thanks for this patch. I was wondering if I could ask for a little
more information on how it would be used.
In the original libcamera ColorSpace implementation I was very keen to
avoid "unknown" or "default" values inside ColorSpaces. I was taking
the view that applications really should say what ColorSpace they
want, but at the same time I knew there will always be (bad)
applications that don't think about colour spaces and so I let them
use std::optional to allow the ColorSpace to be completely
unspecified.
In V4L2, my understanding is that "_DEFAULT" normally means that you
infer the value from the overall V4L2_COLORSPACE (someone please
correct me if that's wrong!). In libcamera we don't have an "overall
colour space" in this way, so what would it then mean? For example, is
"ColorSpace::Range::Default" ever the same as
"ColorSpace::Range::Full", or is it always different? How do we (can
we even) document what it means?
I'd be interested to hear what other folks think. Possibly I'm being a
bit paranoid, but then I have been bitten by "wrong colour space"
problems often enough in the past (usually just before a product
ships!).
Thanks!
David
On Sun, 24 Jul 2022 at 15:44, Umang Jain via libcamera-devel
<libcamera-devel at lists.libcamera.org> wrote:
>
> Add a Colorspace::Default which corresponds to default V4L2
> colorspace identifiers.
>
> \todo Add defaults to python colorspace bindings
>
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> ---
> include/libcamera/color_space.h | 5 +++++
> src/libcamera/color_space.cpp | 29 ++++++++++++++++++++++++++++-
> src/libcamera/v4l2_device.cpp | 5 +++++
> 3 files changed, 38 insertions(+), 1 deletion(-)
>
> diff --git a/include/libcamera/color_space.h b/include/libcamera/color_space.h
> index 086c56c1..0ad8da17 100644
> --- a/include/libcamera/color_space.h
> +++ b/include/libcamera/color_space.h
> @@ -16,6 +16,7 @@ class ColorSpace
> {
> public:
> enum class Primaries {
> + Default,
> Raw,
> Smpte170m,
> Rec709,
> @@ -23,12 +24,14 @@ public:
> };
>
> enum class TransferFunction {
> + Default,
> Linear,
> Srgb,
> Rec709,
> };
>
> enum class YcbcrEncoding {
> + Default,
> None,
> Rec601,
> Rec709,
> @@ -36,6 +39,7 @@ public:
> };
>
> enum class Range {
> + Default,
> Full,
> Limited,
> };
> @@ -45,6 +49,7 @@ public:
> {
> }
>
> + static const ColorSpace Default;
> static const ColorSpace Raw;
> static const ColorSpace Jpeg;
> static const ColorSpace Srgb;
> diff --git a/src/libcamera/color_space.cpp b/src/libcamera/color_space.cpp
> index 895e5c8e..d52f58cf 100644
> --- a/src/libcamera/color_space.cpp
> +++ b/src/libcamera/color_space.cpp
> @@ -50,6 +50,9 @@ namespace libcamera {
> * \enum ColorSpace::Primaries
> * \brief The color primaries for this color space
> *
> + * \var ColorSpace::Primaries::Default
> + * \brief Use the default primaries as defined by the driver
> + *
> * \var ColorSpace::Primaries::Raw
> * \brief These are raw colors directly from a sensor, the primaries are
> * unspecified
> @@ -68,6 +71,9 @@ namespace libcamera {
> * \enum ColorSpace::TransferFunction
> * \brief The transfer function used for this color space
> *
> + * \var ColorSpace::TransferFunction::Default
> + * \brief Use the default transfer function as defined by the driver
> + *
> * \var ColorSpace::TransferFunction::Linear
> * \brief This color space uses a linear (identity) transfer function
> *
> @@ -82,6 +88,9 @@ namespace libcamera {
> * \enum ColorSpace::YcbcrEncoding
> * \brief The Y'CbCr encoding
> *
> + * \var ColorSpace::YcbcrEncoding::Default
> + * \brief Use the default Y'CbCr encoding as defined by the driver
> + *
> * \var ColorSpace::YcbcrEncoding::None
> * \brief There is no defined Y'CbCr encoding (used for non-YUV formats)
> *
> @@ -99,6 +108,9 @@ namespace libcamera {
> * \enum ColorSpace::Range
> * \brief The range (sometimes "quantisation") for this color space
> *
> + * \var ColorSpace::Range::Default
> + * Use the default range as defined by the driver
> + *
> * \var ColorSpace::Range::Full
> * \brief This color space uses full range pixel values
> *
> @@ -132,7 +144,8 @@ std::string ColorSpace::toString() const
> {
> /* Print out a brief name only for standard color spaces. */
>
> - static const std::array<std::pair<ColorSpace, const char *>, 6> colorSpaceNames = { {
> + static const std::array<std::pair<ColorSpace, const char *>, 7> colorSpaceNames = { {
> + { ColorSpace::Default, "Default" },
> { ColorSpace::Raw, "RAW" },
> { ColorSpace::Jpeg, "JPEG" },
> { ColorSpace::Srgb, "sRGB" },
> @@ -150,23 +163,27 @@ std::string ColorSpace::toString() const
> /* Assemble a name made of the constituent fields. */
>
> static const std::map<Primaries, std::string> primariesNames = {
> + { Primaries::Default, "Default" },
> { Primaries::Raw, "RAW" },
> { Primaries::Smpte170m, "SMPTE170M" },
> { Primaries::Rec709, "Rec709" },
> { Primaries::Rec2020, "Rec2020" },
> };
> static const std::map<TransferFunction, std::string> transferNames = {
> + { TransferFunction::Default, "Default" },
> { TransferFunction::Linear, "Linear" },
> { TransferFunction::Srgb, "sRGB" },
> { TransferFunction::Rec709, "Rec709" },
> };
> static const std::map<YcbcrEncoding, std::string> encodingNames = {
> + { YcbcrEncoding::Default, "Default" },
> { YcbcrEncoding::None, "None" },
> { YcbcrEncoding::Rec601, "Rec601" },
> { YcbcrEncoding::Rec709, "Rec709" },
> { YcbcrEncoding::Rec2020, "Rec2020" },
> };
> static const std::map<Range, std::string> rangeNames = {
> + { Range::Default, "Default" },
> { Range::Full, "Full" },
> { Range::Limited, "Limited" },
> };
> @@ -232,6 +249,16 @@ std::string ColorSpace::toString(const std::optional<ColorSpace> &colorSpace)
> * \brief The pixel range used with by color space
> */
>
> +/**
> + * \brief A constant representing a default color space
> + */
> +const ColorSpace ColorSpace::Default = {
> + Primaries::Default,
> + TransferFunction::Default,
> + YcbcrEncoding::Default,
> + Range::Default
> +};
> +
> /**
> * \brief A constant representing a raw color space (from a sensor)
> */
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> index 3fc8438f..ecfcf337 100644
> --- a/src/libcamera/v4l2_device.cpp
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -770,6 +770,7 @@ static const std::map<uint32_t, ColorSpace::Range> v4l2ToRange = {
> };
>
> static const std::vector<std::pair<ColorSpace, v4l2_colorspace>> colorSpaceToV4l2 = {
> + { ColorSpace::Default, V4L2_COLORSPACE_DEFAULT },
> { ColorSpace::Raw, V4L2_COLORSPACE_RAW },
> { ColorSpace::Jpeg, V4L2_COLORSPACE_JPEG },
> { ColorSpace::Srgb, V4L2_COLORSPACE_SRGB },
> @@ -779,6 +780,7 @@ static const std::vector<std::pair<ColorSpace, v4l2_colorspace>> colorSpaceToV4l
> };
>
> static const std::map<ColorSpace::Primaries, v4l2_colorspace> primariesToV4l2 = {
> + { ColorSpace::Primaries::Default, V4L2_COLORSPACE_DEFAULT },
> { ColorSpace::Primaries::Raw, V4L2_COLORSPACE_RAW },
> { ColorSpace::Primaries::Smpte170m, V4L2_COLORSPACE_SMPTE170M },
> { ColorSpace::Primaries::Rec709, V4L2_COLORSPACE_REC709 },
> @@ -786,18 +788,21 @@ static const std::map<ColorSpace::Primaries, v4l2_colorspace> primariesToV4l2 =
> };
>
> static const std::map<ColorSpace::TransferFunction, v4l2_xfer_func> transferFunctionToV4l2 = {
> + { ColorSpace::TransferFunction::Default, V4L2_XFER_FUNC_DEFAULT },
> { ColorSpace::TransferFunction::Linear, V4L2_XFER_FUNC_NONE },
> { ColorSpace::TransferFunction::Srgb, V4L2_XFER_FUNC_SRGB },
> { ColorSpace::TransferFunction::Rec709, V4L2_XFER_FUNC_709 },
> };
>
> static const std::map<ColorSpace::YcbcrEncoding, v4l2_ycbcr_encoding> ycbcrEncodingToV4l2 = {
> + { ColorSpace::YcbcrEncoding::Default, V4L2_YCBCR_ENC_DEFAULT },
> { ColorSpace::YcbcrEncoding::Rec601, V4L2_YCBCR_ENC_601 },
> { ColorSpace::YcbcrEncoding::Rec709, V4L2_YCBCR_ENC_709 },
> { ColorSpace::YcbcrEncoding::Rec2020, V4L2_YCBCR_ENC_BT2020 },
> };
>
> static const std::map<ColorSpace::Range, v4l2_quantization> rangeToV4l2 = {
> + { ColorSpace::Range::Default, V4L2_QUANTIZATION_DEFAULT },
> { ColorSpace::Range::Full, V4L2_QUANTIZATION_FULL_RANGE },
> { ColorSpace::Range::Limited, V4L2_QUANTIZATION_LIM_RANGE },
> };
> --
> 2.31.1
>
More information about the libcamera-devel
mailing list