[libcamera-devel,08/10] android: camera_device: Handle NOISE_REDUCTION_MODES
diff mbox series

Message ID 20201009122101.73858-9-jacopo@jmondi.org
State Accepted
Headers show
Series
  • android: Introduce draft controls
Related show

Commit Message

Jacopo Mondi Oct. 9, 2020, 12:20 p.m. UTC 
Register the ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES
static metadata property inspecting the values retuned by the pipeline
handler through the properties::draft::AvailableNoiseReductionModes
libcamera property.

Reserve in the static metadata pack enough space to support all the 5
available noise reduction modes Android defines.

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

Comments

Kieran Bingham Oct. 9, 2020, 12:46 p.m. UTC | #1 
Hi Jacopo,

On 09/10/2020 13:20, Jacopo Mondi wrote:
> Register the ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES
> static metadata property inspecting the values retuned by the pipeline
> handler through the properties::draft::AvailableNoiseReductionModes
> libcamera property.
> 
> Reserve in the static metadata pack enough space to support all the 5
> available noise reduction modes Android defines.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>

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

> ---
>  src/android/camera_device.cpp | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index bccec358ea13..39d8218a67e5 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -538,7 +538,7 @@ std::tuple<uint32_t, uint32_t> CameraDevice::calculateStaticMetadataSize()
>  	 * Currently: 51 entries, 687 bytes of static metadata
>  	 */
>  	uint32_t numEntries = 51;
> -	uint32_t byteSize = 687;
> +	uint32_t byteSize = 691;
>  
>  	/*
>  	 * Calculate space occupation in bytes for dynamically built metadata
> @@ -815,9 +815,18 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
>  				  &minFocusDistance, 1);
>  
>  	/* Noise reduction modes. */
> -	uint8_t noiseReductionModes = ANDROID_NOISE_REDUCTION_MODE_OFF;
> -	staticMetadata_->addEntry(ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES,
> -				  &noiseReductionModes, 1);
> +	if (cameraProperties.contains(properties::draft::AvailableNoiseReductionModes)) {
> +		Span<const int32_t> pipelineNoiseReductionModes =
> +			cameraProperties.get<Span<const int32_t>>
> +			(properties::draft::AvailableNoiseReductionModes);
> +
> +		std::vector<uint8_t> noiseReductionModes;
> +		for (int32_t mode : pipelineNoiseReductionModes)
> +			noiseReductionModes.push_back(mode);
> +		staticMetadata_->addEntry(ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES,
> +					  noiseReductionModes.data(),
> +					  noiseReductionModes.size());
> +	}
>  
>  	/* Scaler static metadata. */
>  	float maxDigitalZoom = 1;
>
Kieran Bingham Oct. 9, 2020, 12:48 p.m. UTC | #2 
Hi Jacopo,

On 09/10/2020 13:20, Jacopo Mondi wrote:
> Register the ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES
> static metadata property inspecting the values retuned by the pipeline
> handler through the properties::draft::AvailableNoiseReductionModes
> libcamera property.
> 
> Reserve in the static metadata pack enough space to support all the 5
> available noise reduction modes Android defines.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  src/android/camera_device.cpp | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index bccec358ea13..39d8218a67e5 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -538,7 +538,7 @@ std::tuple<uint32_t, uint32_t> CameraDevice::calculateStaticMetadataSize()
>  	 * Currently: 51 entries, 687 bytes of static metadata
>  	 */
>  	uint32_t numEntries = 51;
> -	uint32_t byteSize = 687;
> +	uint32_t byteSize = 691;

'5 available noise modes' ... 687 + 5 = 692?

>  
>  	/*
>  	 * Calculate space occupation in bytes for dynamically built metadata
> @@ -815,9 +815,18 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
>  				  &minFocusDistance, 1);
>  
>  	/* Noise reduction modes. */
> -	uint8_t noiseReductionModes = ANDROID_NOISE_REDUCTION_MODE_OFF;
> -	staticMetadata_->addEntry(ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES,
> -				  &noiseReductionModes, 1);
> +	if (cameraProperties.contains(properties::draft::AvailableNoiseReductionModes)) {
> +		Span<const int32_t> pipelineNoiseReductionModes =
> +			cameraProperties.get<Span<const int32_t>>
> +			(properties::draft::AvailableNoiseReductionModes);
> +
> +		std::vector<uint8_t> noiseReductionModes;
> +		for (int32_t mode : pipelineNoiseReductionModes)
> +			noiseReductionModes.push_back(mode);
> +		staticMetadata_->addEntry(ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES,
> +					  noiseReductionModes.data(),
> +					  noiseReductionModes.size());
> +	}
>  
>  	/* Scaler static metadata. */
>  	float maxDigitalZoom = 1;
>
Kieran Bingham Oct. 9, 2020, 12:54 p.m. UTC | #3 
On 09/10/2020 13:55, Jacopo Mondi wrote:
> Hi Kieran,
> 
> On Fri, Oct 09, 2020 at 01:48:02PM +0100, Kieran Bingham wrote:
>> Hi Jacopo,
>>
>> On 09/10/2020 13:20, Jacopo Mondi wrote:
>>> Register the ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES
>>> static metadata property inspecting the values retuned by the pipeline
>>> handler through the properties::draft::AvailableNoiseReductionModes
>>> libcamera property.
>>>
>>> Reserve in the static metadata pack enough space to support all the 5
>>> available noise reduction modes Android defines.
>>>
>>> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
>>> ---
>>>  src/android/camera_device.cpp | 17 +++++++++++++----
>>>  1 file changed, 13 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
>>> index bccec358ea13..39d8218a67e5 100644
>>> --- a/src/android/camera_device.cpp
>>> +++ b/src/android/camera_device.cpp
>>> @@ -538,7 +538,7 @@ std::tuple<uint32_t, uint32_t> CameraDevice::calculateStaticMetadataSize()
>>>  	 * Currently: 51 entries, 687 bytes of static metadata
>>>  	 */
>>>  	uint32_t numEntries = 51;
>>> -	uint32_t byteSize = 687;
>>> +	uint32_t byteSize = 691;
>>
>> '5 available noise modes' ... 687 + 5 = 692?
>>
> 
> One was already reported :)

