[libcamera-devel,7/7] android: Add CONTROL_MODE_OFF in template and static metadata
diff mbox series

Message ID 20211123104042.3100902-8-paul.elder@ideasonboard.com
State Superseded
Delegated to: Paul Elder
Headers show
Series
  • android: Miscellaneous fixes
Related show

Commit Message

Paul Elder Nov. 23, 2021, 10:40 a.m. UTC 
Add CONTROL_MODE_OFF to the available control modes in static metadata,
if both AE off and AWB off are available. Also set CONTROL_MODE_OFF in
the manual template.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
---
 src/android/camera_capabilities.cpp | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

Comments

Kieran Bingham Nov. 25, 2021, 11:16 a.m. UTC | #1 
Quoting Paul Elder (2021-11-23 10:40:42)
> Add CONTROL_MODE_OFF to the available control modes in static metadata,
> if both AE off and AWB off are available. Also set CONTROL_MODE_OFF in
> the manual template.
> 

Seems reasonable.

> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
>  src/android/camera_capabilities.cpp | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
> index f7397d27..5d90047b 100644
> --- a/src/android/camera_capabilities.cpp
> +++ b/src/android/camera_capabilities.cpp
> @@ -999,7 +999,17 @@ int CameraCapabilities::initializeStaticMetadata()
>         staticMetadata_->addEntry(ANDROID_CONTROL_AWB_LOCK_AVAILABLE,
>                                   awbLockAvailable);
>  
> -       char availableControlModes = ANDROID_CONTROL_MODE_AUTO;
> +       /*
> +        * \todo Get this from some combination of the available AE and AWB
> +        * modes

Does that mean there are other expected combinations to handle? But I
guess that's reliant upon the AE/AWB series.

So

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>


> +        */
> +       std::vector<uint8_t> availableControlModes = { ANDROID_CONTROL_MODE_AUTO };
> +       if (staticMetadata_->entryContains<uint8_t>(ANDROID_CONTROL_AE_AVAILABLE_MODES,
> +                                                   ANDROID_CONTROL_AE_MODE_OFF) &&
> +           staticMetadata_->entryContains<uint8_t>(ANDROID_CONTROL_AWB_AVAILABLE_MODES,
> +                                                   ANDROID_CONTROL_AWB_MODE_OFF)) {
> +               availableControlModes.push_back(ANDROID_CONTROL_MODE_OFF);
> +       }
>         staticMetadata_->addEntry(ANDROID_CONTROL_AVAILABLE_MODES,
>                                   availableControlModes);
>  
> @@ -1467,6 +1477,12 @@ std::unique_ptr<CameraMetadata> CameraCapabilities::requestTemplateManual() cons
>         if (!manualTemplate)
>                 return nullptr;
>  
> +       if (staticMetadata_->entryContains<uint8_t>(ANDROID_CONTROL_AVAILABLE_MODES,
> +                                                   ANDROID_CONTROL_MODE_OFF)) {
> +               uint8_t mode = ANDROID_CONTROL_MODE_OFF;
> +               manualTemplate->appendEntry(ANDROID_CONTROL_MODE, mode);
> +       }
> +
>         return manualTemplate;
>  }
>  
> -- 
> 2.27.0
>
Paul Elder Dec. 20, 2021, 11:14 p.m. UTC | #2 
Hi Kieran,

