[libcamera-devel,v2,2/2] android: camera_device: Report number of out streams
diff mbox series

Message ID 20201217140358.17614-3-jacopo@jmondi.org
State Accepted
Delegated to: Jacopo Mondi
Headers show
Series
  • android: Report
Related show

Commit Message

Jacopo Mondi Dec. 17, 2020, 2:03 p.m. UTC 
Report the number of supported output streams through the
ANDROID_REQUEST_MAX_NUM_OUTPUT_STREAMS static metadata.

The camera HAL currently supports:
- 1 optional RAW stream
- 2 YUV streams
- 1 JPEG stream

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/android/camera_device.cpp | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

Comments

Niklas Söderlund Dec. 18, 2020, 2:44 p.m. UTC | #1 
Hi Jacopo,

Thanks for your patch.

On 2020-12-17 15:03:58 +0100, Jacopo Mondi wrote:
> Report the number of supported output streams through the
> ANDROID_REQUEST_MAX_NUM_OUTPUT_STREAMS static metadata.
> 
> The camera HAL currently supports:
> - 1 optional RAW stream
> - 2 YUV streams
> - 1 JPEG stream
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 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 26aa3bc4b123..b4618a680f41 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -675,10 +675,10 @@ std::tuple<uint32_t, uint32_t> CameraDevice::calculateStaticMetadataSize()
>  {
>  	/*
>  	 * \todo Keep this in sync with the actual number of entries.
> -	 * Currently: 52 entries, 698 bytes of static metadata
> +	 * Currently: 53 entries, 714 bytes of static metadata
>  	 */
> -	uint32_t numEntries = 52;
> -	uint32_t byteSize = 698;
> +	uint32_t numEntries = 53;
> +	uint32_t byteSize = 714;
>  
>  	/*
>  	 * Calculate space occupation in bytes for dynamically built metadata
> @@ -1090,15 +1090,23 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
>  	};
>  
>  	/* Report if camera supports RAW. */
> +	bool rawStreamAvailable = false;
>  	std::unique_ptr<CameraConfiguration> cameraConfig =
>  		camera_->generateConfiguration({ StreamRole::Raw });
>  	if (cameraConfig && !cameraConfig->empty()) {
>  		const PixelFormatInfo &info =
>  			PixelFormatInfo::info(cameraConfig->at(0).pixelFormat);
> -		if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)
> +		if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW) {
> +			rawStreamAvailable = true;

I wonder if we should make this check more restrictive and check for 
RAW16 instead of any RAW format?

>  			availableCapabilities.push_back(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW);
> +		}
>  	}
>  
> +	/* Number of { RAW, YUV, JPEG } supported output streams */
> +	int32_t numOutStreams[] = { rawStreamAvailable, 2, 1 };
> +	staticMetadata_->addEntry(ANDROID_REQUEST_MAX_NUM_OUTPUT_STREAMS,
> +				  &numOutStreams, 3);
> +
>  	staticMetadata_->addEntry(ANDROID_REQUEST_AVAILABLE_CAPABILITIES,
>  				  availableCapabilities.data(),
>  				  availableCapabilities.size());
> @@ -1150,6 +1158,7 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
>  		ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL,
>  		ANDROID_REQUEST_PARTIAL_RESULT_COUNT,
>  		ANDROID_REQUEST_PIPELINE_MAX_DEPTH,
> +		ANDROID_REQUEST_MAX_NUM_OUTPUT_STREAMS,
>  		ANDROID_REQUEST_MAX_NUM_INPUT_STREAMS,
>  		ANDROID_REQUEST_AVAILABLE_CAPABILITIES,
>  	};
> -- 
> 2.29.2
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Jacopo Mondi Dec. 18, 2020, 2:49 p.m. UTC | #2 
Hi Niklas,

