[libcamera-devel,3/3] android: camera_device: Report RAW capability if supported
diff mbox series

Message ID 20200728185548.3361465-4-niklas.soderlund@ragnatech.se
State Superseded
Headers show
Series
  • android: camera_device: Add RAW support
Related show

Commit Message

Niklas Söderlund July 28, 2020, 6:55 p.m. UTC 
Probe the libcamera Camera for RAW support and if supported report RAW
capability in the static metadata reported to Android.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 src/android/camera_device.cpp | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Jacopo Mondi July 29, 2020, 8:51 a.m. UTC | #1 
Hi Niklas,

On Tue, Jul 28, 2020 at 08:55:48PM +0200, Niklas Söderlund wrote:
> Probe the libcamera Camera for RAW support and if supported report RAW
> capability in the static metadata reported to Android.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> ---
>  src/android/camera_device.cpp | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 95a563b1aafe86de..76c128564550419a 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -15,6 +15,7 @@
>  #include <libcamera/formats.h>
>  #include <libcamera/property_ids.h>
>
> +#include "libcamera/internal/formats.h"
>  #include "libcamera/internal/log.h"
>  #include "libcamera/internal/utils.h"
>
> @@ -727,6 +728,15 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
>  	std::vector<uint8_t> availableCapabilities = {
>  		ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BACKWARD_COMPATIBLE,
>  	};
> +
> +	/* Report if camera supports RAW. */
> +	std::unique_ptr<CameraConfiguration> cameraConfig =
> +		camera_->generateConfiguration({ StillCaptureRaw });
> +	const PixelFormatInfo &info =
> +		PixelFormatInfo::info(cameraConfig->at(0).pixelFormat);
> +	if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)
> +		availableCapabilities.push_back(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW);
> +

The documentation reports

"RAW (v3.2) [optional]
The camera device supports outputting RAW buffers and metadata for
interpreting them.  Devices supporting the RAW capability allow both
for saving DNG files, and for direct application processing of raw
sensor images.

RAW_SENSOR is supported as an output format.
The maximum available resolution for RAW_SENSOR streams will match
either the value in android.sensor.info.pixelArraySize or
android.sensor.info.preCorrectionActiveArraySize.  All DNG-related
optional metadata entries are provided by the camera device."

Searching the documentation I see a few mentions of RAW_SENSOR and
"supported as an output format" might means that we just need to
report one RAW streams as part of the
ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT property as
we're doing already.

The size check might instead be faulty. Currently the list of
resolutions to test against a format to construct the
streamConfigurations_ output formats map is taken from

	std::unique_ptr<CameraConfiguration> cameraConfig =
		camera_->generateConfiguration({ StillCapture });
	StreamConfiguration &cfg = cameraConfig->at(0);
	const Size maxRes = cfg.size;

Hence, it's not the pixelArraySize. It will, as there's a comment
above the preceeding block that says:
	 * \todo Get this from the camera properties once defined

But at the moment, it's not. I wonder if we should record a todo for
raw or just let this happen.

Anyway, I encourage to run CTS and cros_camera_test once new features
are advertised to the camera service to know if all the required
properties are actually there.

Thanks
  j

>  	staticMetadata_->addEntry(ANDROID_REQUEST_AVAILABLE_CAPABILITIES,
>  				  availableCapabilities.data(),
>  				  availableCapabilities.size());
> --
> 2.27.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Niklas Söderlund July 29, 2020, 11:10 a.m. UTC | #2 
Hi Jacopo,

Thanks for your comments.

