[libcamera-devel,3/3] android: camera_device: Use reference to settings
diff mbox series

Message ID 20210129142615.669464-4-jacopo@jmondi.org
State Accepted
Commit 94d42ce0143ed683b979a69b4ad4b76ec924347c
Headers show
Series
  • android: Add missing Request keys
Related show

Commit Message

Jacopo Mondi Jan. 29, 2021, 2:26 p.m. UTC 
In preparation to use the keys part of a capture request to
fill in the result metadata, instantiate a reference to
descriptor_->settings_ and use it.

While at it, move the 'ret' variable declaration to the beginning of
the function and rename it in 'found', as it will be used in many
places and move the \todo comment up as it applies to all metadata
whose value is copied from settings.

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

Comments

Niklas Söderlund Jan. 29, 2021, 11:16 p.m. UTC | #1 
Hi Jacopo,

Thanks for your work.

On 2021-01-29 15:26:15 +0100, Jacopo Mondi wrote:
> In preparation to use the keys part of a capture request to
> fill in the result metadata, instantiate a reference to
> descriptor_->settings_ and use it.

s/instantiate/create/
s/and use it//

> 
> While at it, move the 'ret' variable declaration to the beginning of
> the function and rename it in 'found', as it will be used in many
> places and move the \todo comment up as it applies to all metadata
> whose value is copied from settings.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
>  src/android/camera_device.cpp | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index b66e7c71884a..0c424978838d 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -1936,7 +1936,9 @@ CameraDevice::getResultMetadata(Camera3RequestDescriptor *descriptor,
>  				int64_t timestamp)
>  {
>  	const ControlList &metadata = descriptor->request_->metadata();
> +	const CameraMetadata &settings = descriptor->settings_;
>  	camera_metadata_ro_entry_t entry;
> +	bool found;
>  
>  	/*
>  	 * \todo Keep this in sync with the actual number of entries.
> @@ -1961,6 +1963,12 @@ CameraDevice::getResultMetadata(Camera3RequestDescriptor *descriptor,
>  		return nullptr;
>  	}
>  
> +	/*
> +	 * \todo The value of the results metadata copied from the settings
> +	 * will have to be passed to the libcamera::Camera and extracted
> +	 * from libcamera::Request::metadata.
> +	 */
> +
>  	uint8_t value = ANDROID_CONTROL_AE_ANTIBANDING_MODE_OFF;
>  	resultMetadata->addEntry(ANDROID_CONTROL_AE_ANTIBANDING_MODE, &value, 1);
>  
> @@ -1975,10 +1983,9 @@ CameraDevice::getResultMetadata(Camera3RequestDescriptor *descriptor,
>  				 aeFpsTarget.data(), aeFpsTarget.size());
>  
>  	value = ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER_IDLE;
> -	/* \todo Handle IPA appropriately */
> -	bool ret = descriptor->settings_.getEntry(ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER, &entry);
> +	found = settings.getEntry(ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER, &entry);
>  	resultMetadata->addEntry(ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER,
> -				 ret ? entry.data.u8 : &value, 1);
> +				 found ? entry.data.u8 : &value, 1);
>  
>  	value = ANDROID_CONTROL_AE_STATE_CONVERGED;
>  	resultMetadata->addEntry(ANDROID_CONTROL_AE_STATE, &value, 1);
> @@ -2022,8 +2029,7 @@ CameraDevice::getResultMetadata(Camera3RequestDescriptor *descriptor,
>  	value = ANDROID_FLASH_STATE_UNAVAILABLE;
>  	resultMetadata->addEntry(ANDROID_FLASH_STATE, &value, 1);
>  
> -	ret = descriptor->settings_.getEntry(ANDROID_LENS_APERTURE, &entry);
> -	if (ret)
> +	if (settings.getEntry(ANDROID_LENS_APERTURE, &entry))
>  		resultMetadata->addEntry(ANDROID_LENS_APERTURE, entry.data.f, 1);
>  
>  	float focal_length = 1.0;
> -- 
> 2.30.0
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Paul Elder Feb. 1, 2021, 2:52 a.m. UTC | #2 
Hi Jacopo,

On Fri, Jan 29, 2021 at 03:26:15PM +0100, Jacopo Mondi wrote:
> In preparation to use the keys part of a capture request to
> fill in the result metadata, instantiate a reference to
> descriptor_->settings_ and use it.
> 
> While at it, move the 'ret' variable declaration to the beginning of
> the function and rename it in 'found', as it will be used in many
> places and move the \todo comment up as it applies to all metadata
> whose value is copied from settings.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> ---
>  src/android/camera_device.cpp | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index b66e7c71884a..0c424978838d 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -1936,7 +1936,9 @@ CameraDevice::getResultMetadata(Camera3RequestDescriptor *descriptor,
>  				int64_t timestamp)
>  {
>  	const ControlList &metadata = descriptor->request_->metadata();
> +	const CameraMetadata &settings = descriptor->settings_;
>  	camera_metadata_ro_entry_t entry;
> +	bool found;
>  
>  	/*
>  	 * \todo Keep this in sync with the actual number of entries.
> @@ -1961,6 +1963,12 @@ CameraDevice::getResultMetadata(Camera3RequestDescriptor *descriptor,
>  		return nullptr;
>  	}
>  
> +	/*
> +	 * \todo The value of the results metadata copied from the settings
> +	 * will have to be passed to the libcamera::Camera and extracted
> +	 * from libcamera::Request::metadata.
> +	 */
> +
>  	uint8_t value = ANDROID_CONTROL_AE_ANTIBANDING_MODE_OFF;
>  	resultMetadata->addEntry(ANDROID_CONTROL_AE_ANTIBANDING_MODE, &value, 1);
>  
> @@ -1975,10 +1983,9 @@ CameraDevice::getResultMetadata(Camera3RequestDescriptor *descriptor,
>  				 aeFpsTarget.data(), aeFpsTarget.size());
>  
>  	value = ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER_IDLE;
> -	/* \todo Handle IPA appropriately */
> -	bool ret = descriptor->settings_.getEntry(ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER, &entry);
> +	found = settings.getEntry(ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER, &entry);
>  	resultMetadata->addEntry(ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER,
> -				 ret ? entry.data.u8 : &value, 1);
> +				 found ? entry.data.u8 : &value, 1);
>  
>  	value = ANDROID_CONTROL_AE_STATE_CONVERGED;
>  	resultMetadata->addEntry(ANDROID_CONTROL_AE_STATE, &value, 1);
> @@ -2022,8 +2029,7 @@ CameraDevice::getResultMetadata(Camera3RequestDescriptor *descriptor,
>  	value = ANDROID_FLASH_STATE_UNAVAILABLE;
>  	resultMetadata->addEntry(ANDROID_FLASH_STATE, &value, 1);
>  
> -	ret = descriptor->settings_.getEntry(ANDROID_LENS_APERTURE, &entry);
> -	if (ret)
> +	if (settings.getEntry(ANDROID_LENS_APERTURE, &entry))
>  		resultMetadata->addEntry(ANDROID_LENS_APERTURE, entry.data.f, 1);
>  
>  	float focal_length = 1.0;
> -- 
> 2.30.0
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Patch
diff mbox series 

diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index b66e7c71884a..0c424978838d 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -1936,7 +1936,9 @@  CameraDevice::getResultMetadata(Camera3RequestDescriptor *descriptor,
 				int64_t timestamp)
 {
 	const ControlList &metadata = descriptor->request_->metadata();
+	const CameraMetadata &settings = descriptor->settings_;
 	camera_metadata_ro_entry_t entry;
+	bool found;
 
 	/*
 	 * \todo Keep this in sync with the actual number of entries.
@@ -1961,6 +1963,12 @@  CameraDevice::getResultMetadata(Camera3RequestDescriptor *descriptor,
 		return nullptr;
 	}
 
+	/*
+	 * \todo The value of the results metadata copied from the settings
+	 * will have to be passed to the libcamera::Camera and extracted
+	 * from libcamera::Request::metadata.
+	 */
+
 	uint8_t value = ANDROID_CONTROL_AE_ANTIBANDING_MODE_OFF;
 	resultMetadata->addEntry(ANDROID_CONTROL_AE_ANTIBANDING_MODE, &value, 1);
 
@@ -1975,10 +1983,9 @@  CameraDevice::getResultMetadata(Camera3RequestDescriptor *descriptor,
 				 aeFpsTarget.data(), aeFpsTarget.size());
 
 	value = ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER_IDLE;
-	/* \todo Handle IPA appropriately */
-	bool ret = descriptor->settings_.getEntry(ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER, &entry);
+	found = settings.getEntry(ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER, &entry);
 	resultMetadata->addEntry(ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER,
-				 ret ? entry.data.u8 : &value, 1);
+				 found ? entry.data.u8 : &value, 1);
 
 	value = ANDROID_CONTROL_AE_STATE_CONVERGED;
 	resultMetadata->addEntry(ANDROID_CONTROL_AE_STATE, &value, 1);
@@ -2022,8 +2029,7 @@  CameraDevice::getResultMetadata(Camera3RequestDescriptor *descriptor,
 	value = ANDROID_FLASH_STATE_UNAVAILABLE;
 	resultMetadata->addEntry(ANDROID_FLASH_STATE, &value, 1);
 
-	ret = descriptor->settings_.getEntry(ANDROID_LENS_APERTURE, &entry);
-	if (ret)
+	if (settings.getEntry(ANDROID_LENS_APERTURE, &entry))
 		resultMetadata->addEntry(ANDROID_LENS_APERTURE, entry.data.f, 1);
 
 	float focal_length = 1.0;