[libcamera-devel] [PATCH v2 4/7] libcamera: v4l2_pixelformat: Add helper function to get the description

[Laurent Pinchart]

Hi Paul,

Thank you for the patch.

On Wed, Sep 08, 2021 at 05:14:34PM +0900, Paul Elder wrote:
> Add a helper function to V4L2PixelFormat for retrieving the V4L2
> description string.
> 
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
>  include/libcamera/internal/v4l2_pixelformat.h |  1 +
>  src/libcamera/v4l2_pixelformat.cpp            | 17 +++++++++++++++++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/include/libcamera/internal/v4l2_pixelformat.h b/include/libcamera/internal/v4l2_pixelformat.h
> index 36c690db..3b24bc73 100644
> --- a/include/libcamera/internal/v4l2_pixelformat.h
> +++ b/include/libcamera/internal/v4l2_pixelformat.h
> @@ -40,6 +40,7 @@ public:
>  	operator uint32_t() const { return fourcc_; }
>  
>  	std::string toString() const;
> +	const char *toDescription() const;
>  
>  	PixelFormat toPixelFormat() const;
>  	static V4L2PixelFormat fromPixelFormat(const PixelFormat &pixelFormat,
> diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp
> index 0a8312c4..f5598d9d 100644
> --- a/src/libcamera/v4l2_pixelformat.cpp
> +++ b/src/libcamera/v4l2_pixelformat.cpp
> @@ -250,6 +250,23 @@ std::string V4L2PixelFormat::toString() const
>  	return ss;
>  }
>  
> +/**
> + * \brief Return the V4L2 description corresponding to the V4L2 format

s/Return/Retrieve/ and maybe "Retrieve the V4L2 description for the
format".

Let's also add an extra paragraph here:

 * The description matches the value used by the kernel, as would be reported by
 * the VIDIOC_ENUM_FMT ioctl.

This should probably be copied to patch 3/7 to document the Info::name
field. And I'd also rename name to description in that patch.

> + * \return The V4L2 description corresponding to the V4L2 format
> + */
> +const char *V4L2PixelFormat::toDescription() const

I'd name the function "description", it's not really a conversion
function.

> +{
> +	const auto iter = vpf2pf.find(*this);
> +	if (iter == vpf2pf.end()) {
> +		LOG(V4L2, Warning)
> +			<< "Unsupported V4L2 pixel format "
> +			<< toString();
> +		return "";

How about "Unsupported" or something similar ?

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> +	}
> +
> +	return iter->second.name;
> +}
> +
>  /**
>   * \brief Convert the V4L2 pixel format to the corresponding PixelFormat
>   * \return The PixelFormat corresponding to the V4L2 pixel format

-- 
Regards,

Laurent Pinchart