[libcamera-devel] [PATCH v3 3/7] libcamera: Convert between ColorSpace class and V4L2 formats

David Plowman david.plowman at raspberrypi.com
Mon Oct 25 11:46:26 CEST 2021


Hi Naush

Thanks again for the review!

On Mon, 25 Oct 2021 at 09:04, Naushir Patuck <naush at raspberrypi.com> wrote:
>
> Hi David,
>
> Thank you for your work.
>
> On Wed, 20 Oct 2021 at 12:08, David Plowman <david.plowman at raspberrypi.com> wrote:
>>
>> These methods are added to the base V4L2Device class so that they can
>> be shared both by the video device class and subdevices.
>>
>> With the ColorSpace class, the color space and related other fields
>> are stored together, corresponding to a number of fields in the
>> various different V4L2 format structures. Template methods are
>> therefore a convenient implementation.
>>
>> Note that we must explicitly instantiate the templates that will be
>> needed.
>>
>> Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
>> ---
>>  include/libcamera/internal/v4l2_device.h |   7 +
>>  src/libcamera/v4l2_device.cpp            | 171 +++++++++++++++++++++++
>>  2 files changed, 178 insertions(+)
>>
>> diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h
>> index f21bc370..77ea30f6 100644
>> --- a/include/libcamera/internal/v4l2_device.h
>> +++ b/include/libcamera/internal/v4l2_device.h
>> @@ -17,6 +17,7 @@
>>  #include <libcamera/base/signal.h>
>>  #include <libcamera/base/span.h>
>>
>> +#include <libcamera/color_space.h>
>>  #include <libcamera/controls.h>
>>
>>  namespace libcamera {
>> @@ -44,6 +45,12 @@ public:
>>
>>         void updateControlInfo();
>>
>> +       template<typename T>
>> +       static ColorSpace v4l2ToColorSpace(const T &v4l2Format);
>> +
>> +       template<typename T>
>> +       static int colorSpaceToV4l2(const ColorSpace &colorSpace, T &v4l2Format);
>
>
> For consistency with other classes, perhaps these should be renamed to
> toColorSpace and fromColorSpace?

Yes, that sounds reasonable!

>
> I wonder if this implementation should also live in the ColorSpace class? This would
> sort of mirror what the various format classes do. But I have no strong preference
> either w

I think there might be a problem putting them in the ColorSpace class
because color_space.h needs to be a public header but the v4l2 ones (I
believe) are internal. Someone will correct me if I've got that wrong!

>
>> +
>>  protected:
>>         V4L2Device(const std::string &deviceNode);
>>         ~V4L2Device();
>> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
>> index 9c783c9c..771dc096 100644
>> --- a/src/libcamera/v4l2_device.cpp
>> +++ b/src/libcamera/v4l2_device.cpp
>> @@ -16,6 +16,8 @@
>>  #include <sys/syscall.h>
>>  #include <unistd.h>
>>
>> +#include <linux/v4l2-mediabus.h>
>> +
>>  #include <libcamera/base/event_notifier.h>
>>  #include <libcamera/base/log.h>
>>  #include <libcamera/base/utils.h>
>> @@ -731,4 +733,173 @@ void V4L2Device::eventAvailable()
>>         frameStart.emit(event.u.frame_sync.frame_sequence);
>>  }
>>
>> +static const std::map<uint32_t, ColorSpace> v4l2ToColorSpaceTable = {
>> +       { V4L2_COLORSPACE_RAW, ColorSpace::Raw },
>> +       { V4L2_COLORSPACE_JPEG, ColorSpace::Jpeg },
>> +       { V4L2_COLORSPACE_SRGB, ColorSpace::Jpeg },
>> +       { V4L2_COLORSPACE_SMPTE170M, ColorSpace::Smpte170m },
>> +       { V4L2_COLORSPACE_REC709, ColorSpace::Rec709 },
>> +       { V4L2_COLORSPACE_BT2020, ColorSpace::Rec2020 },
>> +};
>> +
>> +static const std::map<uint32_t, ColorSpace::YcbcrEncoding> v4l2ToYcbcrEncodingTable = {
>> +       { V4L2_YCBCR_ENC_601, ColorSpace::YcbcrEncoding::Rec601 },
>> +       { V4L2_YCBCR_ENC_709, ColorSpace::YcbcrEncoding::Rec709 },
>> +       { V4L2_YCBCR_ENC_BT2020, ColorSpace::YcbcrEncoding::Rec2020 },
>> +};
>> +
>> +static const std::map<uint32_t, ColorSpace::TransferFunction> v4l2ToTransferFunctionTable = {
>> +       { V4L2_XFER_FUNC_NONE, ColorSpace::TransferFunction::Linear },
>> +       { V4L2_XFER_FUNC_SRGB, ColorSpace::TransferFunction::Srgb },
>> +       { V4L2_XFER_FUNC_709, ColorSpace::TransferFunction::Rec709 },
>> +};
>> +
>> +static const std::map<uint32_t, ColorSpace::Range> v4l2ToRangeTable = {
>> +       { V4L2_QUANTIZATION_FULL_RANGE, ColorSpace::Range::Full },
>> +       { V4L2_QUANTIZATION_LIM_RANGE, ColorSpace::Range::Limited },
>> +};
>> +
>> +static const std::vector<std::pair<ColorSpace, v4l2_colorspace>> colorSpaceToV4l2Table = {
>> +       { ColorSpace::Raw, V4L2_COLORSPACE_RAW },
>> +       { ColorSpace::Jpeg, V4L2_COLORSPACE_JPEG },
>> +       { ColorSpace::Smpte170m, V4L2_COLORSPACE_SMPTE170M },
>> +       { ColorSpace::Rec709, V4L2_COLORSPACE_REC709 },
>> +       { ColorSpace::Rec2020, V4L2_COLORSPACE_BT2020 },
>> +};
>> +
>> +static const std::map<ColorSpace::Primaries, v4l2_colorspace> primariesToV4l2Table = {
>> +       { ColorSpace::Primaries::Raw, V4L2_COLORSPACE_RAW },
>> +       { ColorSpace::Primaries::Smpte170m, V4L2_COLORSPACE_SMPTE170M },
>> +       { ColorSpace::Primaries::Rec709, V4L2_COLORSPACE_REC709 },
>> +       { ColorSpace::Primaries::Rec2020, V4L2_COLORSPACE_BT2020 },
>> +};
>> +
>> +static const std::map<ColorSpace::YcbcrEncoding, v4l2_ycbcr_encoding> ycbcrEncodingToV4l2Table = {
>> +       { 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::TransferFunction, v4l2_xfer_func> transferFunctionToV4l2Table = {
>> +       { 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::Range, v4l2_quantization> rangeToV4l2Table = {
>> +       { ColorSpace::Range::Full, V4L2_QUANTIZATION_FULL_RANGE },
>> +       { ColorSpace::Range::Limited, V4L2_QUANTIZATION_LIM_RANGE },
>> +};
>> +
>> +/**
>> + * \brief Convert the color space fields in a V4L2 format to a ColorSpace
>> + * \param[in] v4l2Format A V4L2 format containing color space information
>> + *
>> + * The colorspace, ycbcr_enc, xfer_func and quantization fields within a
>> + * V4L2 format structure are converted to a corresponding ColorSpace.
>> + *
>> + * If any V4L2 fields are not recognised those are marked as undefined in
>> + * the ColorSpace, but other fields are still initialised where possible.
>> + * This situation can be detected using the returned value's
>> + * ColorSpace::isFullyDefined() method.
>> + *
>> + * \return The ColorSpace corresponding to the input V4L2 format
>> + */
>> +template<typename T>
>> +ColorSpace V4L2Device::v4l2ToColorSpace(const T &v4l2Format)
>> +{
>> +       ColorSpace colorSpace;
>> +
>> +       auto itColor = v4l2ToColorSpaceTable.find(v4l2Format.colorspace);
>> +       if (itColor != v4l2ToColorSpaceTable.end())
>> +               colorSpace = itColor->second;
>> +
>> +       auto itYcbcrEncoding = v4l2ToYcbcrEncodingTable.find(v4l2Format.ycbcr_enc);
>> +       if (itYcbcrEncoding != v4l2ToYcbcrEncodingTable.end())
>> +               colorSpace.ycbcrEncoding = itYcbcrEncoding->second;
>> +
>> +       auto itTransfer = v4l2ToTransferFunctionTable.find(v4l2Format.xfer_func);
>> +       if (itTransfer != v4l2ToTransferFunctionTable.end())
>> +               colorSpace.transferFunction = itTransfer->second;
>> +
>> +       auto itRange = v4l2ToRangeTable.find(v4l2Format.quantization);
>> +       if (itRange != v4l2ToRangeTable.end())
>> +               colorSpace.range = itRange->second;
>> +
>> +       return colorSpace;
>> +}
>> +
>> +template ColorSpace V4L2Device::v4l2ToColorSpace(const struct v4l2_pix_format &);
>> +template ColorSpace V4L2Device::v4l2ToColorSpace(const struct v4l2_pix_format_mplane &);
>> +template ColorSpace V4L2Device::v4l2ToColorSpace(const struct v4l2_mbus_framefmt &);
>> +
>> +/**
>> + * \brief Fill in the color space fields of a V4L2 format from a ColorSpace
>> + * \param[in] colorSpace The ColorSpace to be converted
>> + * \param[out] v4l2Format A V4L2 format containing color space information
>> + *
>> + * The colorspace, ycbcr_enc, xfer_func and quantization fields within a
>> + * V4L2 format structure are filled in from a corresponding ColorSpace.
>> + *
>> + * An error is returned if any of the V4L2 fields do not support the
>> + * value given in the ColorSpace. Such fields are set to the V4L2
>> + * "default" values, but all other fields are still filled in where
>> + * possible.
>> + *
>> + * \return 0 on success or a negative error code otherwise
>> + */
>> +template<typename T>
>> +int V4L2Device::colorSpaceToV4l2(const ColorSpace &colorSpace, T &v4l2Format)
>> +{
>> +       int ret = 0;
>> +
>> +       v4l2Format.colorspace = V4L2_COLORSPACE_DEFAULT;
>> +       v4l2Format.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
>> +       v4l2Format.xfer_func = V4L2_XFER_FUNC_DEFAULT;
>> +       v4l2Format.quantization = V4L2_QUANTIZATION_DEFAULT;
>> +
>> +       auto itColor = std::find_if(colorSpaceToV4l2Table.begin(), colorSpaceToV4l2Table.end(),
>> +                                   [&colorSpace](const auto &item) {
>> +                                           return colorSpace == item.first;
>> +                                   });
>
>
> You can use std::map::find() here, to cut down on the typing :)

After running those little experiments (as per my previous message),
my inclination is slightly, though not strongly, against inventing an
artificial ordering just to use a map when the tables are very small.
Though one would revisit that if we ever get to "several dozen" colour
spaces.

Thanks!
David

>
>>
>> +       if (itColor != colorSpaceToV4l2Table.end()) {
>> +               v4l2Format.colorspace = itColor->second;
>> +               return ret;
>> +       }
>> +
>> +       /*
>> +        * If the colorSpace doesn't precisely match a standard color space,
>> +        * then we must choose a V4L2 colorspace with matching primaries.
>> +        */
>> +       auto itPrimaries = primariesToV4l2Table.find(colorSpace.primaries);
>> +       if (itPrimaries != primariesToV4l2Table.end())
>> +               v4l2Format.colorspace = itPrimaries->second;
>> +       else
>> +               ret = -1;
>> +
>> +       auto itYcbcrEncoding = ycbcrEncodingToV4l2Table.find(colorSpace.ycbcrEncoding);
>> +       if (itYcbcrEncoding != ycbcrEncodingToV4l2Table.end())
>> +               v4l2Format.ycbcr_enc = itYcbcrEncoding->second;
>> +       else
>> +               ret = -1;
>> +
>> +       auto itTransfer = transferFunctionToV4l2Table.find(colorSpace.transferFunction);
>> +       if (itTransfer != transferFunctionToV4l2Table.end())
>> +               v4l2Format.xfer_func = itTransfer->second;
>> +       else
>> +               ret = -1;
>> +
>> +       auto itRange = rangeToV4l2Table.find(colorSpace.range);
>> +       if (itRange != rangeToV4l2Table.end())
>> +               v4l2Format.quantization = itRange->second;
>> +       else
>> +               ret = -1;
>> +
>> +       return ret;
>> +}
>> +
>> +template int V4L2Device::colorSpaceToV4l2(const ColorSpace &, struct v4l2_pix_format &);
>> +template int V4L2Device::colorSpaceToV4l2(const ColorSpace &, struct v4l2_pix_format_mplane &);
>> +template int V4L2Device::colorSpaceToV4l2(const ColorSpace &, struct v4l2_mbus_framefmt &);
>> +
>>  } /* namespace libcamera */
>> --
>> 2.20.1
>>


More information about the libcamera-devel mailing list