[libcamera-devel] [PATCH RFC 3/7] android: yuv: prepare support for other pixel formats

[Jacopo Mondi]

Hi Mattijs

On Fri, Sep 15, 2023 at 09:57:27AM +0200, Mattijs Korpershoek via libcamera-devel wrote:
> PostProcessorYuv assumes that both source and destination pixel formats
> will always be formats::NV12.
> This might change in the future, for example when doing pixel format
> conversion via libyuv.
>
> Add the necessary plumbing so that other formats can be added easily in
> the future.
>
> Also increase plane number to 3, since some YUV format [1] are
> fully planar (and thus have 3 planes).
>
> [1] https://docs.kernel.org/userspace-api/media/v4l/pixfmt-yuv-planar.html#fully-planar-yuv-formats
> Signed-off-by: Mattijs Korpershoek <mkorpershoek at baylibre.com>
> ---
>  src/android/yuv/post_processor_yuv.cpp | 26 +++++++++++++++-----------
>  src/android/yuv/post_processor_yuv.h   | 12 ++++++++----
>  2 files changed, 23 insertions(+), 15 deletions(-)
>
> diff --git a/src/android/yuv/post_processor_yuv.cpp b/src/android/yuv/post_processor_yuv.cpp
> index d58090db14ee..734bb85b7351 100644
> --- a/src/android/yuv/post_processor_yuv.cpp
> +++ b/src/android/yuv/post_processor_yuv.cpp
> @@ -90,18 +90,18 @@ void PostProcessorYuv::process(Camera3RequestDescriptor::StreamBuffer *streamBuf
>  bool PostProcessorYuv::isValidBuffers(const FrameBuffer &source,
>  				      const CameraBuffer &destination) const
>  {
> -	if (source.planes().size() != 2) {
> +	if (source.planes().size() != sourceNumPlanes_) {
>  		LOG(YUV, Error) << "Invalid number of source planes: "
>  				<< source.planes().size();
>  		return false;
>  	}
> -	if (destination.numPlanes() != 2) {
> +	if (destination.numPlanes() != destinationNumPlanes_) {
>  		LOG(YUV, Error) << "Invalid number of destination planes: "
>  				<< destination.numPlanes();
>  		return false;
>  	}
>
> -	for (unsigned int i = 0; i < 2; i++) {
> +	for (unsigned int i = 0; i < sourceNumPlanes_; i++) {
>  		if (source.planes()[i].length < sourceLength_[i]) {
>  			LOG(YUV, Error)
>  				<< "The source planes lengths are too small, "
> @@ -112,7 +112,7 @@ bool PostProcessorYuv::isValidBuffers(const FrameBuffer &source,
>  			return false;
>  		}
>  	}
> -	for (unsigned int i = 0; i < 2; i++) {
> +	for (unsigned int i = 0; i < destinationNumPlanes_; i++) {
>  		if (destination.plane(i).size() < destinationLength_[i]) {
>  			LOG(YUV, Error)
>  				<< "The destination planes lengths are too small, "
> @@ -132,18 +132,22 @@ void PostProcessorYuv::calculateLengths(const StreamConfiguration &inCfg,
>  {
>  	sourceSize_ = inCfg.size;
>  	destinationSize_ = outCfg.size;
> +	sourceFormat_ = inCfg.pixelFormat;
> +	destinationFormat_ = outCfg.pixelFormat;
>
> -	const PixelFormatInfo &sourceInfo = PixelFormatInfo::info(formats::NV12);
> -	for (unsigned int i = 0; i < 2; i++) {
> +	const PixelFormatInfo &sourceInfo = PixelFormatInfo::info(sourceFormat_);
> +	sourceNumPlanes_ = sourceInfo.numPlanes();
> +	for (unsigned int i = 0; i < sourceInfo.numPlanes(); i++) {

Can't you use the just assigned sourceNumPlanes_ ?

>  		sourceStride_[i] = inCfg.stride;
>  		sourceLength_[i] = sourceInfo.planeSize(sourceSize_.height, i,
>  							sourceStride_[i]);
>  	}
>
> -	const PixelFormatInfo &destinationInfo = PixelFormatInfo::info(formats::NV12);
> -	for (unsigned int i = 0; i < 2; i++) {
> -		destinationStride_[i] = sourceInfo.stride(destinationSize_.width, i, 1);
> -		destinationLength_[i] = sourceInfo.planeSize(destinationSize_.height, i,
> -							     destinationStride_[i]);
> +	const PixelFormatInfo &destinationInfo = PixelFormatInfo::info(destinationFormat_);
> +	destinationNumPlanes_ = destinationInfo.numPlanes();
> +	for (unsigned int i = 0; i < destinationInfo.numPlanes(); i++) {

ditto

> +		destinationStride_[i] = destinationInfo.stride(destinationSize_.width, i, 1);
> +		destinationLength_[i] = destinationInfo.planeSize(destinationSize_.height, i,
> +								  destinationStride_[i]);
>  	}
>  }
> diff --git a/src/android/yuv/post_processor_yuv.h b/src/android/yuv/post_processor_yuv.h
> index a7ac17c564b6..bfe35f46c6dc 100644
> --- a/src/android/yuv/post_processor_yuv.h
> +++ b/src/android/yuv/post_processor_yuv.h
> @@ -28,8 +28,12 @@ private:
>
>  	libcamera::Size sourceSize_;
>  	libcamera::Size destinationSize_;
> -	unsigned int sourceLength_[2] = {};
> -	unsigned int destinationLength_[2] = {};
> -	unsigned int sourceStride_[2] = {};
> -	unsigned int destinationStride_[2] = {};
> +	libcamera::PixelFormat sourceFormat_;
> +	libcamera::PixelFormat destinationFormat_;
> +	unsigned int sourceLength_[3] = {};
> +	unsigned int destinationLength_[3] = {};
> +	unsigned int sourceStride_[3] = {};
> +	unsigned int destinationStride_[3] = {};
> +	unsigned int sourceNumPlanes_;
> +	unsigned int destinationNumPlanes_;

The rest looks good.
Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>

>  };
>
> --
> 2.41.0
>