On Fri, Dec 18, 2020 at 03:44:16PM +0100, Niklas Söderlund wrote:
> Hi Jacopo,
>
> Thanks for your patch.
>
> On 2020-12-17 15:03:58 +0100, Jacopo Mondi wrote:
> > Report the number of supported output streams through the
> > ANDROID_REQUEST_MAX_NUM_OUTPUT_STREAMS static metadata.
> >
> > The camera HAL currently supports:
> > - 1 optional RAW stream
> > - 2 YUV streams
> > - 1 JPEG stream
> >
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > 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 26aa3bc4b123..b4618a680f41 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -675,10 +675,10 @@ std::tuple<uint32_t, uint32_t> CameraDevice::calculateStaticMetadataSize()
> >  {
> >  	/*
> >  	 * \todo Keep this in sync with the actual number of entries.
> > -	 * Currently: 52 entries, 698 bytes of static metadata
> > +	 * Currently: 53 entries, 714 bytes of static metadata
> >  	 */
> > -	uint32_t numEntries = 52;
> > -	uint32_t byteSize = 698;
> > +	uint32_t numEntries = 53;
> > +	uint32_t byteSize = 714;
> >
> >  	/*
> >  	 * Calculate space occupation in bytes for dynamically built metadata
> > @@ -1090,15 +1090,23 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
> >  	};
> >
> >  	/* Report if camera supports RAW. */
> > +	bool rawStreamAvailable = false;
> >  	std::unique_ptr<CameraConfiguration> cameraConfig =
> >  		camera_->generateConfiguration({ StreamRole::Raw });
> >  	if (cameraConfig && !cameraConfig->empty()) {
> >  		const PixelFormatInfo &info =
> >  			PixelFormatInfo::info(cameraConfig->at(0).pixelFormat);
> > -		if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)
> > +		if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW) {
> > +			rawStreamAvailable = true;
>
> I wonder if we should make this check more restrictive and check for
> RAW16 instead of any RAW format?


Yes, probably, that's related to the question of which formats should
be supported in order to claim support for RAW to Android.

I think this is unrelated to this patch, isn't it ?

>
> >  			availableCapabilities.push_back(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW);
> > +		}
> >  	}
> >
> > +	/* Number of { RAW, YUV, JPEG } supported output streams */
> > +	int32_t numOutStreams[] = { rawStreamAvailable, 2, 1 };
> > +	staticMetadata_->addEntry(ANDROID_REQUEST_MAX_NUM_OUTPUT_STREAMS,
> > +				  &numOutStreams, 3);
> > +
> >  	staticMetadata_->addEntry(ANDROID_REQUEST_AVAILABLE_CAPABILITIES,
> >  				  availableCapabilities.data(),
> >  				  availableCapabilities.size());
> > @@ -1150,6 +1158,7 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
> >  		ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL,
> >  		ANDROID_REQUEST_PARTIAL_RESULT_COUNT,
> >  		ANDROID_REQUEST_PIPELINE_MAX_DEPTH,
> > +		ANDROID_REQUEST_MAX_NUM_OUTPUT_STREAMS,
> >  		ANDROID_REQUEST_MAX_NUM_INPUT_STREAMS,
> >  		ANDROID_REQUEST_AVAILABLE_CAPABILITIES,
> >  	};
> > --
> > 2.29.2
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel@lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
>
> --
> Regards,
> Niklas Söderlund
Niklas Söderlund Dec. 18, 2020, 2:54 p.m. UTC | #3 
Hi Jacopo,

