[libcamera-devel] [PATCH v2 5/6] libcamera: formats: Add support for 8-, 10- and 12-bit Bayer formats

[Kieran Bingham]

Hi Laurent,

On 30/04/2020 04:07, Laurent Pinchart wrote:
> Add the 8-, 10- and 12-but Bayer formats, in both unpacked and
> MIPI-packed variants, to the format tables in PixelFormatInfo and
> V4L2PixelFormat.
> 

Perfect, another case of my comments addressed before you read them.


> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  src/libcamera/formats.cpp          | 82 ++++++++++++++++++++++++++++++
>  src/libcamera/v4l2_pixelformat.cpp | 30 +++++++++++
>  2 files changed, 112 insertions(+)
> 
> diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp
> index 4a351020b0d9..f6f8bc1202c2 100644
> --- a/src/libcamera/formats.cpp
> +++ b/src/libcamera/formats.cpp
> @@ -194,6 +194,88 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{
>  		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_GREY),
>  	} },
>  
> +	/* Bayer formats. */
> +	{ PixelFormat(DRM_FORMAT_SBGGR8), {
> +		.format = PixelFormat(DRM_FORMAT_SBGGR8),
> +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8),
> +	} },
> +	{ PixelFormat(DRM_FORMAT_SGBRG8), {
> +		.format = PixelFormat(DRM_FORMAT_SGBRG8),
> +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8),
> +	} },
> +	{ PixelFormat(DRM_FORMAT_SGRBG8), {
> +		.format = PixelFormat(DRM_FORMAT_SGRBG8),
> +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8),
> +	} },
> +	{ PixelFormat(DRM_FORMAT_SRGGB8), {
> +		.format = PixelFormat(DRM_FORMAT_SRGGB8),
> +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8),
> +	} },
> +	{ PixelFormat(DRM_FORMAT_SBGGR10), {
> +		.format = PixelFormat(DRM_FORMAT_SBGGR10),
> +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10),
> +	} },
> +	{ PixelFormat(DRM_FORMAT_SGBRG10), {
> +		.format = PixelFormat(DRM_FORMAT_SGBRG10),
> +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10),
> +	} },
> +	{ PixelFormat(DRM_FORMAT_SGRBG10), {
> +		.format = PixelFormat(DRM_FORMAT_SGRBG10),
> +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10),
> +	} },
> +	{ PixelFormat(DRM_FORMAT_SRGGB10), {
> +		.format = PixelFormat(DRM_FORMAT_SRGGB10),
> +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10),
> +	} },
> +	{ PixelFormat(DRM_FORMAT_SBGGR10, MIPI_FORMAT_MOD_CSI2_PACKED), {
> +		.format = PixelFormat(DRM_FORMAT_SBGGR10, MIPI_FORMAT_MOD_CSI2_PACKED),
> +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P),
> +	} },
> +	{ PixelFormat(DRM_FORMAT_SGBRG10, MIPI_FORMAT_MOD_CSI2_PACKED), {
> +		.format = PixelFormat(DRM_FORMAT_SGBRG10, MIPI_FORMAT_MOD_CSI2_PACKED),
> +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P),
> +	} },
> +	{ PixelFormat(DRM_FORMAT_SGRBG10, MIPI_FORMAT_MOD_CSI2_PACKED), {
> +		.format = PixelFormat(DRM_FORMAT_SGRBG10, MIPI_FORMAT_MOD_CSI2_PACKED),
> +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P),
> +	} },
> +	{ PixelFormat(DRM_FORMAT_SRGGB10, MIPI_FORMAT_MOD_CSI2_PACKED), {
> +		.format = PixelFormat(DRM_FORMAT_SRGGB10, MIPI_FORMAT_MOD_CSI2_PACKED),
> +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P),
> +	} },
> +	{ PixelFormat(DRM_FORMAT_SBGGR12), {
> +		.format = PixelFormat(DRM_FORMAT_SBGGR12),
> +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12),
> +	} },
> +	{ PixelFormat(DRM_FORMAT_SGBRG12), {
> +		.format = PixelFormat(DRM_FORMAT_SGBRG12),
> +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12),
> +	} },
> +	{ PixelFormat(DRM_FORMAT_SGRBG12), {
> +		.format = PixelFormat(DRM_FORMAT_SGRBG12),
> +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12),
> +	} },
> +	{ PixelFormat(DRM_FORMAT_SRGGB12), {
> +		.format = PixelFormat(DRM_FORMAT_SRGGB12),
> +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12),
> +	} },
> +	{ PixelFormat(DRM_FORMAT_SBGGR12, MIPI_FORMAT_MOD_CSI2_PACKED), {
> +		.format = PixelFormat(DRM_FORMAT_SBGGR12, MIPI_FORMAT_MOD_CSI2_PACKED),
> +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P),
> +	} },
> +	{ PixelFormat(DRM_FORMAT_SGBRG12, MIPI_FORMAT_MOD_CSI2_PACKED), {
> +		.format = PixelFormat(DRM_FORMAT_SGBRG12, MIPI_FORMAT_MOD_CSI2_PACKED),
> +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P),
> +	} },
> +	{ PixelFormat(DRM_FORMAT_SGRBG12, MIPI_FORMAT_MOD_CSI2_PACKED), {
> +		.format = PixelFormat(DRM_FORMAT_SGRBG12, MIPI_FORMAT_MOD_CSI2_PACKED),
> +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P),
> +	} },
> +	{ PixelFormat(DRM_FORMAT_SRGGB12, MIPI_FORMAT_MOD_CSI2_PACKED), {
> +		.format = PixelFormat(DRM_FORMAT_SRGGB12, MIPI_FORMAT_MOD_CSI2_PACKED),
> +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P),
> +	} },
> +
>  	/* Compressed formats. */
>  	{ PixelFormat(DRM_FORMAT_MJPEG), {
>  		.format = PixelFormat(DRM_FORMAT_MJPEG),
> diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp
> index 580c0fc9d983..fd7e2d286f3f 100644
> --- a/src/libcamera/v4l2_pixelformat.cpp
> +++ b/src/libcamera/v4l2_pixelformat.cpp
> @@ -68,6 +68,36 @@ const std::map<V4L2PixelFormat, PixelFormat> vpf2pf{
>  	/* Greyscale formats. */
>  	{ V4L2PixelFormat(V4L2_PIX_FMT_GREY), PixelFormat(DRM_FORMAT_R8) },
>  
> +	/* Bayer formats. */
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8), PixelFormat(DRM_FORMAT_SBGGR8) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8), PixelFormat(DRM_FORMAT_SGBRG8) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8), PixelFormat(DRM_FORMAT_SGRBG8) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8), PixelFormat(DRM_FORMAT_SRGGB8) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10), PixelFormat(DRM_FORMAT_SBGGR10) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10), PixelFormat(DRM_FORMAT_SGBRG10) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10), PixelFormat(DRM_FORMAT_SGRBG10) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10), PixelFormat(DRM_FORMAT_SRGGB10) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10),
> +	  PixelFormat(DRM_FORMAT_SBGGR10, MIPI_FORMAT_MOD_CSI2_PACKED) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P),
> +	  PixelFormat(DRM_FORMAT_SGBRG10, MIPI_FORMAT_MOD_CSI2_PACKED) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P),
> +	  PixelFormat(DRM_FORMAT_SGRBG10, MIPI_FORMAT_MOD_CSI2_PACKED) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P),
> +	  PixelFormat(DRM_FORMAT_SRGGB10, MIPI_FORMAT_MOD_CSI2_PACKED) },

I'd still rather keep these entries as a single line though ...
I think the table would be much more readable.

Either-way,

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>


> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12), PixelFormat(DRM_FORMAT_SBGGR12) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12), PixelFormat(DRM_FORMAT_SGBRG12) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12), PixelFormat(DRM_FORMAT_SGRBG12) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12), PixelFormat(DRM_FORMAT_SRGGB12) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P),
> +	  PixelFormat(DRM_FORMAT_SBGGR12, MIPI_FORMAT_MOD_CSI2_PACKED) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P),
> +	  PixelFormat(DRM_FORMAT_SGBRG12, MIPI_FORMAT_MOD_CSI2_PACKED) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P),
> +	  PixelFormat(DRM_FORMAT_SGRBG12, MIPI_FORMAT_MOD_CSI2_PACKED) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P),
> +	  PixelFormat(DRM_FORMAT_SRGGB12, MIPI_FORMAT_MOD_CSI2_PACKED) },
> +
>  	/* Compressed formats. */
>  	{ V4L2PixelFormat(V4L2_PIX_FMT_MJPEG), PixelFormat(DRM_FORMAT_MJPEG) },
>  };
> 

-- 
Regards
--
Kieran