Aha ok - that's fine then.

I really wish this wasn't manual ;-)

--
Thanks

Kieran


> 
>>>
>>>  	/*
>>>  	 * Calculate space occupation in bytes for dynamically built metadata
>>> @@ -815,9 +815,18 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
>>>  				  &minFocusDistance, 1);
>>>
>>>  	/* Noise reduction modes. */
>>> -	uint8_t noiseReductionModes = ANDROID_NOISE_REDUCTION_MODE_OFF;
>>> -	staticMetadata_->addEntry(ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES,
>>> -				  &noiseReductionModes, 1);
>>> +	if (cameraProperties.contains(properties::draft::AvailableNoiseReductionModes)) {
>>> +		Span<const int32_t> pipelineNoiseReductionModes =
>>> +			cameraProperties.get<Span<const int32_t>>
>>> +			(properties::draft::AvailableNoiseReductionModes);
>>> +
>>> +		std::vector<uint8_t> noiseReductionModes;
>>> +		for (int32_t mode : pipelineNoiseReductionModes)
>>> +			noiseReductionModes.push_back(mode);
>>> +		staticMetadata_->addEntry(ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES,
>>> +					  noiseReductionModes.data(),
>>> +					  noiseReductionModes.size());
>>> +	}
>>>
>>>  	/* Scaler static metadata. */
>>>  	float maxDigitalZoom = 1;
>>>
>>
>> --
>> Regards
>> --
>> Kieran
Jacopo Mondi Oct. 9, 2020, 12:55 p.m. UTC | #4 
Hi Kieran,

On Fri, Oct 09, 2020 at 01:48:02PM +0100, Kieran Bingham wrote:
> Hi Jacopo,
>
> On 09/10/2020 13:20, Jacopo Mondi wrote:
> > Register the ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES
> > static metadata property inspecting the values retuned by the pipeline
> > handler through the properties::draft::AvailableNoiseReductionModes
> > libcamera property.
> >
> > Reserve in the static metadata pack enough space to support all the 5
> > available noise reduction modes Android defines.
> >
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > ---
> >  src/android/camera_device.cpp | 17 +++++++++++++----
> >  1 file changed, 13 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > index bccec358ea13..39d8218a67e5 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -538,7 +538,7 @@ std::tuple<uint32_t, uint32_t> CameraDevice::calculateStaticMetadataSize()
> >  	 * Currently: 51 entries, 687 bytes of static metadata
> >  	 */
> >  	uint32_t numEntries = 51;
> > -	uint32_t byteSize = 687;
> > +	uint32_t byteSize = 691;
>
> '5 available noise modes' ... 687 + 5 = 692?
>

One was already reported :)

> >
> >  	/*
> >  	 * Calculate space occupation in bytes for dynamically built metadata
> > @@ -815,9 +815,18 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
> >  				  &minFocusDistance, 1);
> >
> >  	/* Noise reduction modes. */
> > -	uint8_t noiseReductionModes = ANDROID_NOISE_REDUCTION_MODE_OFF;
> > -	staticMetadata_->addEntry(ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES,
> > -				  &noiseReductionModes, 1);
> > +	if (cameraProperties.contains(properties::draft::AvailableNoiseReductionModes)) {
> > +		Span<const int32_t> pipelineNoiseReductionModes =
> > +			cameraProperties.get<Span<const int32_t>>
> > +			(properties::draft::AvailableNoiseReductionModes);
> > +
> > +		std::vector<uint8_t> noiseReductionModes;
> > +		for (int32_t mode : pipelineNoiseReductionModes)
> > +			noiseReductionModes.push_back(mode);
> > +		staticMetadata_->addEntry(ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES,
> > +					  noiseReductionModes.data(),
> > +					  noiseReductionModes.size());
> > +	}
> >
> >  	/* Scaler static metadata. */
> >  	float maxDigitalZoom = 1;
> >
>
> --
> Regards
> --
> Kieran
Jacopo Mondi Oct. 9, 2020, 1:22 p.m. UTC | #5 
Hi Kieran,

