[libcamera-devel] [PATCH 4/5] android: camera_device: Break out size calculation

[Kieran Bingham]

Hi Jacopo,

On 02/09/2020 11:47, Jacopo Mondi wrote:
> As the RAW stream sizes needs to be calculated differently from the
> processed one, break out from the initializeStreamConfigurations()
> function the procedure to calculate the processed (RGB/YUV) resolutions


"... break out the procedure to calculate the processed (RGB/YUV)
resolutions from initializeStreamConfigurations() ..."



> in order to prepare for RAW sizes calculation.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/android/camera_device.cpp | 55 ++++++++++++++++++++++++-----------
>  src/android/camera_device.h   |  5 ++++
>  2 files changed, 43 insertions(+), 17 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 493d6cecde72..765c3292e4f3 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -283,6 +283,37 @@ int CameraDevice::initialize()
>  	return ret;
>  }
>  
> +std::vector<Size> CameraDevice::listProcessedResolutions(CameraConfiguration *cameraConfig,

I would perhaps name this "findSupportedResolutions" or
"filterValidatedResolutions" or such.


does it matter that this validation phase doesn't take other streams
into consideration?

I assume not at this level, as it's just checking/validating sizes anyway.

With a rename if you wish:

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

> +							 const PixelFormat &pixelFormat,
> +							 const std::vector<Size> &resolutions)
> +{
> +	std::vector<Size> supportedResolutions;
> +
> +	StreamConfiguration &cfg = cameraConfig->at(0);
> +	for (const Size &res : resolutions) {
> +		cfg.pixelFormat = pixelFormat;
> +		cfg.size = res;
> +
> +		std::stringstream ss;
> +		ss << "Tested (" << res.toString() << ")["
> +		   << pixelFormat.toString() << "]: ";
> +
> +		CameraConfiguration::Status status = cameraConfig->validate();
> +		if (status != CameraConfiguration::Valid) {
> +			ss << " not supported";
> +			LOG(HAL, Debug) << ss.str();
> +			continue;
> +		}
> +
> +		ss << " supported";
> +		LOG(HAL, Debug) << ss.str();
> +
> +		supportedResolutions.push_back(res);
> +	}
> +
> +	return supportedResolutions;
> +}
> +
>  /*
>   * Initialize the format conversion map to translate from Android format
>   * identifier to libcamera pixel formats and fill in the list of supported
> @@ -435,24 +466,14 @@ int CameraDevice::initializeStreamConfigurations()
>  				<< camera3Format.name << " to: "
>  				<< mappedFormat.toString();
>  
> -		for (const Size &res : cameraResolutions) {
> -			cfg.pixelFormat = mappedFormat;
> -			cfg.size = res;
> -
> -			std::stringstream ss;
> -			ss << "Testing (" << res.toString() << ")["
> -			   << mappedFormat.toString() << "]: ";
> -
> -			CameraConfiguration::Status status = cameraConfig->validate();
> -			if (status != CameraConfiguration::Valid) {
> -				ss << " not supported";
> -				LOG(HAL, Debug) << ss.str();
> -				continue;
> -			}
> -
> -			ss << " supported";
> -			LOG(HAL, Debug) << ss.str();
> +		const PixelFormatInfo &info = PixelFormatInfo::info(mappedFormat);
> +		if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)
> +			continue;
>  
> +		std::vector<Size> resolutions = listProcessedResolutions(cameraConfig.get(),
> +									 mappedFormat,
> +									 cameraResolutions);
> +		for (const Size &res : resolutions) {
>  			streamConfigurations_.push_back({ res, androidFormat });
>  
>  			/*
> diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> index 3934f194f1b5..18fd5ff03cde 100644
> --- a/src/android/camera_device.h
> +++ b/src/android/camera_device.h
> @@ -93,6 +93,11 @@ private:
>  	};
>  
>  	int initializeStreamConfigurations();
> +	std::vector<libcamera::Size>
> +	listProcessedResolutions(libcamera::CameraConfiguration *cameraConfig,
> +				 const libcamera::PixelFormat &pixelFormat,
> +				 const std::vector<libcamera::Size> &resolutions);
> +
>  	std::tuple<uint32_t, uint32_t> calculateStaticMetadataSize();
>  	libcamera::FrameBuffer *createFrameBuffer(const buffer_handle_t camera3buffer);
>  	void notifyShutter(uint32_t frameNumber, uint64_t timestamp);
> 

-- 
Regards
--
Kieran