[libcamera-devel,6/7] android: camera_device: Use CameraMetadata in processControls()
diff mbox series

Message ID 20210121165305.367801-7-jacopo@jmondi.org
State Superseded
Delegated to: Jacopo Mondi
Headers show
Series
  • android: camera device and metadata improvements
Related show

Commit Message

Jacopo Mondi Jan. 21, 2021, 4:53 p.m. UTC 
Use the CameraMetadata API to retrieve the values of the request
associated settings in CameraDevice::processControls() function.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/android/camera_device.cpp | 15 ++++++---------
 src/android/camera_device.h   |  3 +--
 2 files changed, 7 insertions(+), 11 deletions(-)

Comments

Laurent Pinchart Jan. 21, 2021, 9:39 p.m. UTC | #1 
Hi Jacopo,

Thank you for the patch.

On Thu, Jan 21, 2021 at 05:53:04PM +0100, Jacopo Mondi wrote:
> Use the CameraMetadata API to retrieve the values of the request
> associated settings in CameraDevice::processControls() function.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  src/android/camera_device.cpp | 15 ++++++---------
>  src/android/camera_device.h   |  3 +--
>  2 files changed, 7 insertions(+), 11 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 9733c84612bd..d185fdf7fb2f 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -1623,10 +1623,10 @@ FrameBuffer *CameraDevice::createFrameBuffer(const buffer_handle_t camera3buffer
>  	return new FrameBuffer(std::move(planes));
>  }
>  
> -int CameraDevice::processControls(const camera3_capture_request_t *camera3Request,
> -				  Camera3RequestDescriptor *descriptor)
> +int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)
>  {
> -	if (!camera3Request->settings)
> +	const CameraMetadata &settings = descriptor->settings_;
> +	if (!settings.isValid())
>  		return 0;
>  
>  	/*
> @@ -1635,12 +1635,9 @@ int CameraDevice::processControls(const camera3_capture_request_t *camera3Reques
>  	 * \todo As soon as more controls are handled, this part should be
>  	 * broken out to a dedicated function.
>  	 */
> -	const camera_metadata_t *camera3Settings = camera3Request->settings;
>  	camera_metadata_ro_entry_t entry;
> -	int ret = find_camera_metadata_ro_entry(camera3Settings,
> -						ANDROID_SCALER_CROP_REGION,
> -						&entry);
> -	if (!ret) {
> +	bool found = settings.getEntry(ANDROID_SCALER_CROP_REGION, &entry);
> +	if (found) {

You could write

	if (settings.getEntry(ANDROID_SCALER_CROP_REGION, &entry)) {

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  		const int32_t *data = entry.data.i32;
>  		Rectangle cropRegion{ data[0], data[1],
>  				      static_cast<unsigned int>(data[2]),
> @@ -1752,7 +1749,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
>  	 * Translate controls from Android to libcamera and queue the request
>  	 * to the CameraWorker thread.
>  	 */
> -	int ret = processControls(camera3Request, descriptor);
> +	int ret = processControls(descriptor);
>  	if (ret)
>  		return ret;
>  
> diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> index bad3be8e8066..597d11fcda2e 100644
> --- a/src/android/camera_device.h
> +++ b/src/android/camera_device.h
> @@ -104,8 +104,7 @@ private:
>  	void notifyError(uint32_t frameNumber, camera3_stream_t *stream);
>  	CameraMetadata *requestTemplatePreview();
>  	libcamera::PixelFormat toPixelFormat(int format) const;
> -	int processControls(const camera3_capture_request_t *camera3Request,
> -			    Camera3RequestDescriptor *descriptor);
> +	int processControls(Camera3RequestDescriptor *descriptor);
>  	std::unique_ptr<CameraMetadata> getResultMetadata(
>  		Camera3RequestDescriptor *descriptor, int64_t timestamp);
>
Paul Elder Jan. 22, 2021, 4:25 a.m. UTC | #2 
Hi Jacopo,

On Thu, Jan 21, 2021 at 05:53:04PM +0100, Jacopo Mondi wrote:
> Use the CameraMetadata API to retrieve the values of the request
> associated settings in CameraDevice::processControls() function.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>

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

> ---
>  src/android/camera_device.cpp | 15 ++++++---------
>  src/android/camera_device.h   |  3 +--
>  2 files changed, 7 insertions(+), 11 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 9733c84612bd..d185fdf7fb2f 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -1623,10 +1623,10 @@ FrameBuffer *CameraDevice::createFrameBuffer(const buffer_handle_t camera3buffer
>  	return new FrameBuffer(std::move(planes));
>  }
>  
> -int CameraDevice::processControls(const camera3_capture_request_t *camera3Request,
> -				  Camera3RequestDescriptor *descriptor)
> +int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)
>  {
> -	if (!camera3Request->settings)
> +	const CameraMetadata &settings = descriptor->settings_;
> +	if (!settings.isValid())
>  		return 0;
>  
>  	/*
> @@ -1635,12 +1635,9 @@ int CameraDevice::processControls(const camera3_capture_request_t *camera3Reques
>  	 * \todo As soon as more controls are handled, this part should be
>  	 * broken out to a dedicated function.
>  	 */
> -	const camera_metadata_t *camera3Settings = camera3Request->settings;
>  	camera_metadata_ro_entry_t entry;
> -	int ret = find_camera_metadata_ro_entry(camera3Settings,
> -						ANDROID_SCALER_CROP_REGION,
> -						&entry);
> -	if (!ret) {
> +	bool found = settings.getEntry(ANDROID_SCALER_CROP_REGION, &entry);
> +	if (found) {
>  		const int32_t *data = entry.data.i32;
>  		Rectangle cropRegion{ data[0], data[1],
>  				      static_cast<unsigned int>(data[2]),
> @@ -1752,7 +1749,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
>  	 * Translate controls from Android to libcamera and queue the request
>  	 * to the CameraWorker thread.
>  	 */
> -	int ret = processControls(camera3Request, descriptor);
> +	int ret = processControls(descriptor);
>  	if (ret)
>  		return ret;
>  
> diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> index bad3be8e8066..597d11fcda2e 100644
> --- a/src/android/camera_device.h
> +++ b/src/android/camera_device.h
> @@ -104,8 +104,7 @@ private:
>  	void notifyError(uint32_t frameNumber, camera3_stream_t *stream);
>  	CameraMetadata *requestTemplatePreview();
>  	libcamera::PixelFormat toPixelFormat(int format) const;
> -	int processControls(const camera3_capture_request_t *camera3Request,
> -			    Camera3RequestDescriptor *descriptor);
> +	int processControls(Camera3RequestDescriptor *descriptor);
>  	std::unique_ptr<CameraMetadata> getResultMetadata(
>  		Camera3RequestDescriptor *descriptor, int64_t timestamp);
>  
> -- 
> 2.29.2
>
Jacopo Mondi Jan. 22, 2021, 9:29 a.m. UTC | #3 
Hi Paul, Laurent

On Fri, Jan 22, 2021 at 01:25:35PM +0900, paul.elder@ideasonboard.com wrote:
> Hi Jacopo,
>
> On Thu, Jan 21, 2021 at 05:53:04PM +0100, Jacopo Mondi wrote:
> > Use the CameraMetadata API to retrieve the values of the request
> > associated settings in CameraDevice::processControls() function.
> >
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
>
> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
>

Thanks, this patch and the next one will probably be skept
as they're based on my exposure+scaler series which is in review.

I'll rather re-base those changes on top of this series.

I'll push the reset of the series soon

Thanks
  j

> > ---
> >  src/android/camera_device.cpp | 15 ++++++---------
> >  src/android/camera_device.h   |  3 +--
> >  2 files changed, 7 insertions(+), 11 deletions(-)
> >
> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > index 9733c84612bd..d185fdf7fb2f 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -1623,10 +1623,10 @@ FrameBuffer *CameraDevice::createFrameBuffer(const buffer_handle_t camera3buffer
> >  	return new FrameBuffer(std::move(planes));
> >  }
> >
> > -int CameraDevice::processControls(const camera3_capture_request_t *camera3Request,
> > -				  Camera3RequestDescriptor *descriptor)
> > +int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)
> >  {
> > -	if (!camera3Request->settings)
> > +	const CameraMetadata &settings = descriptor->settings_;
> > +	if (!settings.isValid())
> >  		return 0;
> >
> >  	/*
> > @@ -1635,12 +1635,9 @@ int CameraDevice::processControls(const camera3_capture_request_t *camera3Reques
> >  	 * \todo As soon as more controls are handled, this part should be
> >  	 * broken out to a dedicated function.
> >  	 */
> > -	const camera_metadata_t *camera3Settings = camera3Request->settings;
> >  	camera_metadata_ro_entry_t entry;
> > -	int ret = find_camera_metadata_ro_entry(camera3Settings,
> > -						ANDROID_SCALER_CROP_REGION,
> > -						&entry);
> > -	if (!ret) {
> > +	bool found = settings.getEntry(ANDROID_SCALER_CROP_REGION, &entry);
> > +	if (found) {
> >  		const int32_t *data = entry.data.i32;
> >  		Rectangle cropRegion{ data[0], data[1],
> >  				      static_cast<unsigned int>(data[2]),
> > @@ -1752,7 +1749,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
> >  	 * Translate controls from Android to libcamera and queue the request
> >  	 * to the CameraWorker thread.
> >  	 */
> > -	int ret = processControls(camera3Request, descriptor);
> > +	int ret = processControls(descriptor);
> >  	if (ret)
> >  		return ret;
> >
> > diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> > index bad3be8e8066..597d11fcda2e 100644
> > --- a/src/android/camera_device.h
> > +++ b/src/android/camera_device.h
> > @@ -104,8 +104,7 @@ private:
> >  	void notifyError(uint32_t frameNumber, camera3_stream_t *stream);
> >  	CameraMetadata *requestTemplatePreview();
> >  	libcamera::PixelFormat toPixelFormat(int format) const;
> > -	int processControls(const camera3_capture_request_t *camera3Request,
> > -			    Camera3RequestDescriptor *descriptor);
> > +	int processControls(Camera3RequestDescriptor *descriptor);
> >  	std::unique_ptr<CameraMetadata> getResultMetadata(
> >  		Camera3RequestDescriptor *descriptor, int64_t timestamp);
> >
> > --
> > 2.29.2
> >

Patch
diff mbox series 

diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index 9733c84612bd..d185fdf7fb2f 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -1623,10 +1623,10 @@  FrameBuffer *CameraDevice::createFrameBuffer(const buffer_handle_t camera3buffer
 	return new FrameBuffer(std::move(planes));
 }
 
-int CameraDevice::processControls(const camera3_capture_request_t *camera3Request,
-				  Camera3RequestDescriptor *descriptor)
+int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)
 {
-	if (!camera3Request->settings)
+	const CameraMetadata &settings = descriptor->settings_;
+	if (!settings.isValid())
 		return 0;
 
 	/*
@@ -1635,12 +1635,9 @@  int CameraDevice::processControls(const camera3_capture_request_t *camera3Reques
 	 * \todo As soon as more controls are handled, this part should be
 	 * broken out to a dedicated function.
 	 */
-	const camera_metadata_t *camera3Settings = camera3Request->settings;
 	camera_metadata_ro_entry_t entry;
-	int ret = find_camera_metadata_ro_entry(camera3Settings,
-						ANDROID_SCALER_CROP_REGION,
-						&entry);
-	if (!ret) {
+	bool found = settings.getEntry(ANDROID_SCALER_CROP_REGION, &entry);
+	if (found) {
 		const int32_t *data = entry.data.i32;
 		Rectangle cropRegion{ data[0], data[1],
 				      static_cast<unsigned int>(data[2]),
@@ -1752,7 +1749,7 @@  int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
 	 * Translate controls from Android to libcamera and queue the request
 	 * to the CameraWorker thread.
 	 */
-	int ret = processControls(camera3Request, descriptor);
+	int ret = processControls(descriptor);
 	if (ret)
 		return ret;
 
diff --git a/src/android/camera_device.h b/src/android/camera_device.h
index bad3be8e8066..597d11fcda2e 100644
--- a/src/android/camera_device.h
+++ b/src/android/camera_device.h
@@ -104,8 +104,7 @@  private:
 	void notifyError(uint32_t frameNumber, camera3_stream_t *stream);
 	CameraMetadata *requestTemplatePreview();
 	libcamera::PixelFormat toPixelFormat(int format) const;
-	int processControls(const camera3_capture_request_t *camera3Request,
-			    Camera3RequestDescriptor *descriptor);
+	int processControls(Camera3RequestDescriptor *descriptor);
 	std::unique_ptr<CameraMetadata> getResultMetadata(
 		Camera3RequestDescriptor *descriptor, int64_t timestamp);