On Thu, Nov 25, 2021 at 11:16:42AM +0000, Kieran Bingham wrote:
> Quoting Paul Elder (2021-11-23 10:40:42)
> > Add CONTROL_MODE_OFF to the available control modes in static metadata,
> > if both AE off and AWB off are available. Also set CONTROL_MODE_OFF in
> > the manual template.
> > 
> 
> Seems reasonable.
> 
> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> > ---
> >  src/android/camera_capabilities.cpp | 18 +++++++++++++++++-
> >  1 file changed, 17 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
> > index f7397d27..5d90047b 100644
> > --- a/src/android/camera_capabilities.cpp
> > +++ b/src/android/camera_capabilities.cpp
> > @@ -999,7 +999,17 @@ int CameraCapabilities::initializeStaticMetadata()
> >         staticMetadata_->addEntry(ANDROID_CONTROL_AWB_LOCK_AVAILABLE,
> >                                   awbLockAvailable);
> >  
> > -       char availableControlModes = ANDROID_CONTROL_MODE_AUTO;
> > +       /*
> > +        * \todo Get this from some combination of the available AE and AWB
> > +        * modes
> 
> Does that mean there are other expected combinations to handle? But I
> guess that's reliant upon the AE/AWB series.

"some combination" I mean like what we have here is that we assume that
auto is always supported, and off is the optional one, and we only
report CONTROL_MODE off if both are off-able. What do we do if they only
support off? Or if only only supports auto and one only supports off?
That's what the todo covers.

> 
> So
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>


Thanks,

Paul

> 
> 
> > +        */
> > +       std::vector<uint8_t> availableControlModes = { ANDROID_CONTROL_MODE_AUTO };
> > +       if (staticMetadata_->entryContains<uint8_t>(ANDROID_CONTROL_AE_AVAILABLE_MODES,
> > +                                                   ANDROID_CONTROL_AE_MODE_OFF) &&
> > +           staticMetadata_->entryContains<uint8_t>(ANDROID_CONTROL_AWB_AVAILABLE_MODES,
> > +                                                   ANDROID_CONTROL_AWB_MODE_OFF)) {
> > +               availableControlModes.push_back(ANDROID_CONTROL_MODE_OFF);
> > +       }
> >         staticMetadata_->addEntry(ANDROID_CONTROL_AVAILABLE_MODES,
> >                                   availableControlModes);
> >  
> > @@ -1467,6 +1477,12 @@ std::unique_ptr<CameraMetadata> CameraCapabilities::requestTemplateManual() cons
> >         if (!manualTemplate)
> >                 return nullptr;
> >  
> > +       if (staticMetadata_->entryContains<uint8_t>(ANDROID_CONTROL_AVAILABLE_MODES,
> > +                                                   ANDROID_CONTROL_MODE_OFF)) {
> > +               uint8_t mode = ANDROID_CONTROL_MODE_OFF;
> > +               manualTemplate->appendEntry(ANDROID_CONTROL_MODE, mode);
> > +       }
> > +
> >         return manualTemplate;
> >  }
> >  
> > -- 
> > 2.27.0
> >

Patch
diff mbox series 

diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
index f7397d27..5d90047b 100644
--- a/src/android/camera_capabilities.cpp
+++ b/src/android/camera_capabilities.cpp
@@ -999,7 +999,17 @@  int CameraCapabilities::initializeStaticMetadata()
 	staticMetadata_->addEntry(ANDROID_CONTROL_AWB_LOCK_AVAILABLE,
 				  awbLockAvailable);
 
-	char availableControlModes = ANDROID_CONTROL_MODE_AUTO;
+	/*
+	 * \todo Get this from some combination of the available AE and AWB
+	 * modes
+	 */
+	std::vector<uint8_t> availableControlModes = { ANDROID_CONTROL_MODE_AUTO };
+	if (staticMetadata_->entryContains<uint8_t>(ANDROID_CONTROL_AE_AVAILABLE_MODES,
+						    ANDROID_CONTROL_AE_MODE_OFF) &&
+	    staticMetadata_->entryContains<uint8_t>(ANDROID_CONTROL_AWB_AVAILABLE_MODES,
+						    ANDROID_CONTROL_AWB_MODE_OFF)) {
+		availableControlModes.push_back(ANDROID_CONTROL_MODE_OFF);
+	}
 	staticMetadata_->addEntry(ANDROID_CONTROL_AVAILABLE_MODES,
 				  availableControlModes);
 
@@ -1467,6 +1477,12 @@  std::unique_ptr<CameraMetadata> CameraCapabilities::requestTemplateManual() cons
 	if (!manualTemplate)
 		return nullptr;
 
+	if (staticMetadata_->entryContains<uint8_t>(ANDROID_CONTROL_AVAILABLE_MODES,
+						    ANDROID_CONTROL_MODE_OFF)) {
+		uint8_t mode = ANDROID_CONTROL_MODE_OFF;
+		manualTemplate->appendEntry(ANDROID_CONTROL_MODE, mode);
+	}
+
 	return manualTemplate;
 }