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

Message ID 20190904120847.11934-4-jacopo@jmondi.org
State Superseded
Headers show
Series
  • android: Rework metadata tags
Related show

Commit Message

Jacopo Mondi Sept. 4, 2019, 12:08 p.m. UTC
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.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/android/camera_device.cpp | 123 +++++++---------------------------
 1 file changed, 24 insertions(+), 99 deletions(-)

Comments

Laurent Pinchart Sept. 4, 2019, 12:44 p.m. UTC | #1
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@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_;

Patch

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_;
+	}
 
-	/* \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_;