[libcamera-devel,v6,9/9] android, controls: Add and plumb MaxLatency control
diff mbox series

Message ID 20210730103536.81117-10-paul.elder@ideasonboard.com
State Accepted
Headers show
Series
  • android: Support capability and hardware level detection
Related show

Commit Message

Paul Elder July 30, 2021, 10:35 a.m. UTC 
Add a MaxLatency control, and plumb it into the HAL accordingly.

Bug: https://bugs.libcamera.org/show_bug.cgi?id=50
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

---
Changes in v3:
- use v3 setMetadata
- add comment to explain 4
- remove todo

Changes in v2:
- use new setMetadata
- rebase on camera capabilities refactor
---
 src/android/camera_capabilities.cpp |  7 +++++--
 src/libcamera/control_ids.yaml      | 10 ++++++++++
 2 files changed, 15 insertions(+), 2 deletions(-)

Comments

Laurent Pinchart Aug. 2, 2021, 1:28 a.m. UTC | #1 
Hi Paul,

Thank you for the patch.

On Fri, Jul 30, 2021 at 07:35:36PM +0900, Paul Elder wrote:
> Add a MaxLatency control, and plumb it into the HAL accordingly.
> 
> Bug: https://bugs.libcamera.org/show_bug.cgi?id=50
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> 
> ---
> Changes in v3:
> - use v3 setMetadata
> - add comment to explain 4
> - remove todo
> 
> Changes in v2:
> - use new setMetadata
> - rebase on camera capabilities refactor
> ---
>  src/android/camera_capabilities.cpp |  7 +++++--
>  src/libcamera/control_ids.yaml      | 10 ++++++++++
>  2 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
> index 1bf3ffa5..77ad8c03 100644
> --- a/src/android/camera_capabilities.cpp
> +++ b/src/android/camera_capabilities.cpp
> @@ -1119,8 +1119,11 @@ int CameraCapabilities::initializeStaticMetadata()
>  	}
>  
>  	/* Sync static metadata. */
> -	int32_t maxLatency = ANDROID_SYNC_MAX_LATENCY_UNKNOWN;
> -	staticMetadata_->addEntry(ANDROID_SYNC_MAX_LATENCY, maxLatency);
> +	setMetadata(
> +		staticMetadata_.get(), ANDROID_SYNC_MAX_LATENCY,
> +		controlsInfo, &controls::draft::MaxLatency,
> +		ControlRange::Def,
> +		ANDROID_SYNC_MAX_LATENCY_UNKNOWN);

That's a strange indentation.

>  
>  	/* Flash static metadata. */
>  	char flashAvailable = ANDROID_FLASH_INFO_AVAILABLE_FALSE;
> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> index d92f29f5..9d4638ae 100644
> --- a/src/libcamera/control_ids.yaml
> +++ b/src/libcamera/control_ids.yaml
> @@ -622,6 +622,16 @@ controls:
>          detection, additional format conversions etc) count as an additional
>          pipeline stage.
>  
> +  - MaxLatency:
> +      type: int32_t
> +      draft: true
> +      description: |
> +        The maximum number of frames that can occur after a request (different
> +        than the previous) has been submitted, and before the result's state
> +        becomes synchronized. A value of -1 indicates unknown latency, and 0
> +        indicates per-frame control. Currently identical to
> +        ANDROID_SYNC_MAX_LATENCY.
> +
>    - TestPatternMode:
>        type: int32_t
>        draft: true
Paul Elder Aug. 2, 2021, 5:40 a.m. UTC | #2 
Hi Laurent,

