[libcamera-devel] [PATCH 3/3] android: capabilities: Centralize RAW support check

[Umang Jain]

Hi Jacopo,

Thank you for the patch.

On 7/15/21 9:02 PM, Jacopo Mondi wrote:
> The validation of RAW stream support is performed in two different
> places:
>
> - At initializeStreamConfigurations() time, by verifying that the
>    libcamera format associated with HAL_PIXEL_FORMAT_BLOB is a Raw format
>    and ensuring the Camera successfully validates it
> - As initializeStaticMetadata() time by generating a CameraConfiguration
>    for the Raw stream role and ensuring it is a Raw format with a 16 bit
>    depth
>
> The first check is used to build the list of supported Raw stream
> resolutions. The latter is used to register the
> ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW capability.
>
> As building the list of supported Raw streams doesn't serve any
> purpose if the RAW capability is nor registered, centralize the Raw
s/nor/not
> format support verification at initializeStreamConfigurations() time by
> ensuring the supported format is a Raw one with a 16 bit depth.
>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>   src/android/camera_capabilities.cpp | 23 ++++++++---------------
>   src/android/camera_capabilities.h   |  1 +
>   2 files changed, 9 insertions(+), 15 deletions(-)
>
> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
> index 15e54192adff..9e2714f132c4 100644
> --- a/src/android/camera_capabilities.cpp
> +++ b/src/android/camera_capabilities.cpp
> @@ -122,6 +122,7 @@ int CameraCapabilities::initialize(std::shared_ptr<libcamera::Camera> camera,
>   	camera_ = camera;
>   	orientation_ = orientation;
>   	facing_ = facing;
> +	rawStreamAvailable_ = false;
>   
>   	/* Acquire the camera and initialize available stream configurations. */
>   	int ret = camera_->acquire();
> @@ -324,11 +325,14 @@ int CameraCapabilities::initializeStreamConfigurations()
>   
>   		std::vector<Size> resolutions;
>   		const PixelFormatInfo &info = PixelFormatInfo::info(mappedFormat);
> -		if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)
> +		if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW &&
> +		    info.bitsPerPixel == 16) {
> +			rawStreamAvailable_ = true;

Nice, to have this in one place now.

Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>

>   			resolutions = initializeRawResolutions(mappedFormat);
> -		else
> +		} else {
>   			resolutions = initializeYUVResolutions(mappedFormat,
>   							       cameraResolutions);
> +		}
>   
>   		for (const Size &res : resolutions) {
>   			streamConfigurations_.push_back({ res, androidFormat });
> @@ -866,22 +870,11 @@ int CameraCapabilities::initializeStaticMetadata()
>   	};
>   
>   	/* Report if camera supports RAW. */
> -	bool rawStreamAvailable = false;
> -	std::unique_ptr<CameraConfiguration> cameraConfig =
> -		camera_->generateConfiguration({ StreamRole::Raw });
> -	if (cameraConfig && !cameraConfig->empty()) {
> -		const PixelFormatInfo &info =
> -			PixelFormatInfo::info(cameraConfig->at(0).pixelFormat);
> -		/* Only advertise RAW support if RAW16 is possible. */
> -		if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW &&
> -		    info.bitsPerPixel == 16) {
> -			rawStreamAvailable = true;
> +	if (rawStreamAvailable_)
>   			availableCapabilities.push_back(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW);
> -		}
> -	}
>   
>   	/* Number of { RAW, YUV, JPEG } supported output streams */
> -	int32_t numOutStreams[] = { rawStreamAvailable, 2, 1 };
> +	int32_t numOutStreams[] = { rawStreamAvailable_, 2, 1 };
>   	staticMetadata_->addEntry(ANDROID_REQUEST_MAX_NUM_OUTPUT_STREAMS,
>   				  numOutStreams);
>   
> diff --git a/src/android/camera_capabilities.h b/src/android/camera_capabilities.h
> index e7aa46c0a689..42a976d3b482 100644
> --- a/src/android/camera_capabilities.h
> +++ b/src/android/camera_capabilities.h
> @@ -55,6 +55,7 @@ private:
>   
>   	int facing_;
>   	int orientation_;
> +	bool rawStreamAvailable_;
>   
>   	std::vector<Camera3StreamConfiguration> streamConfigurations_;
>   	std::map<int, libcamera::PixelFormat> formatsMap_;