On Fri, Oct 09, 2020 at 01:54:21PM +0100, Kieran Bingham wrote:
>
>
> On 09/10/2020 13:55, Jacopo Mondi wrote:
> > Hi Kieran,
> >
> > On Fri, Oct 09, 2020 at 01:48:02PM +0100, Kieran Bingham wrote:
> >> Hi Jacopo,
> >>
> >> On 09/10/2020 13:20, Jacopo Mondi wrote:
> >>> Register the ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES
> >>> static metadata property inspecting the values retuned by the pipeline
> >>> handler through the properties::draft::AvailableNoiseReductionModes
> >>> libcamera property.
> >>>
> >>> Reserve in the static metadata pack enough space to support all the 5
> >>> available noise reduction modes Android defines.
> >>>
> >>> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> >>> ---
> >>>  src/android/camera_device.cpp | 17 +++++++++++++----
> >>>  1 file changed, 13 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> >>> index bccec358ea13..39d8218a67e5 100644
> >>> --- a/src/android/camera_device.cpp
> >>> +++ b/src/android/camera_device.cpp
> >>> @@ -538,7 +538,7 @@ std::tuple<uint32_t, uint32_t> CameraDevice::calculateStaticMetadataSize()
> >>>  	 * Currently: 51 entries, 687 bytes of static metadata
> >>>  	 */
> >>>  	uint32_t numEntries = 51;
> >>> -	uint32_t byteSize = 687;
> >>> +	uint32_t byteSize = 691;
> >>
> >> '5 available noise modes' ... 687 + 5 = 692?
> >>
> >
> > One was already reported :)
>
> Aha ok - that's fine then.
>
> I really wish this wasn't manual ;-)

It's awful I agree.

Considering this clarification, I think the values in
PIPELINE_MAX_DEPTH, COLOR_CORRECTION_ABERRATION_MODE, NOISE_REDUCTION_MODES
are correct. LENS_SHADING_MAP_MODES is not, as it's brand new, so
space should be reserved not for 1, but for the 2 possible values it
supports.

>
> --
> Thanks
>
> Kieran
>
>
> >
> >>>
> >>>  	/*
> >>>  	 * Calculate space occupation in bytes for dynamically built metadata
> >>> @@ -815,9 +815,18 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
> >>>  				  &minFocusDistance, 1);
> >>>
> >>>  	/* Noise reduction modes. */
> >>> -	uint8_t noiseReductionModes = ANDROID_NOISE_REDUCTION_MODE_OFF;
> >>> -	staticMetadata_->addEntry(ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES,
> >>> -				  &noiseReductionModes, 1);
> >>> +	if (cameraProperties.contains(properties::draft::AvailableNoiseReductionModes)) {
> >>> +		Span<const int32_t> pipelineNoiseReductionModes =
> >>> +			cameraProperties.get<Span<const int32_t>>
> >>> +			(properties::draft::AvailableNoiseReductionModes);
> >>> +
> >>> +		std::vector<uint8_t> noiseReductionModes;
> >>> +		for (int32_t mode : pipelineNoiseReductionModes)
> >>> +			noiseReductionModes.push_back(mode);
> >>> +		staticMetadata_->addEntry(ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES,
> >>> +					  noiseReductionModes.data(),
> >>> +					  noiseReductionModes.size());
> >>> +	}
> >>>
> >>>  	/* Scaler static metadata. */
> >>>  	float maxDigitalZoom = 1;
> >>>
> >>
> >> --
> >> Regards
> >> --
> >> Kieran
>
> --
> Regards
> --
> Kieran

Patch
diff mbox series 

diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index bccec358ea13..39d8218a67e5 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -538,7 +538,7 @@  std::tuple<uint32_t, uint32_t> CameraDevice::calculateStaticMetadataSize()
 	 * Currently: 51 entries, 687 bytes of static metadata
 	 */
 	uint32_t numEntries = 51;
-	uint32_t byteSize = 687;
+	uint32_t byteSize = 691;
 
 	/*
 	 * Calculate space occupation in bytes for dynamically built metadata
@@ -815,9 +815,18 @@  const camera_metadata_t *CameraDevice::getStaticMetadata()
 				  &minFocusDistance, 1);
 
 	/* Noise reduction modes. */
-	uint8_t noiseReductionModes = ANDROID_NOISE_REDUCTION_MODE_OFF;
-	staticMetadata_->addEntry(ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES,
-				  &noiseReductionModes, 1);
+	if (cameraProperties.contains(properties::draft::AvailableNoiseReductionModes)) {
+		Span<const int32_t> pipelineNoiseReductionModes =
+			cameraProperties.get<Span<const int32_t>>
+			(properties::draft::AvailableNoiseReductionModes);
+
+		std::vector<uint8_t> noiseReductionModes;
+		for (int32_t mode : pipelineNoiseReductionModes)
+			noiseReductionModes.push_back(mode);
+		staticMetadata_->addEntry(ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES,
+					  noiseReductionModes.data(),
+					  noiseReductionModes.size());
+	}
 
 	/* Scaler static metadata. */
 	float maxDigitalZoom = 1;