[libcamera-devel] [PATCH] libcamera: v4l2_subdevice: Silence warning for unknown metadata formats

[Naushir Patuck]

Hi Laurent,

Thank you for this fix.

On Mon, 5 Sept 2022 at 17:46, Laurent Pinchart <
laurent.pinchart at ideasonboard.com> wrote:

> Commit e297673e7686 ("libcamera: v4l2_device: Adjust colorspace based on
> pixel format") has introduced a warning when trying to convert a color
> space from V4L2 to libcamera if the media bus code is unknown. This was
> meant to catch unknown image formats, but turned out to be also
> triggered for metadata formats.
>
> Color spaces are not applicable to metadata formats, there should thus
> be no warning. Fix it by skipping the color space translation and
> returning std::nullopt directly if the kernel reports
> V4L2_COLORSPACE_DEFAULT. This doesn't introduce any change in behaviour
> other than getting rid of the warning, as the V4L2Device::toColorSpace()
> function returns std::nullopt alread in that case.
>
> Fixes: e297673e7686 ("libcamera: v4l2_device: Adjust colorspace based on
> pixel format")
> Reported-by: Naushir Patuck <naush at raspberrypi.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>

This does indeed remove the WARN message that I highlighted.

Tested-by: Naushir Patuck <naush at raspberrypi.com>
Reviewed-by: Naushir Patuck <naush at raspberrypi.com>

---
>  include/libcamera/internal/v4l2_subdevice.h |  3 ++
>  src/libcamera/v4l2_subdevice.cpp            | 51 ++++++++++-----------
>  2 files changed, 26 insertions(+), 28 deletions(-)
>
> diff --git a/include/libcamera/internal/v4l2_subdevice.h
> b/include/libcamera/internal/v4l2_subdevice.h
> index 00be17bb1465..69862de0585a 100644
> --- a/include/libcamera/internal/v4l2_subdevice.h
> +++ b/include/libcamera/internal/v4l2_subdevice.h
> @@ -101,6 +101,9 @@ protected:
>  private:
>         LIBCAMERA_DISABLE_COPY(V4L2Subdevice)
>
> +       std::optional<ColorSpace>
> +       toColorSpace(const v4l2_mbus_framefmt &format) const;
> +
>         std::vector<unsigned int> enumPadCodes(unsigned int pad);
>         std::vector<SizeRange> enumPadSizes(unsigned int pad,
>                                             unsigned int code);
> diff --git a/src/libcamera/v4l2_subdevice.cpp
> b/src/libcamera/v4l2_subdevice.cpp
> index 95bfde340d8c..f3a9a0096c6c 100644
> --- a/src/libcamera/v4l2_subdevice.cpp
> +++ b/src/libcamera/v4l2_subdevice.cpp
> @@ -477,6 +477,27 @@ V4L2Subdevice::Formats
> V4L2Subdevice::formats(unsigned int pad)
>         return formats;
>  }
>
> +std::optional<ColorSpace> V4L2Subdevice::toColorSpace(const
> v4l2_mbus_framefmt &format) const
> +{
> +       if (format.colorspace == V4L2_COLORSPACE_DEFAULT)
> +               return std::nullopt;
> +
> +       PixelFormatInfo::ColourEncoding colourEncoding;
> +       auto iter = formatInfoMap.find(format.code);
> +       if (iter != formatInfoMap.end()) {
> +               colourEncoding = iter->second.colourEncoding;
> +       } else {
> +               LOG(V4L2, Warning)
> +                       << "Unknown subdev format "
> +                       << utils::hex(format.code, 4)
> +                       << ", defaulting to RGB encoding";
> +
> +               colourEncoding = PixelFormatInfo::ColourEncodingRGB;
> +       }
> +
> +       return V4L2Device::toColorSpace(format, colourEncoding);
> +}
> +
>  /**
>   * \brief Retrieve the image format set on one of the V4L2 subdevice pads
>   * \param[in] pad The 0-indexed pad number the format is to be retrieved
> from
> @@ -503,20 +524,7 @@ int V4L2Subdevice::getFormat(unsigned int pad,
> V4L2SubdeviceFormat *format,
>         format->size.width = subdevFmt.format.width;
>         format->size.height = subdevFmt.format.height;
>         format->mbus_code = subdevFmt.format.code;
> -
> -       PixelFormatInfo::ColourEncoding colourEncoding;
> -       auto iter = formatInfoMap.find(subdevFmt.format.code);
> -       if (iter != formatInfoMap.end()) {
> -               colourEncoding = iter->second.colourEncoding;
> -       } else {
> -               LOG(V4L2, Warning)
> -                       << "Unknown subdev format "
> -                       << utils::hex(subdevFmt.format.code, 4)
> -                       << ", defaulting to RGB encoding";
> -
> -               colourEncoding = PixelFormatInfo::ColourEncodingRGB;
> -       }
> -       format->colorSpace = toColorSpace(subdevFmt.format,
> colourEncoding);
> +       format->colorSpace = toColorSpace(subdevFmt.format);
>
>         return 0;
>  }
> @@ -562,20 +570,7 @@ int V4L2Subdevice::setFormat(unsigned int pad,
> V4L2SubdeviceFormat *format,
>         format->size.width = subdevFmt.format.width;
>         format->size.height = subdevFmt.format.height;
>         format->mbus_code = subdevFmt.format.code;
> -
> -       PixelFormatInfo::ColourEncoding colourEncoding;
> -       auto iter = formatInfoMap.find(subdevFmt.format.code);
> -       if (iter != formatInfoMap.end()) {
> -               colourEncoding = iter->second.colourEncoding;
> -       } else {
> -               LOG(V4L2, Warning)
> -                       << "Unknown subdev format "
> -                       << utils::hex(subdevFmt.format.code, 4)
> -                       << ", defaulting to RGB encoding";
> -
> -               colourEncoding = PixelFormatInfo::ColourEncodingRGB;
> -       }
> -       format->colorSpace = toColorSpace(subdevFmt.format,
> colourEncoding);
> +       format->colorSpace = toColorSpace(subdevFmt.format);
>
>         return 0;
>  }
>
> base-commit: da9bb8dea6821edcfd3464e8cba37ad4a91087d6
> --
> Regards,
>
> Laurent Pinchart
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20220908/a9d4600d/attachment.htm>