[libcamera-devel,1/5] android: camera_device: Report more control modes

Message ID 20200725164243.168297-2-jacopo@jmondi.org
State Superseded, archived
Delegated to: Jacopo Mondi
Headers show
Series
  • android: cts: Fix Manual and VideoRecording templates
Related show

Commit Message

Jacopo Mondi July 25, 2020, 4:42 p.m. UTC
In order to prepare to construct the request template for the manual
mode use case, which requires all automatic controls of the pipeline to
be disabled, advertise support for the AE, AWB and AF algorithms on/off
modes to the static camera characteristics metadata pack,

While at it, change the type of the supported control modes from char to
uint8_t, as it's the type used for all other 'byte' controls.

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

Comments

Niklas Söderlund July 28, 2020, 11:51 p.m. UTC | #1
Hi Jacopo,

Thanks for your patch.

On 2020-07-25 18:42:39 +0200, Jacopo Mondi wrote:
> In order to prepare to construct the request template for the manual
> mode use case, which requires all automatic controls of the pipeline to
> be disabled, advertise support for the AE, AWB and AF algorithms on/off
> modes to the static camera characteristics metadata pack,
> 
> While at it, change the type of the supported control modes from char to
> uint8_t, as it's the type used for all other 'byte' controls.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  src/android/camera_device.cpp | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index d667c4f126f3..fa4570fabdd7 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -377,7 +377,7 @@ std::tuple<uint32_t, uint32_t> CameraDevice::calculateStaticMetadataSize()
>  	 * Currently: 50 entries, 647 bytes of static metadata
>  	 */
>  	uint32_t numEntries = 50;
> -	uint32_t byteSize = 667;
> +	uint32_t byteSize = 671;
>  
>  	/*
>  	 * Calculate space occupation in bytes for dynamically built metadata
> @@ -438,6 +438,7 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
>  
>  	std::vector<uint8_t> aeAvailableModes = {
>  		ANDROID_CONTROL_AE_MODE_ON,
> +		ANDROID_CONTROL_AE_MODE_OFF,
>  	};

Mostly for my education, should we not query the libcamera Camera on 
what it supports regarding AE here and other below before advertising 
support for something?

>  	staticMetadata_->addEntry(ANDROID_CONTROL_AE_AVAILABLE_MODES,
>  				  aeAvailableModes.data(),
> @@ -464,6 +465,7 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
>  				  aeCompensationStep, 1);
>  
>  	std::vector<uint8_t> availableAfModes = {
> +		ANDROID_CONTROL_AF_MODE_AUTO,
>  		ANDROID_CONTROL_AF_MODE_OFF,
>  	};
>  	staticMetadata_->addEntry(ANDROID_CONTROL_AF_AVAILABLE_MODES,
> @@ -492,6 +494,7 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
>  				  availableStabilizationModes.size());
>  
>  	std::vector<uint8_t> availableAwbModes = {
> +		ANDROID_CONTROL_AWB_MODE_AUTO,
>  		ANDROID_CONTROL_AWB_MODE_OFF,
>  	};
>  	staticMetadata_->addEntry(ANDROID_CONTROL_AWB_AVAILABLE_MODES,
> @@ -522,9 +525,13 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
>  	staticMetadata_->addEntry(ANDROID_CONTROL_AWB_LOCK_AVAILABLE,
>  				  &awbLockAvailable, 1);
>  
> -	char availableControlModes = ANDROID_CONTROL_MODE_AUTO;
> +	std::vector<uint8_t> availableControlModes = {
> +		ANDROID_CONTROL_MODE_AUTO,
> +		ANDROID_CONTROL_MODE_OFF,
> +	};
>  	staticMetadata_->addEntry(ANDROID_CONTROL_AVAILABLE_MODES,
> -				  &availableControlModes, 1);
> +				  availableControlModes.data(),
> +				  availableControlModes.size());
>  
>  	/* JPEG static metadata. */
>  	std::vector<int32_t> availableThumbnailSizes = {
> -- 
> 2.27.0
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Jacopo Mondi July 29, 2020, 8:27 a.m. UTC | #2
Hi Niklas,

On Wed, Jul 29, 2020 at 01:51:24AM +0200, Niklas Söderlund wrote:
> Hi Jacopo,
>
> Thanks for your patch.
>
> On 2020-07-25 18:42:39 +0200, Jacopo Mondi wrote:
> > In order to prepare to construct the request template for the manual
> > mode use case, which requires all automatic controls of the pipeline to
> > be disabled, advertise support for the AE, AWB and AF algorithms on/off
> > modes to the static camera characteristics metadata pack,
> >
> > While at it, change the type of the supported control modes from char to
> > uint8_t, as it's the type used for all other 'byte' controls.
> >
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > ---
> >  src/android/camera_device.cpp | 13 ++++++++++---
> >  1 file changed, 10 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > index d667c4f126f3..fa4570fabdd7 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -377,7 +377,7 @@ std::tuple<uint32_t, uint32_t> CameraDevice::calculateStaticMetadataSize()
> >  	 * Currently: 50 entries, 647 bytes of static metadata
> >  	 */
> >  	uint32_t numEntries = 50;
> > -	uint32_t byteSize = 667;
> > +	uint32_t byteSize = 671;
> >
> >  	/*
> >  	 * Calculate space occupation in bytes for dynamically built metadata
> > @@ -438,6 +438,7 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
> >
> >  	std::vector<uint8_t> aeAvailableModes = {
> >  		ANDROID_CONTROL_AE_MODE_ON,
> > +		ANDROID_CONTROL_AE_MODE_OFF,
> >  	};
>
> Mostly for my education, should we not query the libcamera Camera on
> what it supports regarding AE here and other below before advertising
> support for something?
>

We will. This serves just to generate capture intents correctly. Apart
from formats, nothing goes to the Camera to check what to report at
the moment.

> >  	staticMetadata_->addEntry(ANDROID_CONTROL_AE_AVAILABLE_MODES,
> >  				  aeAvailableModes.data(),
> > @@ -464,6 +465,7 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
> >  				  aeCompensationStep, 1);
> >
> >  	std::vector<uint8_t> availableAfModes = {
> > +		ANDROID_CONTROL_AF_MODE_AUTO,
> >  		ANDROID_CONTROL_AF_MODE_OFF,
> >  	};
> >  	staticMetadata_->addEntry(ANDROID_CONTROL_AF_AVAILABLE_MODES,
> > @@ -492,6 +494,7 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
> >  				  availableStabilizationModes.size());
> >
> >  	std::vector<uint8_t> availableAwbModes = {
> > +		ANDROID_CONTROL_AWB_MODE_AUTO,
> >  		ANDROID_CONTROL_AWB_MODE_OFF,
> >  	};
> >  	staticMetadata_->addEntry(ANDROID_CONTROL_AWB_AVAILABLE_MODES,
> > @@ -522,9 +525,13 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
> >  	staticMetadata_->addEntry(ANDROID_CONTROL_AWB_LOCK_AVAILABLE,
> >  				  &awbLockAvailable, 1);
> >
> > -	char availableControlModes = ANDROID_CONTROL_MODE_AUTO;
> > +	std::vector<uint8_t> availableControlModes = {
> > +		ANDROID_CONTROL_MODE_AUTO,
> > +		ANDROID_CONTROL_MODE_OFF,
> > +	};
> >  	staticMetadata_->addEntry(ANDROID_CONTROL_AVAILABLE_MODES,
> > -				  &availableControlModes, 1);
> > +				  availableControlModes.data(),
> > +				  availableControlModes.size());
> >
> >  	/* JPEG static metadata. */
> >  	std::vector<int32_t> availableThumbnailSizes = {
> > --
> > 2.27.0
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel@lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
>
> --
> Regards,
> Niklas Söderlund

Patch

diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index d667c4f126f3..fa4570fabdd7 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -377,7 +377,7 @@  std::tuple<uint32_t, uint32_t> CameraDevice::calculateStaticMetadataSize()
 	 * Currently: 50 entries, 647 bytes of static metadata
 	 */
 	uint32_t numEntries = 50;
-	uint32_t byteSize = 667;
+	uint32_t byteSize = 671;
 
 	/*
 	 * Calculate space occupation in bytes for dynamically built metadata
@@ -438,6 +438,7 @@  const camera_metadata_t *CameraDevice::getStaticMetadata()
 
 	std::vector<uint8_t> aeAvailableModes = {
 		ANDROID_CONTROL_AE_MODE_ON,
+		ANDROID_CONTROL_AE_MODE_OFF,
 	};
 	staticMetadata_->addEntry(ANDROID_CONTROL_AE_AVAILABLE_MODES,
 				  aeAvailableModes.data(),
@@ -464,6 +465,7 @@  const camera_metadata_t *CameraDevice::getStaticMetadata()
 				  aeCompensationStep, 1);
 
 	std::vector<uint8_t> availableAfModes = {
+		ANDROID_CONTROL_AF_MODE_AUTO,
 		ANDROID_CONTROL_AF_MODE_OFF,
 	};
 	staticMetadata_->addEntry(ANDROID_CONTROL_AF_AVAILABLE_MODES,
@@ -492,6 +494,7 @@  const camera_metadata_t *CameraDevice::getStaticMetadata()
 				  availableStabilizationModes.size());
 
 	std::vector<uint8_t> availableAwbModes = {
+		ANDROID_CONTROL_AWB_MODE_AUTO,
 		ANDROID_CONTROL_AWB_MODE_OFF,
 	};
 	staticMetadata_->addEntry(ANDROID_CONTROL_AWB_AVAILABLE_MODES,
@@ -522,9 +525,13 @@  const camera_metadata_t *CameraDevice::getStaticMetadata()
 	staticMetadata_->addEntry(ANDROID_CONTROL_AWB_LOCK_AVAILABLE,
 				  &awbLockAvailable, 1);
 
-	char availableControlModes = ANDROID_CONTROL_MODE_AUTO;
+	std::vector<uint8_t> availableControlModes = {
+		ANDROID_CONTROL_MODE_AUTO,
+		ANDROID_CONTROL_MODE_OFF,
+	};
 	staticMetadata_->addEntry(ANDROID_CONTROL_AVAILABLE_MODES,
-				  &availableControlModes, 1);
+				  availableControlModes.data(),
+				  availableControlModes.size());
 
 	/* JPEG static metadata. */
 	std::vector<int32_t> availableThumbnailSizes = {