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

[Laurent Pinchart]

Hi Kieran,

On Thu, Apr 30, 2020 at 11:01:41AM +0100, Kieran Bingham wrote:
> 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.

I had done so in the beginning, when working with my large external
monitor. Then I switched to my laptop screen and cried :-)

> 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,

Laurent Pinchart