[libcamera-devel] [PATCH v3 08/10] android: camera_buffer: Add method to get the JPEG blob size

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Tue, Mar 02, 2021 at 12:51:06PM +0100, Jacopo Mondi wrote:
> To maintain compatibility with platforms that do not provide a memory
> backend implementation add a method to be return the size of the buffer
> used for JPEG encoding capped to a maximum size.
> 
> Platforms that implement a memory backend will always calculate the
> correct buffer size.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/android/camera_buffer.h              |  2 ++
>  src/android/jpeg/post_processor_jpeg.cpp | 12 ++++++++++--
>  src/android/mm/generic_camera_buffer.cpp | 14 ++++++++++++++
>  3 files changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/src/android/camera_buffer.h b/src/android/camera_buffer.h
> index 2311cdaf96b2..342aac6d3f14 100644
> --- a/src/android/camera_buffer.h
> +++ b/src/android/camera_buffer.h
> @@ -26,6 +26,8 @@ public:
>  
>  	libcamera::Span<const uint8_t> plane(unsigned int plane) const;
>  	libcamera::Span<uint8_t> plane(unsigned int plane);
> +
> +	size_t jpegBlobSize(size_t maxJpegBlobSize);

You can make this function const.

I'd name the parameter maxJpegBufferSize, in this context "blob" refers
to the small camera3_jpeg_blob structure at the end. Similarly, I'd
either name the function jpegBufferSize(), or name it jpegBlobOffset()
and include the - sizeof(struct camera3_jpeg_blob) there.

>  };
>  
>  #endif /* __ANDROID_CAMERA_BUFFER_H__ */
> diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp
> index 83244ce6769e..65ab6b196ad1 100644
> --- a/src/android/jpeg/post_processor_jpeg.cpp
> +++ b/src/android/jpeg/post_processor_jpeg.cpp
> @@ -182,8 +182,16 @@ int PostProcessorJpeg::process(const FrameBuffer &source,
>  	}
>  
>  	/* Fill in the JPEG blob header. */
> -	uint8_t *resultPtr = destination->plane(0).data()
> -			   + destination->plane(0).size()
> +	/*
> +	 * \todo For backward compatibility reasons with the android_generic
> +	 * memory backend, continue using the maxJpegBufferSize in case the
> +	 * computed buffer size is larger. This can be dropped once all
> +	 * supported platforms will have a working memory backend that
> +	 * returns the correct buffer size.
> +	 */
> +	size_t blobSize = std::min<unsigned int>(cameraDevice_->maxJpegBufferSize(),
> +						 destination->plane(0).size());
> +	uint8_t *resultPtr = destination->plane(0).data() + blobSize
>  			   - sizeof(struct camera3_jpeg_blob);

Bad rebase ? I don't think this belongs to this patch (and I would
expect to see jpegBlobSize() used here :-)).

>  	auto *blob = reinterpret_cast<struct camera3_jpeg_blob *>(resultPtr);
>  	blob->jpeg_blob_id = CAMERA3_JPEG_BLOB_ID;
> diff --git a/src/android/mm/generic_camera_buffer.cpp b/src/android/mm/generic_camera_buffer.cpp
> index 45a83c351266..5637b3f415eb 100644
> --- a/src/android/mm/generic_camera_buffer.cpp
> +++ b/src/android/mm/generic_camera_buffer.cpp
> @@ -27,6 +27,8 @@ public:
>  	unsigned int numPlanes() const;
>  
>  	Span<uint8_t> plane(unsigned int plane);
> +
> +	size_t jpegBlobSize(size_t maxJpegBlobSize);
>  };
>  
>  CameraBuffer::Private::Private(CameraBuffer *cameraBuffer,
> @@ -77,6 +79,12 @@ Span<uint8_t> CameraBuffer::Private::plane(unsigned int plane)
>  	return maps_[plane];
>  }
>  
> +size_t CameraBuffer::Private::jpegBlobSize(size_t maxJpegBlobSize)
> +{
> +	return std::min<unsigned int>(plane(0).size(),
> +				      maxJpegBlobSize);
> +}
> +
>  CameraBuffer::CameraBuffer(buffer_handle_t camera3Buffer, int flags)
>  	: Extensible(new Private(this, camera3Buffer, flags))
>  {
> @@ -109,3 +117,9 @@ Span<uint8_t> CameraBuffer::plane(unsigned int plane)
>  	Private *const d = LIBCAMERA_D_PTR();
>  	return d->plane(plane);
>  }
> +
> +size_t CameraBuffer::jpegBlobSize(size_t maxJpegBlobSize)
> +{
> +	Private *const d = LIBCAMERA_D_PTR();
> +	return d->jpegBlobSize(maxJpegBlobSize);
> +}

-- 
Regards,

Laurent Pinchart