[libcamera-devel] [PATCH 2/3] libcamera: colorspace: Add a default colorspace
Naushir Patuck
naush at raspberrypi.com
Mon Jul 25 17:40:44 CEST 2022
Hi all,
On Mon, 25 Jul 2022 at 13:22, Nicolas Dufresne via libcamera-devel <
libcamera-devel at lists.libcamera.org> wrote:
> 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.
>
Wouldn't the pipeline_handler::validate() take care of this? It ought to
know the
sensor output colourspace, and return that regardless of what the
application
may have requested. Apologies if I got this wrong.
Naush
>
> 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
> > >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20220725/3483e281/attachment.htm>
More information about the libcamera-devel
mailing list