[libcamera-devel] [PATCH] android: yuv: Fix wrong access of source buffer

[Laurent Pinchart]

Hi Hiro,

Thank you for the patch.

On Thu, Aug 05, 2021 at 10:28:05PM +0900, Hirokazu Honda wrote:
> File descriptors of FrameBuffer given in PostProcessor::process()
> for all the planes point the same buffer. To access the beginning
> of the second or later plane, it is necessary to add offsets to
> the beginning of the buffer.
> Fix the wrong access to the second plane of NV12 in
> PostProcessorYuv.

This is an issue that comes from V4L2, where NV12, a multi-planar
format, can be stored with the two planes contiguously in the same
dmabuf, or in separate dmabufs. We have two V4L2 pixel formats for those
(for instance, for NV12, that's V4L2_PIX_FMT_NV12 and
V4L2_PIX_FMT_NV12M), and two APIs, the V4L2 single-planar API that only
supports the single dmabuf format, and the V4L2 multi-planar API that
supports either formats.

This patch will work with V4L2_PIX_FMT_NV12 but not with
V4L2_PIX_FMT_NV12M. Let's not force applications to deal with this mess,
I don't want to expose it through the libcamera API. We should instead
handle it in the MappedBuffer class, so that maps()[1] will return the
correct value.

In order to support both NV12 and NV12M, we'll have to add an offset
field to FrameBuffer::Plane, but in the short term, as we only support
the single-dmabuf formats in our pipeline handlers, I think we can
hardcode the calculation below in the MappedBuffer class (I haven't
tried it though, so there may be something I'm missing).

> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> ---
>  src/android/yuv/post_processor_yuv.cpp | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/src/android/yuv/post_processor_yuv.cpp b/src/android/yuv/post_processor_yuv.cpp
> index 772e805b..3b801e96 100644
> --- a/src/android/yuv/post_processor_yuv.cpp
> +++ b/src/android/yuv/post_processor_yuv.cpp
> @@ -62,9 +62,12 @@ int PostProcessorYuv::process(const FrameBuffer &source,
>  		return -EINVAL;
>  	}
>  
> -	int ret = libyuv::NV12Scale(sourceMapped.maps()[0].data(),
> +	const uint8_t *sourceY = sourceMapped.maps()[0].data();
> +	const uint8_t *sourceUV = sourceY + sourceStride_[0] * sourceSize_.height;
> +
> +	int ret = libyuv::NV12Scale(sourceY,
>  				    sourceStride_[0],
> -				    sourceMapped.maps()[1].data(),
> +				    sourceUV,
>  				    sourceStride_[1],
>  				    sourceSize_.width, sourceSize_.height,
>  				    destination->plane(0).data(),

-- 
Regards,

Laurent Pinchart