[libcamera-devel] [PATCH 1/5] android: yuv: Use CameraBuffer::stride() in PostProcessorYuv

[Jacopo Mondi]

hi Hiro,

On Tue, Aug 31, 2021 at 03:34:34PM +0900, Hirokazu Honda wrote:
> PostProcessorYuv computes strides for a CameraBuffer. CameraBuffer has
> stride() function. This replaces the computation with the function.
>
> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> ---
>  src/android/yuv/post_processor_yuv.cpp | 13 +++++++------
>  src/android/yuv/post_processor_yuv.h   |  1 -
>  2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/src/android/yuv/post_processor_yuv.cpp b/src/android/yuv/post_processor_yuv.cpp
> index 6952fc38..12d6297d 100644
> --- a/src/android/yuv/post_processor_yuv.cpp
> +++ b/src/android/yuv/post_processor_yuv.cpp
> @@ -69,9 +69,9 @@ int PostProcessorYuv::process(const FrameBuffer &source,
>  				    sourceStride_[1],
>  				    sourceSize_.width, sourceSize_.height,
>  				    destination->plane(0).data(),
> -				    destinationStride_[0],
> +				    destination->stride(0),
>  				    destination->plane(1).data(),
> -				    destinationStride_[1],
> +				    destination->stride(1),
>  				    destinationSize_.width,
>  				    destinationSize_.height,
>  				    libyuv::FilterMode::kFilterBilinear);
> @@ -115,8 +115,8 @@ bool PostProcessorYuv::isValidBuffers(const FrameBuffer &source,
>  			<< destination.plane(0).size() << ", "
>  			<< destination.plane(1).size()
>  			<< "}, expected size: {"
> -			<< sourceLength_[0] << ", "
> -			<< sourceLength_[1] << "}";
> +			<< destinationLength_[0] << ", "
> +			<< destinationLength_[1] << "}";

Is this intended ?

>  		return false;
>  	}
>
> @@ -132,13 +132,14 @@ void PostProcessorYuv::calculateLengths(const StreamConfiguration &inCfg,
>  	const PixelFormatInfo &nv12Info = PixelFormatInfo::info(formats::NV12);
>  	for (unsigned int i = 0; i < 2; i++) {
>  		sourceStride_[i] = inCfg.stride;
> -		destinationStride_[i] = nv12Info.stride(destinationSize_.width, i, 1);
> +		const unsigned int destinationStride =
> +			nv12Info.stride(destinationSize_.width, i, 1);
>
>  		const unsigned int vertSubSample =
>  			nv12Info.planes[i].verticalSubSampling;
>  		sourceLength_[i] = sourceStride_[i] *
>  			((sourceSize_.height + vertSubSample - 1) / vertSubSample);
> -		destinationLength_[i] = destinationStride_[i] *
> +		destinationLength_[i] = destinationStride *
>  			((destinationSize_.height + vertSubSample - 1) / vertSubSample);

A comment on the existing implementation:
destinationLength_[i]  is used in isValidBuffer() only for this
comparison:

	if (destination.plane(0).size() < destinationLength_[0] ||
	    destination.plane(1).size() < destinationLength_[1]) {

shouldn't we compare the source and dest ?

Also as destination is a CameraBuffer, shouldn't we use
CameraBuffer::size(i) there ?

Thanks
  j


>  	}
>  }
> diff --git a/src/android/yuv/post_processor_yuv.h b/src/android/yuv/post_processor_yuv.h
> index f8b1ba23..1a0b5e89 100644
> --- a/src/android/yuv/post_processor_yuv.h
> +++ b/src/android/yuv/post_processor_yuv.h
> @@ -36,7 +36,6 @@ private:
>  	unsigned int sourceLength_[2] = {};
>  	unsigned int destinationLength_[2] = {};
>  	unsigned int sourceStride_[2] = {};
> -	unsigned int destinationStride_[2] = {};
>  };
>
>  #endif /* __ANDROID_POST_PROCESSOR_YUV_H__ */
> --
> 2.33.0.259.gc128427fd7-goog
>