On 2020-07-29 10:51:06 +0200, Jacopo Mondi wrote:
> Hi Niklas,
> 
> On Tue, Jul 28, 2020 at 08:55:48PM +0200, Niklas Söderlund wrote:
> > Probe the libcamera Camera for RAW support and if supported report RAW
> > capability in the static metadata reported to Android.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> > ---
> >  src/android/camera_device.cpp | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > index 95a563b1aafe86de..76c128564550419a 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -15,6 +15,7 @@
> >  #include <libcamera/formats.h>
> >  #include <libcamera/property_ids.h>
> >
> > +#include "libcamera/internal/formats.h"
> >  #include "libcamera/internal/log.h"
> >  #include "libcamera/internal/utils.h"
> >
> > @@ -727,6 +728,15 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
> >  	std::vector<uint8_t> availableCapabilities = {
> >  		ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BACKWARD_COMPATIBLE,
> >  	};
> > +
> > +	/* Report if camera supports RAW. */
> > +	std::unique_ptr<CameraConfiguration> cameraConfig =
> > +		camera_->generateConfiguration({ StillCaptureRaw });
> > +	const PixelFormatInfo &info =
> > +		PixelFormatInfo::info(cameraConfig->at(0).pixelFormat);
> > +	if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)
> > +		availableCapabilities.push_back(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW);
> > +
> 
> The documentation reports
> 
> "RAW (v3.2) [optional]
> The camera device supports outputting RAW buffers and metadata for
> interpreting them.  Devices supporting the RAW capability allow both
> for saving DNG files, and for direct application processing of raw
> sensor images.
> 
> RAW_SENSOR is supported as an output format.
> The maximum available resolution for RAW_SENSOR streams will match
> either the value in android.sensor.info.pixelArraySize or
> android.sensor.info.preCorrectionActiveArraySize.  All DNG-related
> optional metadata entries are provided by the camera device."
> 
> Searching the documentation I see a few mentions of RAW_SENSOR and
> "supported as an output format" might means that we just need to
> report one RAW streams as part of the
> ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT property as
> we're doing already.

I was a bit confused about this at first as well. Then after digging 
thru the code it dawned on me, RAW_SENSOR == RAW16. In earlier code only 
RAW16 was supported and the snapshot of the code base we are targeting 
the transition does not seems to be complete in all parts of the 
documentation.

See ANDROID_SCALER_AVAILABLE_FORMATS_RAW16 vs HAL_PIXEL_FORMAT_RAW16. At 
our code point the NDROID_SCALER_AVAILABLE_FORMATS_* are depricated in 
favor of HAL_PIXEL_FORMAT_* but the intersection between the two are 
kept ABI compatible.

> 
> The size check might instead be faulty. Currently the list of
> resolutions to test against a format to construct the
> streamConfigurations_ output formats map is taken from
> 
> 	std::unique_ptr<CameraConfiguration> cameraConfig =
> 		camera_->generateConfiguration({ StillCapture });
> 	StreamConfiguration &cfg = cameraConfig->at(0);
> 	const Size maxRes = cfg.size;
> 
> Hence, it's not the pixelArraySize. It will, as there's a comment
> above the preceeding block that says:
> 	 * \todo Get this from the camera properties once defined
> 
> But at the moment, it's not. I wonder if we should record a todo for
> raw or just let this happen.

I read the TODO and toight it was enough. There are so much that needs 
to be fetched from the camera and I think we should let it happen as a 
larger targeted change.

> 
> Anyway, I encourage to run CTS and cros_camera_test once new features
> are advertised to the camera service to know if all the required
> properties are actually there.
> 
> Thanks
>   j
> 
> >  	staticMetadata_->addEntry(ANDROID_REQUEST_AVAILABLE_CAPABILITIES,
> >  				  availableCapabilities.data(),
> >  				  availableCapabilities.size());
> > --
> > 2.27.0
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel@lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel

Patch
diff mbox series 

diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index 95a563b1aafe86de..76c128564550419a 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -15,6 +15,7 @@ 
 #include <libcamera/formats.h>
 #include <libcamera/property_ids.h>
 
+#include "libcamera/internal/formats.h"
 #include "libcamera/internal/log.h"
 #include "libcamera/internal/utils.h"
 
@@ -727,6 +728,15 @@  const camera_metadata_t *CameraDevice::getStaticMetadata()
 	std::vector<uint8_t> availableCapabilities = {
 		ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BACKWARD_COMPATIBLE,
 	};
+
+	/* Report if camera supports RAW. */
+	std::unique_ptr<CameraConfiguration> cameraConfig =
+		camera_->generateConfiguration({ StillCaptureRaw });
+	const PixelFormatInfo &info =
+		PixelFormatInfo::info(cameraConfig->at(0).pixelFormat);
+	if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)
+		availableCapabilities.push_back(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW);
+
 	staticMetadata_->addEntry(ANDROID_REQUEST_AVAILABLE_CAPABILITIES,
 				  availableCapabilities.data(),
 				  availableCapabilities.size());