[libcamera-devel] [PATCH v3 3/4] android: camera_device: Rework template generation

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Wed, Sep 04, 2019 at 02:08:46PM +0200, Jacopo Mondi wrote:
> Rework the template generation procedure by adding two missing requested
> keys, fixing the capture intent to update it based on the requested
> template type, and removing static metadata keys which didn't belong
> there.

As this is temporary code there's no need to spend too much time on
this, but having split this in one change per patch would have made it
easier to review.

> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/android/camera_device.cpp | 123 +++++++---------------------------
>  1 file changed, 24 insertions(+), 99 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index dee60e3d2931..eaa0e1de12c2 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -544,52 +544,29 @@ const camera_metadata_t *CameraDevice::constructDefaultRequestSettings(int type)
>  		return nullptr;
>  	}
>  
> -	if (requestTemplate_)
> +	if (requestTemplate_) {
> +		camera_metadata_entry_t captureIntentEntry;
> +
> +		find_camera_metadata_entry(requestTemplate_,
> +			ANDROID_CONTROL_CAPTURE_INTENT, &captureIntentEntry);
> +		ret = update_camera_metadata_entry(requestTemplate_,
> +				captureIntentEntry.index,
> +				&captureIntent, 1, &captureIntentEntry);
> +		METADATA_ASSERT(ret);
> +
>  		return requestTemplate_;
> +	}

According to the HAL documentation,

     * The HAL retains ownership of this structure, but the pointer to the
     * structure must be valid until the device is closed. The framework and the
     * HAL may not modify the buffer once it is returned by this call. The same
     * buffer may be returned for subsequent calls for the same template, or for
     * other templates.

So it looks like you will need to make turn the requestTemplate_ into a
map indexed by type :-S

