[libcamera-devel,v2,2/4] android: camera_device: Use correct sizes for static metadata
diff mbox series

Message ID 20190829165311.29196-3-jacopo@jmondi.org
State Superseded
Headers show
Series
  • android: Rework metadata tags
Related show

Commit Message

Jacopo Mondi Aug. 29, 2019, 4:53 p.m. UTC 
Use more opportune sizes for the static metadata pack, and for the
dynamic metadata sizes which where wrongly set to use the ones defined
for the static pack.

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

Comments

Niklas Söderlund Aug. 30, 2019, 12:17 p.m. UTC | #1 
Hi Jacopo,

Thanks for your work.

On 2019-08-29 18:53:09 +0200, Jacopo Mondi wrote:
> Use more opportune sizes for the static metadata pack, and for the
> dynamic metadata sizes which where wrongly set to use the ones defined
> for the static pack.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  src/android/camera_device.cpp | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 02bb6fc5d042..bc8ffed92c61 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -128,11 +128,11 @@ camera_metadata_t *CameraDevice::getStaticMetadata()
>  	 * more.
>  	 */
>  
> -	/* \todo Use correct sizes */
> -	#define STATIC_ENTRY_CAP 256
> -	#define STATIC_DATA_CAP 6688
> -	staticMetadata_ = allocate_camera_metadata(STATIC_ENTRY_CAP,
> -						   STATIC_DATA_CAP);
> +	/*
> +	 * \todo Keep this in sync with the actual number of entries.
> +	 * Currently: 46 entries, 390 bytes
> +	 */
> +	staticMetadata_ = allocate_camera_metadata(50, 500);

Where do the values 50 and 500 come from, is it simply 50 > 46 and 500 > 
390 ?

