[libcamera-devel] [RFC 1/1] libcamera: V4L2VideoDevice::to[V4L2]PixelFormat(): add Bayer formats

[Andrey Konovalov]

Hi Niklas,

Thank you for the review!

On 07.04.2020 15:40, Niklas Söderlund wrote:
> Hi Andrey,
> 
> Thanks for your work.
> 
> On 2020-04-05 00:57:30 +0300, Andrey Konovalov wrote:
>> Add support for 8-bit, 10-bit, and 10-bit packed raw Bayer formats in
>> toPixelFormat() and toV4L2PixelFormat() methods of V4L2VideoDevice class.
>>
>> Signed-off-by: Andrey Konovalov <andrey.konovalov at linaro.org>
>> ---
>>   src/libcamera/v4l2_videodevice.cpp | 56 ++++++++++++++++++++++++++++++
>>   1 file changed, 56 insertions(+)
>>
>> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
>> index eb33a68..2e3aafc 100644
>> --- a/src/libcamera/v4l2_videodevice.cpp
>> +++ b/src/libcamera/v4l2_videodevice.cpp
>> @@ -1687,6 +1687,36 @@ PixelFormat V4L2VideoDevice::toPixelFormat(V4L2PixelFormat v4l2Fourcc)
>>   	case V4L2_PIX_FMT_MJPEG:
>>   		return PixelFormat(DRM_FORMAT_MJPEG);
>>   
>> +	/* 8-bit Bayer formats. */
>> +	case V4L2_PIX_FMT_SRGGB8:
>> +		return PixelFormat(DRM_FORMAT_SRGGB8);
>> +	case V4L2_PIX_FMT_SGRBG8:
>> +		return PixelFormat(DRM_FORMAT_SGRBG8);
>> +	case V4L2_PIX_FMT_SGBRG8:
>> +		return PixelFormat(DRM_FORMAT_SGBRG8);
>> +	case V4L2_PIX_FMT_SBGGR8:
>> +		return PixelFormat(DRM_FORMAT_SBGGR8);
>> +
>> +	/* 10-bit Bayer formats. */
>> +	case V4L2_PIX_FMT_SRGGB10:
>> +		return PixelFormat(DRM_FORMAT_SRGGB10);
>> +	case V4L2_PIX_FMT_SGRBG10:
>> +		return PixelFormat(DRM_FORMAT_SGRBG10);
>> +	case V4L2_PIX_FMT_SGBRG10:
>> +		return PixelFormat(DRM_FORMAT_SGBRG10);
>> +	case V4L2_PIX_FMT_SBGGR10:
>> +		return PixelFormat(DRM_FORMAT_SBGGR10);
>> +
>> +	/* 10-bit Bayer packed formats. */
>> +	case V4L2_PIX_FMT_SRGGB10P:
>> +		return PixelFormat(DRM_FORMAT_SRGGB10, MIPI_FORMAT_MOD_CSI2_PACKED);
>> +	case V4L2_PIX_FMT_SGRBG10P:
>> +		return PixelFormat(DRM_FORMAT_SGRBG10, MIPI_FORMAT_MOD_CSI2_PACKED);
>> +	case V4L2_PIX_FMT_SGBRG10P:
>> +		return PixelFormat(DRM_FORMAT_SGBRG10, MIPI_FORMAT_MOD_CSI2_PACKED);
>> +	case V4L2_PIX_FMT_SBGGR10P:
>> +		return PixelFormat(DRM_FORMAT_SBGGR10, MIPI_FORMAT_MOD_CSI2_PACKED);
> 
> This fits the idea we had when adding the MIPI_FORMAT_MOD_CSI2_PACKED
> modifier. But it's I misunderstood how modifies are used in DRM when
> doing so. The patch for DRM is not accepted in upstream Linux yet and
> this is a local change we took into libcamera where we bet on it was the
> right approach.

Ah.. OK. I am not familiar with modifiers usage in DRM. Will take a look.
Thanks for the explanation.

> Now we know we need to think one more time about if this is right
> solution for this problem so I'm  bit reluctant building more things on
> top of.

OK. This makes sense.

> If other thinks it's fine to do so I will not block it however.
> 
>> +
>>   	/* V4L2 formats not yet supported by DRM. */
>>   	case V4L2_PIX_FMT_GREY:
>>   	default:
>> @@ -1734,6 +1764,8 @@ V4L2PixelFormat V4L2VideoDevice::toV4L2PixelFormat(const PixelFormat &pixelForma
>>   V4L2PixelFormat V4L2VideoDevice::toV4L2PixelFormat(const PixelFormat &pixelFormat,
>>   						   bool multiplanar)
>>   {
>> +	bool csi2Packed = pixelFormat.modifier() == MIPI_FORMAT_MOD_CSI2_PACKED;
>> +
>>   	switch (pixelFormat) {
>>   	/* RGB formats. */
>>   	case DRM_FORMAT_BGR888:
>> @@ -1777,6 +1809,30 @@ V4L2PixelFormat V4L2VideoDevice::toV4L2PixelFormat(const PixelFormat &pixelForma
>>   	/* Compressed formats. */
>>   	case DRM_FORMAT_MJPEG:
>>   		return V4L2PixelFormat(V4L2_PIX_FMT_MJPEG);
>> +
>> +	/* 8-bit Bayer formats. */
>> +	case DRM_FORMAT_SRGGB8:
>> +		return V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8);
>> +	case DRM_FORMAT_SGRBG8:
>> +		return V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8);
>> +	case DRM_FORMAT_SGBRG8:
>> +		return V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8);
>> +	case DRM_FORMAT_SBGGR8:
>> +		return V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8);
>> +
>> +	/* 10-bit Bayer formats, the packed ones included. */
>> +	case DRM_FORMAT_SRGGB10:
>> +		return (csi2Packed) ? V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P)
>> +			: V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10);
>> +	case DRM_FORMAT_SGRBG10:
>> +		return (csi2Packed) ? V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P)
>> +			: V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10);
>> +	case DRM_FORMAT_SGBRG10:
>> +		return (csi2Packed) ? V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P)
>> +			: V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10);
>> +	case DRM_FORMAT_SBGGR10:
>> +		return (csi2Packed) ? V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P)
>> +			: V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10);
> 
> If the issue above is resolved in such a way that this patch is accepted
> this needs to be extended to also support IPU3_FORMAT_MOD_PACKED.

Right. I'll add the support for IPU3_FORMAT_MOD_PACKED in the next version.


Thanks,
Andrey

>>   	}
>>   
>>   	/*
>> -- 
>> 2.17.1
>>
>> _______________________________________________
>> libcamera-devel mailing list
>> libcamera-devel at lists.libcamera.org
>> https://lists.libcamera.org/listinfo/libcamera-devel
>