[libcamera-devel] [PATCH 2/3] libcamera: colorspace: Add a default colorspace
Nicolas Dufresne
nicolas.dufresne at collabora.com
Mon Jul 25 14:21:49 CEST 2022
Hi David,
Le lundi 25 juillet 2022 à 11:46 +0100, David Plowman a écrit :
> 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.
"wrong" application is a bit of an over statement. Even for cameras, the
colorspace is not always something that can be freely picked by the application.
The V4L2 driver, or the HW may have some limitations. A simple example is UVC,
you pretty much get what you get and you can't influence it. For this reason, a
proper colorspace API needs to work both ways. Which basically imply, an
application should be able to not care about selecting / influencing the
colorspace and just read whatever the driver/HW produces.
regards,
Nicolas
>
> 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