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

[Laurent Pinchart]

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.

> * 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;
>  	}
> 

-- 
Regards,

Laurent Pinchart