[libcamera-devel] [PATCH] android: camera_device: Calculate MAX_JPEG_SIZE

[Kieran Bingham]

Hi Jacopo,

On 02/02/2021 12:36, Jacopo Mondi wrote:
> Calculate the JPEG maximum size using the maximum preview format size
> multiplied by a 1.5 factor.
> 
> The same multiplication factor is used in the existing HAL
> implementation in ChromeOS.

This is definitely better than the somewhat arbitrary fixed value I
picked from another HAL.

> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/android/camera_device.cpp | 21 ++++++++++-----------
>  1 file changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index a50b0ebfe60e..cb87d97888ed 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -347,12 +347,6 @@ CameraDevice::CameraDevice(unsigned int id, const std::shared_ptr<Camera> &camer
>  {
>  	camera_->requestCompleted.connect(this, &CameraDevice::requestComplete);
>  
> -	/*
> -	 * \todo Determine a more accurate value for this during
> -	 *  streamConfiguration.
> -	 */
> -	maxJpegBufferSize_ = 13 << 20; /* 13631488 from USB HAL */
> -
>  	maker_ = "libcamera";
>  	model_ = "cameraModel";
>  
> @@ -629,6 +623,7 @@ int CameraDevice::initializeStreamConfigurations()
>  							mappedFormat,
>  							cameraResolutions);
>  
> +		Size maxJpegSize;
>  		for (const Size &res : resolutions) {
>  			streamConfigurations_.push_back({ res, androidFormat });
>  
> @@ -643,9 +638,17 @@ int CameraDevice::initializeStreamConfigurations()
>  			 * \todo Support JPEG streams produced by the Camera
>  			 * natively.
>  			 */
> -			if (androidFormat == HAL_PIXEL_FORMAT_YCbCr_420_888)
> +			if (androidFormat == HAL_PIXEL_FORMAT_YCbCr_420_888) {
>  				streamConfigurations_.push_back(
>  					{ res, HAL_PIXEL_FORMAT_BLOB });
> +
> +				if (res > maxJpegSize) {

Aha, so this runs over each of the supported stream configurations and
thus only the max gets through ? is that right?

(I think it is, so there's no issue)

> +					maxJpegSize = res;
> +					maxJpegBufferSize_ = maxJpegSize.width
> +							   * maxJpegSize.height
> +							   * 1.5;
> +				}
> +			}
>  		}
>  	}
>  
> @@ -878,10 +881,6 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
>  				  availableThumbnailSizes.data(),
>  				  availableThumbnailSizes.size());
>  
> -	/*
> -	 * \todo Calculate the maximum JPEG buffer size by asking the encoder
> -	 * giving the maximum frame size required.
> -	 */

Is this no longer worth considering?
I sort of agree at the moment. And if we decide we want to ask the
encoders we can handle that later. I don't think we need to worry about
the todo.

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>


>  	staticMetadata_->addEntry(ANDROID_JPEG_MAX_SIZE, &maxJpegBufferSize_, 1);
>  
>  	/* Sensor static metadata. */
> 

-- 
Regards
--
Kieran