[libcamera-devel] [PATCH RFC 6/7] android: yuv: add YUYV -> NV12 conversion

[Tomi Valkeinen]

On 25/09/2023 13:56, Laurent Pinchart wrote:
> On Mon, Sep 25, 2023 at 01:52:40PM +0300, Tomi Valkeinen wrote:
>> On 25/09/2023 00:34, Laurent Pinchart wrote:
>>> Hi Mattijs,
>>>
>>> Thank you for the patch.
>>>
>>> On Fri, Sep 15, 2023 at 09:57:30AM +0200, Mattijs Korpershoek via libcamera-devel wrote:
>>>> On am62x platforms, the receiver driver (j721e-csi2rx) only
>>>> supports packed YUV422 formats such as YUYV, YVYU, UYVY and VYUY.
>>>>
>>>> The receiver and the sensor (ov5640) hardware are both capable of
>>>> YUV420, however:
>>>>
>>>> * we are not aware of OV5640 being tested with YUV420 formats on any
>>>>     vendor tree.
>>>
>>> Are you talking about packed YUV 4:2:0 here, or planar YUV 4:2:0 ?
>>> Sensors only output packed formats (there could be exceptions, but they
>>> would be very rare), it is the DMA engine on the host size that would
>>> convert them to planar formats. That would be the DMA engine handled by
>>> the k3-udma driver. It's fairly complicated, and I don't know if it
>>> would hahev the ability to output planar formats. Tomi (on CC) may have
>>> more information.
>>
>> No, I don't know if the CSI-2 RX or the DMA engine support packed to
>> planar conversions. Maybe Jai knows.
>>
>> If the point is to show the frames on the screen, it sounds to me that
>> the YUV422 formats would work the best.
> 
> Do you mean packed YUV 4:2:2 or (a.k.a. YUYV & co), semi-planar YUV
> 4:2:2 (a.k.a. NV16 and NV61) or planar YUV 4:2:2 ? -)

Packed. That's the only option the caneras will will send, isn't it? But 
I could rephrase: any format that is supported both by the DSS and the 
sensor =).

  Tomi