On Mon, Aug 02, 2021 at 04:28:54AM +0300, Laurent Pinchart wrote:
> Hi Paul,
> 
> Thank you for the patch.
> 
> On Fri, Jul 30, 2021 at 07:35:36PM +0900, Paul Elder wrote:
> > Add a MaxLatency control, and plumb it into the HAL accordingly.
> > 
> > Bug: https://bugs.libcamera.org/show_bug.cgi?id=50
> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> > 
> > ---
> > Changes in v3:
> > - use v3 setMetadata
> > - add comment to explain 4
> > - remove todo
> > 
> > Changes in v2:
> > - use new setMetadata
> > - rebase on camera capabilities refactor
> > ---
> >  src/android/camera_capabilities.cpp |  7 +++++--
> >  src/libcamera/control_ids.yaml      | 10 ++++++++++
> >  2 files changed, 15 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
> > index 1bf3ffa5..77ad8c03 100644
> > --- a/src/android/camera_capabilities.cpp
> > +++ b/src/android/camera_capabilities.cpp
> > @@ -1119,8 +1119,11 @@ int CameraCapabilities::initializeStaticMetadata()
> >  	}
> >  
> >  	/* Sync static metadata. */
> > -	int32_t maxLatency = ANDROID_SYNC_MAX_LATENCY_UNKNOWN;
> > -	staticMetadata_->addEntry(ANDROID_SYNC_MAX_LATENCY, maxLatency);
> > +	setMetadata(
> > +		staticMetadata_.get(), ANDROID_SYNC_MAX_LATENCY,
> > +		controlsInfo, &controls::draft::MaxLatency,
> > +		ControlRange::Def,
> > +		ANDROID_SYNC_MAX_LATENCY_UNKNOWN);
> 
> That's a strange indentation.

I was thinking the first line is android metadata + tag, and the second
line mirrors that with libcamera controls info + tag.

Or is it that the name "setMetadata" is short enough that I can put the
first line on the same line as that, and indent everything further a bit
more?


Paul

> 
> >  
> >  	/* Flash static metadata. */
> >  	char flashAvailable = ANDROID_FLASH_INFO_AVAILABLE_FALSE;
> > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> > index d92f29f5..9d4638ae 100644
> > --- a/src/libcamera/control_ids.yaml
> > +++ b/src/libcamera/control_ids.yaml
> > @@ -622,6 +622,16 @@ controls:
> >          detection, additional format conversions etc) count as an additional
> >          pipeline stage.
> >  
> > +  - MaxLatency:
> > +      type: int32_t
> > +      draft: true
> > +      description: |
> > +        The maximum number of frames that can occur after a request (different
> > +        than the previous) has been submitted, and before the result's state
> > +        becomes synchronized. A value of -1 indicates unknown latency, and 0
> > +        indicates per-frame control. Currently identical to
> > +        ANDROID_SYNC_MAX_LATENCY.
> > +
> >    - TestPatternMode:
> >        type: int32_t
> >        draft: true
Laurent Pinchart Aug. 2, 2021, 8:57 a.m. UTC | #3 
Hi Paul,

On Mon, Aug 02, 2021 at 02:40:47PM +0900, paul.elder@ideasonboard.com wrote:
> On Mon, Aug 02, 2021 at 04:28:54AM +0300, Laurent Pinchart wrote:
> > On Fri, Jul 30, 2021 at 07:35:36PM +0900, Paul Elder wrote:
> > > Add a MaxLatency control, and plumb it into the HAL accordingly.
> > > 
> > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=50
> > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> > > 
> > > ---
> > > Changes in v3:
> > > - use v3 setMetadata
> > > - add comment to explain 4
> > > - remove todo
> > > 
> > > Changes in v2:
> > > - use new setMetadata
> > > - rebase on camera capabilities refactor
> > > ---
> > >  src/android/camera_capabilities.cpp |  7 +++++--
> > >  src/libcamera/control_ids.yaml      | 10 ++++++++++
> > >  2 files changed, 15 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
> > > index 1bf3ffa5..77ad8c03 100644
> > > --- a/src/android/camera_capabilities.cpp
> > > +++ b/src/android/camera_capabilities.cpp
> > > @@ -1119,8 +1119,11 @@ int CameraCapabilities::initializeStaticMetadata()
> > >  	}
> > >  
> > >  	/* Sync static metadata. */
> > > -	int32_t maxLatency = ANDROID_SYNC_MAX_LATENCY_UNKNOWN;
> > > -	staticMetadata_->addEntry(ANDROID_SYNC_MAX_LATENCY, maxLatency);
> > > +	setMetadata(
> > > +		staticMetadata_.get(), ANDROID_SYNC_MAX_LATENCY,
> > > +		controlsInfo, &controls::draft::MaxLatency,
> > > +		ControlRange::Def,
> > > +		ANDROID_SYNC_MAX_LATENCY_UNKNOWN);
> > 
> > That's a strange indentation.
> 
> I was thinking the first line is android metadata + tag, and the second
> line mirrors that with libcamera controls info + tag.
> 
> Or is it that the name "setMetadata" is short enough that I can put the
> first line on the same line as that, and indent everything further a bit
> more?