>  
>  	/* Color correction static metadata. */
>  	std::vector<uint8_t> aberrationModes = {
> @@ -966,11 +966,11 @@ camera_metadata_t *CameraDevice::getResultMetadata(int frame_number,
>  {
>  	int ret;
>  
> -	/* \todo Use correct sizes */
> -	#define RESULT_ENTRY_CAP 256
> -	#define RESULT_DATA_CAP 6688
> -	camera_metadata_t *resultMetadata =
> -		allocate_camera_metadata(STATIC_ENTRY_CAP, STATIC_DATA_CAP);
> +	/*
> +	 * \todo Keep this in sync with the actual number of entries.
> +	 * Currently: 13 entries, 36 bytes
> +	 */
> +	camera_metadata_t *resultMetadata = allocate_camera_metadata(15, 50);
>  
>  	const uint8_t ae_state = ANDROID_CONTROL_AE_STATE_CONVERGED;
>  	ret = add_camera_metadata_entry(resultMetadata, ANDROID_CONTROL_AE_STATE,
> -- 
> 2.23.0
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Jacopo Mondi Aug. 30, 2019, 1:10 p.m. UTC | #2 
Hi Niklas,

On Fri, Aug 30, 2019 at 02:17:20PM +0200, Niklas Söderlund wrote:
> Hi Jacopo,
>
> Thanks for your work.
>
> On 2019-08-29 18:53:09 +0200, Jacopo Mondi wrote:
> > Use more opportune sizes for the static metadata pack, and for the
> > dynamic metadata sizes which where wrongly set to use the ones defined
> > for the static pack.
> >
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > ---
> >  src/android/camera_device.cpp | 20 ++++++++++----------
> >  1 file changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > index 02bb6fc5d042..bc8ffed92c61 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -128,11 +128,11 @@ camera_metadata_t *CameraDevice::getStaticMetadata()
> >  	 * more.
> >  	 */
> >
> > -	/* \todo Use correct sizes */
> > -	#define STATIC_ENTRY_CAP 256
> > -	#define STATIC_DATA_CAP 6688
> > -	staticMetadata_ = allocate_camera_metadata(STATIC_ENTRY_CAP,
> > -						   STATIC_DATA_CAP);
> > +	/*
> > +	 * \todo Keep this in sync with the actual number of entries.
> > +	 * Currently: 46 entries, 390 bytes
> > +	 */
> > +	staticMetadata_ = allocate_camera_metadata(50, 500);
>
> Where do the values 50 and 500 come from, is it simply 50 > 46 and 500 >
> 390 ?
>

Yes, I calculated the metadata pack occupation by hand, and I
considered staying a bit larger a safety measure against my
mis-calculations :)

> >
> >  	/* Color correction static metadata. */
> >  	std::vector<uint8_t> aberrationModes = {
> > @@ -966,11 +966,11 @@ camera_metadata_t *CameraDevice::getResultMetadata(int frame_number,
> >  {
> >  	int ret;
> >
> > -	/* \todo Use correct sizes */
> > -	#define RESULT_ENTRY_CAP 256
> > -	#define RESULT_DATA_CAP 6688
> > -	camera_metadata_t *resultMetadata =
> > -		allocate_camera_metadata(STATIC_ENTRY_CAP, STATIC_DATA_CAP);
> > +	/*
> > +	 * \todo Keep this in sync with the actual number of entries.
> > +	 * Currently: 13 entries, 36 bytes
> > +	 */
> > +	camera_metadata_t *resultMetadata = allocate_camera_metadata(15, 50);
> >
> >  	const uint8_t ae_state = ANDROID_CONTROL_AE_STATE_CONVERGED;
> >  	ret = add_camera_metadata_entry(resultMetadata, ANDROID_CONTROL_AE_STATE,
> > --
> > 2.23.0
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel@lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
>
> --
> Regards,
> Niklas Söderlund
Laurent Pinchart Sept. 3, 2019, 8:25 p.m. UTC | #3 
Hi Jacopo,

Thank you for the patch.

On Thu, Aug 29, 2019 at 06:53:09PM +0200, Jacopo Mondi wrote:
> Use more opportune sizes for the static metadata pack, and for the
> dynamic metadata sizes which where wrongly set to use the ones defined
> for the static pack.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>

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

> ---
>  src/android/camera_device.cpp | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 02bb6fc5d042..bc8ffed92c61 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -128,11 +128,11 @@ camera_metadata_t *CameraDevice::getStaticMetadata()
>  	 * more.
>  	 */
>  
> -	/* \todo Use correct sizes */
> -	#define STATIC_ENTRY_CAP 256
> -	#define STATIC_DATA_CAP 6688
> -	staticMetadata_ = allocate_camera_metadata(STATIC_ENTRY_CAP,
> -						   STATIC_DATA_CAP);
> +	/*
> +	 * \todo Keep this in sync with the actual number of entries.
> +	 * Currently: 46 entries, 390 bytes
> +	 */
> +	staticMetadata_ = allocate_camera_metadata(50, 500);
>  
>  	/* Color correction static metadata. */
>  	std::vector<uint8_t> aberrationModes = {
> @@ -966,11 +966,11 @@ camera_metadata_t *CameraDevice::getResultMetadata(int frame_number,
>  {
>  	int ret;
>  
> -	/* \todo Use correct sizes */
> -	#define RESULT_ENTRY_CAP 256
> -	#define RESULT_DATA_CAP 6688
> -	camera_metadata_t *resultMetadata =
> -		allocate_camera_metadata(STATIC_ENTRY_CAP, STATIC_DATA_CAP);
> +	/*
> +	 * \todo Keep this in sync with the actual number of entries.
> +	 * Currently: 13 entries, 36 bytes
> +	 */
> +	camera_metadata_t *resultMetadata = allocate_camera_metadata(15, 50);
>  
>  	const uint8_t ae_state = ANDROID_CONTROL_AE_STATE_CONVERGED;
>  	ret = add_camera_metadata_entry(resultMetadata, ANDROID_CONTROL_AE_STATE,

Patch
diff mbox series 

diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index 02bb6fc5d042..bc8ffed92c61 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -128,11 +128,11 @@  camera_metadata_t *CameraDevice::getStaticMetadata()
 	 * more.
 	 */
 
-	/* \todo Use correct sizes */
-	#define STATIC_ENTRY_CAP 256
-	#define STATIC_DATA_CAP 6688
-	staticMetadata_ = allocate_camera_metadata(STATIC_ENTRY_CAP,
-						   STATIC_DATA_CAP);
+	/*
+	 * \todo Keep this in sync with the actual number of entries.
+	 * Currently: 46 entries, 390 bytes
+	 */
+	staticMetadata_ = allocate_camera_metadata(50, 500);
 
 	/* Color correction static metadata. */
 	std::vector<uint8_t> aberrationModes = {
@@ -966,11 +966,11 @@  camera_metadata_t *CameraDevice::getResultMetadata(int frame_number,
 {
 	int ret;
 
-	/* \todo Use correct sizes */
-	#define RESULT_ENTRY_CAP 256
-	#define RESULT_DATA_CAP 6688
-	camera_metadata_t *resultMetadata =
-		allocate_camera_metadata(STATIC_ENTRY_CAP, STATIC_DATA_CAP);
+	/*
+	 * \todo Keep this in sync with the actual number of entries.
+	 * Currently: 13 entries, 36 bytes
+	 */
+	camera_metadata_t *resultMetadata = allocate_camera_metadata(15, 50);
 
 	const uint8_t ae_state = ANDROID_CONTROL_AE_STATE_CONVERGED;
 	ret = add_camera_metadata_entry(resultMetadata, ANDROID_CONTROL_AE_STATE,