On 2020-12-18 15:49:43 +0100, Jacopo Mondi wrote:
> Hi Niklas,
> 
> On Fri, Dec 18, 2020 at 03:44:16PM +0100, Niklas Söderlund wrote:
> > Hi Jacopo,
> >
> > Thanks for your patch.
> >
> > On 2020-12-17 15:03:58 +0100, Jacopo Mondi wrote:
> > > Report the number of supported output streams through the
> > > ANDROID_REQUEST_MAX_NUM_OUTPUT_STREAMS static metadata.
> > >
> > > The camera HAL currently supports:
> > > - 1 optional RAW stream
> > > - 2 YUV streams
> > > - 1 JPEG stream
> > >
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > > 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 26aa3bc4b123..b4618a680f41 100644
> > > --- a/src/android/camera_device.cpp
> > > +++ b/src/android/camera_device.cpp
> > > @@ -675,10 +675,10 @@ std::tuple<uint32_t, uint32_t> CameraDevice::calculateStaticMetadataSize()
> > >  {
> > >  	/*
> > >  	 * \todo Keep this in sync with the actual number of entries.
> > > -	 * Currently: 52 entries, 698 bytes of static metadata
> > > +	 * Currently: 53 entries, 714 bytes of static metadata
> > >  	 */
> > > -	uint32_t numEntries = 52;
> > > -	uint32_t byteSize = 698;
> > > +	uint32_t numEntries = 53;
> > > +	uint32_t byteSize = 714;
> > >
> > >  	/*
> > >  	 * Calculate space occupation in bytes for dynamically built metadata
> > > @@ -1090,15 +1090,23 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
> > >  	};
> > >
> > >  	/* Report if camera supports RAW. */
> > > +	bool rawStreamAvailable = false;
> > >  	std::unique_ptr<CameraConfiguration> cameraConfig =
> > >  		camera_->generateConfiguration({ StreamRole::Raw });
> > >  	if (cameraConfig && !cameraConfig->empty()) {
> > >  		const PixelFormatInfo &info =
> > >  			PixelFormatInfo::info(cameraConfig->at(0).pixelFormat);
> > > -		if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)
> > > +		if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW) {
> > > +			rawStreamAvailable = true;
> >
> > I wonder if we should make this check more restrictive and check for
> > RAW16 instead of any RAW format?
> 
> 
> Yes, probably, that's related to the question of which formats should
> be supported in order to claim support for RAW to Android.
> 
> I think this is unrelated to this patch, isn't it ?

Maybe it is, I was just thinking a head as I hope to soon tackle RAW and 
CTS so the tests for RAW that require RAW16 are skipped correctly. But 
as I don't know yet how to best do that this is a good step in the right 
direction.

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> 
> >
> > >  			availableCapabilities.push_back(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW);
> > > +		}
> > >  	}
> > >
> > > +	/* Number of { RAW, YUV, JPEG } supported output streams */
> > > +	int32_t numOutStreams[] = { rawStreamAvailable, 2, 1 };
> > > +	staticMetadata_->addEntry(ANDROID_REQUEST_MAX_NUM_OUTPUT_STREAMS,
> > > +				  &numOutStreams, 3);
> > > +
> > >  	staticMetadata_->addEntry(ANDROID_REQUEST_AVAILABLE_CAPABILITIES,
> > >  				  availableCapabilities.data(),
> > >  				  availableCapabilities.size());
> > > @@ -1150,6 +1158,7 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
> > >  		ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL,
> > >  		ANDROID_REQUEST_PARTIAL_RESULT_COUNT,
> > >  		ANDROID_REQUEST_PIPELINE_MAX_DEPTH,
> > > +		ANDROID_REQUEST_MAX_NUM_OUTPUT_STREAMS,
> > >  		ANDROID_REQUEST_MAX_NUM_INPUT_STREAMS,
> > >  		ANDROID_REQUEST_AVAILABLE_CAPABILITIES,
> > >  	};
> > > --
> > > 2.29.2
> > >
> > > _______________________________________________
> > > libcamera-devel mailing list
> > > libcamera-devel@lists.libcamera.org
> > > https://lists.libcamera.org/listinfo/libcamera-devel
> >
> > --
> > Regards,
> > Niklas Söderlund

Patch
diff mbox series 

diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index 26aa3bc4b123..b4618a680f41 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -675,10 +675,10 @@  std::tuple<uint32_t, uint32_t> CameraDevice::calculateStaticMetadataSize()
 {
 	/*
 	 * \todo Keep this in sync with the actual number of entries.
-	 * Currently: 52 entries, 698 bytes of static metadata
+	 * Currently: 53 entries, 714 bytes of static metadata
 	 */
-	uint32_t numEntries = 52;
-	uint32_t byteSize = 698;
+	uint32_t numEntries = 53;
+	uint32_t byteSize = 714;
 
 	/*
 	 * Calculate space occupation in bytes for dynamically built metadata
@@ -1090,15 +1090,23 @@  const camera_metadata_t *CameraDevice::getStaticMetadata()
 	};
 
 	/* Report if camera supports RAW. */
+	bool rawStreamAvailable = false;
 	std::unique_ptr<CameraConfiguration> cameraConfig =
 		camera_->generateConfiguration({ StreamRole::Raw });
 	if (cameraConfig && !cameraConfig->empty()) {
 		const PixelFormatInfo &info =
 			PixelFormatInfo::info(cameraConfig->at(0).pixelFormat);
-		if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)
+		if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW) {
+			rawStreamAvailable = true;
 			availableCapabilities.push_back(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW);
+		}
 	}
 
+	/* Number of { RAW, YUV, JPEG } supported output streams */
+	int32_t numOutStreams[] = { rawStreamAvailable, 2, 1 };
+	staticMetadata_->addEntry(ANDROID_REQUEST_MAX_NUM_OUTPUT_STREAMS,
+				  &numOutStreams, 3);
+
 	staticMetadata_->addEntry(ANDROID_REQUEST_AVAILABLE_CAPABILITIES,
 				  availableCapabilities.data(),
 				  availableCapabilities.size());
@@ -1150,6 +1158,7 @@  const camera_metadata_t *CameraDevice::getStaticMetadata()
 		ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL,
 		ANDROID_REQUEST_PARTIAL_RESULT_COUNT,
 		ANDROID_REQUEST_PIPELINE_MAX_DEPTH,
+		ANDROID_REQUEST_MAX_NUM_OUTPUT_STREAMS,
 		ANDROID_REQUEST_MAX_NUM_INPUT_STREAMS,
 		ANDROID_REQUEST_AVAILABLE_CAPABILITIES,
 	};