Yes, I meant

	setMetadata(staticMetadata_.get(), ANDROID_SYNC_MAX_LATENCY,
		    controlsInfo, &controls::draft::MaxLatency,
		    ControlRange::Def,
		    ANDROID_SYNC_MAX_LATENCY_UNKNOWN);

> > >  
> > >  	/* Flash static metadata. */
> > >  	char flashAvailable = ANDROID_FLASH_INFO_AVAILABLE_FALSE;
> > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> > > index d92f29f5..9d4638ae 100644
> > > --- a/src/libcamera/control_ids.yaml
> > > +++ b/src/libcamera/control_ids.yaml
> > > @@ -622,6 +622,16 @@ controls:
> > >          detection, additional format conversions etc) count as an additional
> > >          pipeline stage.
> > >  
> > > +  - MaxLatency:
> > > +      type: int32_t
> > > +      draft: true
> > > +      description: |
> > > +        The maximum number of frames that can occur after a request (different
> > > +        than the previous) has been submitted, and before the result's state
> > > +        becomes synchronized. A value of -1 indicates unknown latency, and 0
> > > +        indicates per-frame control. Currently identical to
> > > +        ANDROID_SYNC_MAX_LATENCY.
> > > +
> > >    - TestPatternMode:
> > >        type: int32_t
> > >        draft: true

Patch
diff mbox series 

diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
index 1bf3ffa5..77ad8c03 100644
--- a/src/android/camera_capabilities.cpp
+++ b/src/android/camera_capabilities.cpp
@@ -1119,8 +1119,11 @@  int CameraCapabilities::initializeStaticMetadata()
 	}
 
 	/* Sync static metadata. */
-	int32_t maxLatency = ANDROID_SYNC_MAX_LATENCY_UNKNOWN;
-	staticMetadata_->addEntry(ANDROID_SYNC_MAX_LATENCY, maxLatency);
+	setMetadata(
+		staticMetadata_.get(), ANDROID_SYNC_MAX_LATENCY,
+		controlsInfo, &controls::draft::MaxLatency,
+		ControlRange::Def,
+		ANDROID_SYNC_MAX_LATENCY_UNKNOWN);
 
 	/* Flash static metadata. */
 	char flashAvailable = ANDROID_FLASH_INFO_AVAILABLE_FALSE;
diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
index d92f29f5..9d4638ae 100644
--- a/src/libcamera/control_ids.yaml
+++ b/src/libcamera/control_ids.yaml
@@ -622,6 +622,16 @@  controls:
         detection, additional format conversions etc) count as an additional
         pipeline stage.
 
+  - MaxLatency:
+      type: int32_t
+      draft: true
+      description: |
+        The maximum number of frames that can occur after a request (different
+        than the previous) has been submitted, and before the result's state
+        becomes synchronized. A value of -1 indicates unknown latency, and 0
+        indicates per-frame control. Currently identical to
+        ANDROID_SYNC_MAX_LATENCY.
+
   - TestPatternMode:
       type: int32_t
       draft: true