>  
> -	/* \todo Use correct sizes */
> -	#define REQUEST_TEMPLATE_ENTRIES	  30
> -	#define REQUEST_TEMPLATE_DATA		2048
> -	requestTemplate_ = allocate_camera_metadata(REQUEST_TEMPLATE_ENTRIES,
> -						    REQUEST_TEMPLATE_DATA);
> +	/*
> +	 * \todo Keep this in sync with the actual number of entries.
> +	 * Currently: 12 entries, 15 bytes
> +	 */
> +	requestTemplate_ = allocate_camera_metadata(15, 20);
>  	if (!requestTemplate_) {
>  		LOG(HAL, Error) << "Failed to allocate template metadata";
>  		return nullptr;
>  	}
>  
> -	/* Set to 0 the number of 'processed and stalling' streams (ie JPEG). */
> -	int32_t maxOutStream[] = { 0, 2, 0 };
> -	ret = add_camera_metadata_entry(requestTemplate_,
> -			ANDROID_REQUEST_MAX_NUM_OUTPUT_STREAMS,
> -			maxOutStream, 3);
> -	METADATA_ASSERT(ret);
> -
> -	uint8_t maxPipelineDepth = 5;
> -	ret = add_camera_metadata_entry(requestTemplate_,
> -			ANDROID_REQUEST_PIPELINE_MAX_DEPTH,
> -			&maxPipelineDepth, 1);
> -	METADATA_ASSERT(ret);
> -
> -	int32_t inputStreams = 0;
> -	ret = add_camera_metadata_entry(requestTemplate_,
> -			ANDROID_REQUEST_MAX_NUM_INPUT_STREAMS,
> -			&inputStreams, 1);
> -	METADATA_ASSERT(ret);
> -
> -	int32_t partialResultCount = 1;
> -	ret = add_camera_metadata_entry(requestTemplate_,
> -			ANDROID_REQUEST_PARTIAL_RESULT_COUNT,
> -			&partialResultCount, 1);
> -	METADATA_ASSERT(ret);
> -
> -	uint8_t availableCapabilities[] = {
> -		ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BACKWARD_COMPATIBLE,
> -	};
> -	ret = add_camera_metadata_entry(requestTemplate_,
> -			ANDROID_REQUEST_AVAILABLE_CAPABILITIES,
> -			availableCapabilities, 1);
> -	METADATA_ASSERT(ret);
> -
>  	uint8_t aeMode = ANDROID_CONTROL_AE_MODE_ON;
>  	ret = add_camera_metadata_entry(requestTemplate_,
>  			ANDROID_CONTROL_AE_MODE,
> @@ -632,12 +609,6 @@ const camera_metadata_t *CameraDevice::constructDefaultRequestSettings(int type)
>  			&awbLock, 1);
>  	METADATA_ASSERT(ret);
>  
> -	uint8_t awbLockAvailable = ANDROID_CONTROL_AWB_LOCK_AVAILABLE_FALSE;
> -	ret = add_camera_metadata_entry(requestTemplate_,
> -			ANDROID_CONTROL_AWB_LOCK_AVAILABLE,
> -			&awbLockAvailable, 1);
> -	METADATA_ASSERT(ret);
> -
>  	uint8_t flashMode = ANDROID_FLASH_MODE_OFF;
>  	ret = add_camera_metadata_entry(requestTemplate_,
>  			ANDROID_FLASH_MODE,
> @@ -650,67 +621,21 @@ const camera_metadata_t *CameraDevice::constructDefaultRequestSettings(int type)
>  			&faceDetectMode, 1);
>  	METADATA_ASSERT(ret);
>  
> +	uint8_t noiseReduction = ANDROID_NOISE_REDUCTION_MODE_OFF;
>  	ret = add_camera_metadata_entry(requestTemplate_,
> -			ANDROID_CONTROL_CAPTURE_INTENT,
> -			&captureIntent, 1);
> +			ANDROID_NOISE_REDUCTION_MODE, &noiseReduction, 1);
>  	METADATA_ASSERT(ret);
>  
> -	/*
> -	 * This is quite hard to list at the moment wihtout knowing what
> -	 * we could control.
> -	 *
> -	 * For now, just list in the available Request keys and in the available
> -	 * result keys the control and reporting of the AE algorithm.
> -	 */
> -	std::vector<int32_t> availableRequestKeys = {
> -		ANDROID_CONTROL_AE_MODE,
> -		ANDROID_CONTROL_AE_EXPOSURE_COMPENSATION,
> -		ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER,
> -		ANDROID_CONTROL_AE_LOCK,
> -		ANDROID_CONTROL_AF_TRIGGER,
> -		ANDROID_CONTROL_AWB_MODE,
> -		ANDROID_CONTROL_AWB_LOCK,
> -		ANDROID_CONTROL_AWB_LOCK_AVAILABLE,
> -		ANDROID_CONTROL_CAPTURE_INTENT,
> -		ANDROID_FLASH_MODE,
> -		ANDROID_STATISTICS_FACE_DETECT_MODE,
> -	};
> -
> +	uint8_t aberrationMode = ANDROID_COLOR_CORRECTION_ABERRATION_MODE_OFF;
>  	ret = add_camera_metadata_entry(requestTemplate_,
> -			ANDROID_REQUEST_AVAILABLE_REQUEST_KEYS,
> -			availableRequestKeys.data(),
> -			availableRequestKeys.size());
> -	METADATA_ASSERT(ret);
> -
> -	std::vector<int32_t> availableResultKeys = {
> -		ANDROID_CONTROL_AE_MODE,
> -		ANDROID_CONTROL_AE_EXPOSURE_COMPENSATION,
> -		ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER,
> -		ANDROID_CONTROL_AE_LOCK,
> -		ANDROID_CONTROL_AF_TRIGGER,
> -		ANDROID_CONTROL_AWB_MODE,
> -		ANDROID_CONTROL_AWB_LOCK,
> -		ANDROID_CONTROL_AWB_LOCK_AVAILABLE,
> -		ANDROID_CONTROL_CAPTURE_INTENT,
> -		ANDROID_FLASH_MODE,
> -		ANDROID_STATISTICS_FACE_DETECT_MODE,
> -	};
> -	ret = add_camera_metadata_entry(requestTemplate_,
> -			ANDROID_REQUEST_AVAILABLE_RESULT_KEYS,
> -			availableResultKeys.data(),
> -			availableResultKeys.size());
> +			ANDROID_COLOR_CORRECTION_ABERRATION_MODE,
> +			&aberrationMode, 1);
>  	METADATA_ASSERT(ret);
>  
> -	/*
> -	 * \todo The available characteristics are be the tags reported
> -	 * as part of the static metadata reported at hal_get_camera_info()
> -	 * time. As of now, report an empty list.
> -	 */
> -	std::vector<int32_t> availableCharacteristicsKeys = {};
> +	/* Use the capture matching the requested template type. */
>  	ret = add_camera_metadata_entry(requestTemplate_,
> -			ANDROID_REQUEST_AVAILABLE_CHARACTERISTICS_KEYS,
> -			availableCharacteristicsKeys.data(),
> -			availableCharacteristicsKeys.size());
> +			ANDROID_CONTROL_CAPTURE_INTENT,
> +			&captureIntent, 1);
>  	METADATA_ASSERT(ret);
>  
>  	return requestTemplate_;

-- 
Regards,

Laurent Pinchart