>>>> * NV12 has different line lines (even lines are twice as long as odd
>>>
>>> s/lines/lengths/
>>>
>>>>     lines) Different line-sized DMA transfers have not been tested on
>>>
>>> s/)/)./
>>>
>>>>     the TI CSI-RX SHIM IP.
>>>
>>> As mentioned just above, the sensor will not output planar formats, so
>>> the issue is about the
>>>
>>>> On the other hand, the graphics allocator (gralloc) cannot allocate
>>>> YUV422 buffers directly. It mainly allocated NV12 buffers when
>>>> userspace requests HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED.
>>>
>>> The DSS seems to support YUYV, is there a way to fix the TI gralloc to
>>> support it ?
>>>
>>>> Because of the above, we need a pixel conversion from YUYV to NV12,
>>>> which is handled in the yuv processor via libyuv.
>>>>
>>>> Signed-off-by: Mattijs Korpershoek <mkorpershoek at baylibre.com>
>>>> ---
>>>>    src/android/yuv/post_processor_yuv.cpp | 91 +++++++++++++++++++++++++---------
>>>>    1 file changed, 68 insertions(+), 23 deletions(-)
>>>>
>>>> diff --git a/src/android/yuv/post_processor_yuv.cpp b/src/android/yuv/post_processor_yuv.cpp
>>>> index 734bb85b7351..b680b833dafb 100644
>>>> --- a/src/android/yuv/post_processor_yuv.cpp
>>>> +++ b/src/android/yuv/post_processor_yuv.cpp
>>>> @@ -7,6 +7,9 @@
>>>>    
>>>>    #include "post_processor_yuv.h"
>>>>    
>>>> +#include <utility>
>>>> +
>>>> +#include <libyuv/convert.h>
>>>>    #include <libyuv/scale.h>
>>>>    
>>>>    #include <libcamera/base/log.h>
>>>> @@ -22,14 +25,42 @@ using namespace libcamera;
>>>>    
>>>>    LOG_DEFINE_CATEGORY(YUV)
>>>>    
>>>> +namespace {
>>>> +
>>>> +/**
>>>> + * \var supportedConversions
>>>> + * \brief list of supported output pixel formats for an input pixel format
>>>> + */
>>>> +const std::map<PixelFormat, const std::vector<PixelFormat>> supportedConversions = {
>>>> +	{ formats::YUYV, { formats::NV12 } },
>>>> +};
>>>> +
>>>> +} /* namespace */
>>>> +
>>>>    int PostProcessorYuv::configure(const StreamConfiguration &inCfg,
>>>>    				const StreamConfiguration &outCfg)
>>>>    {
>>>>    	if (inCfg.pixelFormat != outCfg.pixelFormat) {
>>>> -		LOG(YUV, Error) << "Pixel format conversion is not supported"
>>>> -				<< " (from " << inCfg.pixelFormat
>>>> -				<< " to " << outCfg.pixelFormat << ")";
>>>> -		return -EINVAL;
>>>> +		const auto it = supportedConversions.find(inCfg.pixelFormat);
>>>> +		if (it == supportedConversions.end()) {
>>>> +			LOG(YUV, Error) << "Unsupported source format " << inCfg.pixelFormat;
>>>
>>> 			LOG(YUV, Error)
>>> 				<< "Unsupported source format "
>>> 				<< inCfg.pixelFormat;
>>>
>>>> +			return -EINVAL;
>>>> +		}
>>>> +
>>>> +		std::vector<PixelFormat> outFormats = it->second;
>>>
>>> No need for a copy.
>>>
>>>> +		const auto &match = std::find(outFormats.begin(), outFormats.end(), outCfg.pixelFormat);
>>>
>>> 		const auto &match = std::find(outFormats.begin(), outFormats.end(),
>>> 					      outCfg.pixelFormat);
>>>
>>>> +		if (match == outFormats.end()) {
>>>> +			LOG(YUV, Error) << "Requested pixel format conversion is not supported"
>>>> +					<< " (from " << inCfg.pixelFormat
>>>> +					<< " to " << outCfg.pixelFormat << ")";
>>>
>>> 			LOG(YUV, Error)
>>> 				<< "Requested pixel format conversion is not supported"
>>> 				<< " (from " << inCfg.pixelFormat
>>> 				<< " to " << outCfg.pixelFormat << ")";
>>>
>>>> +			return -EINVAL;
>>>> +		}
>>>> +	} else {
>>>> +		if (inCfg.pixelFormat != formats::NV12) {
>>>> +			LOG(YUV, Error) << "Unsupported format " << inCfg.pixelFormat
>>>> +					<< " (only NV12 is supported for scaling)";
>>>
>>> 			LOG(YUV, Error)
>>> 				<< "Unsupported format " << inCfg.pixelFormat
>>> 				<< " (only NV12 is supported for scaling)";
>>>
>>>> +			return -EINVAL;
>>>> +		}
>>>>    	}
>>>>    
>>>>    	if (inCfg.size < outCfg.size) {
>>>> @@ -39,12 +70,6 @@ int PostProcessorYuv::configure(const StreamConfiguration &inCfg,
>>>>    		return -EINVAL;
>>>>    	}
>>>>    
>>>> -	if (inCfg.pixelFormat != formats::NV12) {
>>>> -		LOG(YUV, Error) << "Unsupported format " << inCfg.pixelFormat
>>>> -				<< " (only NV12 is supported)";
>>>> -		return -EINVAL;
>>>> -	}
>>>> -
>>>>    	calculateLengths(inCfg, outCfg);
>>>>    	return 0;
>>>>    }
>>>> @@ -66,20 +91,40 @@ void PostProcessorYuv::process(Camera3RequestDescriptor::StreamBuffer *streamBuf
>>>>    		return;
>>>>    	}
>>>>    
>>>> -	int ret = libyuv::NV12Scale(sourceMapped.planes()[0].data(),
>>>> -				    sourceStride_[0],
>>>> -				    sourceMapped.planes()[1].data(),
>>>> -				    sourceStride_[1],
>>>> -				    sourceSize_.width, sourceSize_.height,
>>>> -				    destination->plane(0).data(),
>>>> -				    destinationStride_[0],
>>>> -				    destination->plane(1).data(),
>>>> -				    destinationStride_[1],
>>>> -				    destinationSize_.width,
>>>> -				    destinationSize_.height,
>>>> -				    libyuv::FilterMode::kFilterBilinear);
>>>> +	int ret = 0;
>>>> +	switch (sourceFormat_) {
>>>> +	case formats::NV12:
>>>> +		ret = libyuv::NV12Scale(sourceMapped.planes()[0].data(),
>>>> +					sourceStride_[0],
>>>> +					sourceMapped.planes()[1].data(),
>>>> +					sourceStride_[1],
>>>> +					sourceSize_.width, sourceSize_.height,
>>>> +					destination->plane(0).data(),
>>>> +					destinationStride_[0],
>>>> +					destination->plane(1).data(),
>>>> +					destinationStride_[1],
>>>> +					destinationSize_.width,
>>>> +					destinationSize_.height,
>>>> +					libyuv::FilterMode::kFilterBilinear);
>>>> +		break;
>>>> +	case formats::YUYV:
>>>> +		ret = libyuv::YUY2ToNV12(sourceMapped.planes()[0].data(),
>>>> +					 sourceStride_[0],
>>>> +					 destination->plane(0).data(),
>>>> +					 destinationStride_[0],
>>>> +					 destination->plane(1).data(),
>>>> +					 destinationStride_[1],
>>>> +					 destinationSize_.width,
>>>> +					 destinationSize_.height);
>>>> +		break;
>>>> +	default:
>>>> +		LOG(YUV, Error) << "Unsupported source format " << sourceFormat_;
>>>> +		processComplete.emit(streamBuffer, PostProcessor::Status::Error);
>>>> +		break;
>>>
>>> This can't happen, can it ?
>>>
>>>> +	}
>>>> +
>>>>    	if (ret) {
>>>> -		LOG(YUV, Error) << "Failed NV12 scaling: " << ret;
>>>> +		LOG(YUV, Error) << "Libyuv operation failure: " << ret;
>>>
>>> s/Libyuv/libyuv/
>>>
>>> Although I would write "YUV post-processing failure: ".
>>>
>>>>    		processComplete.emit(streamBuffer, PostProcessor::Status::Error);
>>>>    		return;
>>>>    	}
>>>>
>