[libcamera-devel] [RFC PATCH v2 12/12] android camera_device: Set result metadata from libcamera metadata
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Apr 27 07:21:44 CEST 2021
Hi Paul,
Thank you for the patch.
On Thu, Apr 22, 2021 at 06:41:02PM +0900, Paul Elder wrote:
> Set the result metadata required for FULL hardware level based on the
> metadata returned from libcamera.
I'm afraid I have an annoying request: could you reorder patches to
avoid hardcoding values and only making them dynamic here ? It's hard to
review 07/12 otherwise. It should hopefully not cause conflicts. I think
07/12 and 12/12 should be squashed.
Another option is to keep the current order, and remove from 07/12 the
metadata that is touched in 12/12, keeping only the HAL hardcoded values
in 07/12. Not sure if there's any though, I haven't checked :-)
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
> src/android/camera_device.cpp | 98 ++++++++++++++++-------------------
> src/android/camera_device.h | 2 -
> 2 files changed, 46 insertions(+), 54 deletions(-)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 30692a67..303767e5 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -402,7 +402,7 @@ CameraDevice::Camera3RequestDescriptor::Camera3RequestDescriptor(
>
> CameraDevice::CameraDevice(unsigned int id, std::shared_ptr<Camera> camera)
> : id_(id), running_(false), camera_(std::move(camera)),
> - facing_(CAMERA_FACING_FRONT), orientation_(0), lastTimestamp_(0)
> + facing_(CAMERA_FACING_FRONT), orientation_(0)
> {
> camera_->requestCompleted.connect(this, &CameraDevice::requestComplete);
>
> @@ -2326,10 +2326,6 @@ void CameraDevice::requestComplete(Request *request)
>
> resultMetadata = getResultMetadata(descriptor);
>
> - const ControlList &metadata = descriptor->request_->metadata();
> - if (metadata.contains(controls::SensorTimestamp))
> - lastTimestamp_ = metadata.get(controls::SensorTimestamp);
> -
> /* Handle any JPEG compression. */
> for (camera3_stream_buffer_t &buffer : descriptor.buffers_) {
> CameraStream *cameraStream =
> @@ -2450,7 +2446,6 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons
> const ControlList &metadata = descriptor.request_->metadata();
> const CameraMetadata &settings = descriptor.settings_;
> camera_metadata_ro_entry_t entry;
> - bool found;
>
> /*
> * \todo Keep this in sync with the actual number of entries.
> @@ -2482,9 +2477,10 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons
> */
>
> /* FULL */
> - found = settings.getEntry(ANDROID_BLACK_LEVEL_LOCK, &entry);
> - bool valueBool = found ? *entry.data.u8 : false;
> - resultMetadata->addEntry(ANDROID_BLACK_LEVEL_LOCK, &valueBool, 1);
> + if (metadata.contains(controls::draft::BlackLevelLocked)) {
> + bool valueBool = metadata.get(controls::draft::BlackLevelLocked);
> + resultMetadata->addEntry(ANDROID_BLACK_LEVEL_LOCK, &valueBool, 1);
> + }
>
> uint8_t value = ANDROID_COLOR_CORRECTION_ABERRATION_MODE_OFF;
> resultMetadata->addEntry(ANDROID_COLOR_CORRECTION_ABERRATION_MODE,
> @@ -2497,11 +2493,10 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons
> resultMetadata->addEntry(ANDROID_CONTROL_AE_EXPOSURE_COMPENSATION,
> &value32, 1);
>
> - /* \todo apply this */
> - value = ANDROID_CONTROL_AE_LOCK_OFF;
> - found = settings.getEntry(ANDROID_CONTROL_AE_LOCK, &entry);
> - resultMetadata->addEntry(ANDROID_CONTROL_AE_LOCK,
> - found ? entry.data.u8 : &value, 1);
> + if (metadata.contains(controls::AeLocked)) {
> + value = metadata.get(controls::AeLocked);
> + resultMetadata->addEntry(ANDROID_CONTROL_AE_LOCK, &value, 1);
> + }
>
> value = ANDROID_CONTROL_AE_MODE_ON;
> resultMetadata->addEntry(ANDROID_CONTROL_AE_MODE, &value, 1);
> @@ -2515,10 +2510,11 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons
> resultMetadata->addEntry(ANDROID_CONTROL_AE_TARGET_FPS_RANGE,
> entry.data.i32, 2);
>
> - value = ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER_IDLE;
> - found = settings.getEntry(ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER, &entry);
> - resultMetadata->addEntry(ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER,
> - found ? entry.data.u8 : &value, 1);
> + if (metadata.contains(controls::draft::AePrecaptureTrigger)) {
> + value = metadata.get(controls::draft::AePrecaptureTrigger);
> + resultMetadata->addEntry(ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER,
> + &value, 1);
> + }
>
> value = ANDROID_CONTROL_AE_STATE_CONVERGED;
> resultMetadata->addEntry(ANDROID_CONTROL_AE_STATE, &value, 1);
> @@ -2532,14 +2528,15 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons
> value = ANDROID_CONTROL_AF_TRIGGER_IDLE;
> resultMetadata->addEntry(ANDROID_CONTROL_AF_TRIGGER, &value, 1);
>
> - value = ANDROID_CONTROL_AWB_MODE_AUTO;
> - found = settings.getEntry(ANDROID_CONTROL_AWB_MODE, &entry);
> - resultMetadata->addEntry(ANDROID_CONTROL_AWB_MODE,
> - found ? entry.data.u8 : &value, 1);
> + if (metadata.contains(controls::AwbMode)) {
> + value = metadata.get(controls::AwbMode);
> + resultMetadata->addEntry(ANDROID_CONTROL_AWB_MODE, &value, 1);
> + }
>
> - found = settings.getEntry(ANDROID_CONTROL_AWB_LOCK, &entry);
> - value = found ? *entry.data.u8 : ANDROID_CONTROL_AWB_LOCK_OFF;
> - resultMetadata->addEntry(ANDROID_CONTROL_AWB_LOCK, &value, 1);
> + if (metadata.contains(controls::AwbLocked)) {
> + value = metadata.get(controls::AwbLocked);
> + resultMetadata->addEntry(ANDROID_CONTROL_AWB_LOCK, &value, 1);
> + }
>
> value = value ? ANDROID_CONTROL_AWB_STATE_LOCKED :
> ANDROID_CONTROL_AWB_STATE_CONVERGED;
> @@ -2560,9 +2557,10 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons
> value = ANDROID_CONTROL_VIDEO_STABILIZATION_MODE_OFF;
> resultMetadata->addEntry(ANDROID_CONTROL_VIDEO_STABILIZATION_MODE, &value, 1);
>
> - found = settings.getEntry(ANDROID_EDGE_MODE, &entry);
> - value = found ? *entry.data.u8 : ANDROID_EDGE_MODE_OFF;
> - resultMetadata->addEntry(ANDROID_EDGE_MODE, &value, 1);
> + if (metadata.contains(controls::draft::EdgeMode)) {
> + value = metadata.get(controls::draft::EdgeMode);
> + resultMetadata->addEntry(ANDROID_EDGE_MODE, &value, 1);
> + }
>
> value = ANDROID_FLASH_MODE_OFF;
> resultMetadata->addEntry(ANDROID_FLASH_MODE, &value, 1);
> @@ -2623,15 +2621,16 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons
> resultMetadata->addEntry(ANDROID_STATISTICS_SCENE_FLICKER,
> &value, 1);
>
> - /* \todo handle this */
> - found = settings.getEntry(ANDROID_TONEMAP_MODE, &entry);
> - value = found ? *entry.data.u8 : ANDROID_TONEMAP_MODE_FAST;
> - resultMetadata->addEntry(ANDROID_TONEMAP_MODE, &value, 1);
> + if (metadata.contains(controls::draft::TonemapMode)) {
> + value = metadata.get(controls::draft::TonemapMode);
> + resultMetadata->addEntry(ANDROID_TONEMAP_MODE, &value, 1);
> + }
>
> - value = ANDROID_NOISE_REDUCTION_MODE_OFF;
> - found = settings.getEntry(ANDROID_NOISE_REDUCTION_MODE, &entry);
> - resultMetadata->addEntry(ANDROID_NOISE_REDUCTION_MODE,
> - found ? entry.data.u8 : &value, 1);
> + if (metadata.contains(controls::draft::NoiseReductionMode)) {
> + value = metadata.get(controls::draft::NoiseReductionMode);
> + resultMetadata->addEntry(ANDROID_NOISE_REDUCTION_MODE,
> + &value, 1);
> + }
>
> /* 33.3 msec */
> const int64_t rolling_shutter_skew = 33300000;
> @@ -2659,11 +2658,11 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons
> * and copy to result
> */
>
> - /* \todo get this from camera */
> - value32 = 32;
> - found = settings.getEntry(ANDROID_SENSOR_SENSITIVITY, &entry);
> - resultMetadata->addEntry(ANDROID_SENSOR_SENSITIVITY,
> - found ? entry.data.i32 : &value32, 1);
> + if (metadata.contains(controls::draft::SensorSensitivity)) {
> + value32 = metadata.get(controls::draft::SensorSensitivity);
> + resultMetadata->addEntry(ANDROID_SENSOR_SENSITIVITY,
> + &value32, 1);
> + }
>
> /* Add metadata tags reported by libcamera. */
> if (metadata.contains(controls::draft::PipelineDepth)) {
> @@ -2673,26 +2672,21 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons
> &pipeline_depth, 1);
> }
>
> - found = settings.getEntry(ANDROID_SENSOR_EXPOSURE_TIME, &entry);
> - if (found || metadata.contains(controls::ExposureTime)) {
> + if (metadata.contains(controls::ExposureTime)) {
> int64_t exposure = metadata.get(controls::ExposureTime) * 1000ULL;
> resultMetadata->addEntry(ANDROID_SENSOR_EXPOSURE_TIME,
> - found ? entry.data.i64 : &exposure, 1);
> + &exposure, 1);
> }
>
> if (metadata.contains(controls::SensorTimestamp)) {
> int64_t timestamp = metadata.get(controls::SensorTimestamp);
> resultMetadata->addEntry(ANDROID_SENSOR_TIMESTAMP, ×tamp, 1);
> + }
>
> - int64_t frameDuration = timestamp - lastTimestamp_;
> - /*
> - * frame duration should be at last as long as the requested
> - * exposure time, hardcode it for now
> - */
> - if (found && frameDuration < *entry.data.i64)
> - frameDuration = *entry.data.i64;
> + if (metadata.contains(controls::draft::FrameDuration)) {
> + int64_t duration = metadata.get(controls::draft::FrameDuration);
> resultMetadata->addEntry(ANDROID_SENSOR_FRAME_DURATION,
> - &frameDuration, 1);
> + &duration, 1);
> }
>
> if (metadata.contains(controls::ScalerCrop)) {
> diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> index fcd57fcd..8edbcdfd 100644
> --- a/src/android/camera_device.h
> +++ b/src/android/camera_device.h
> @@ -139,8 +139,6 @@ private:
>
> unsigned int maxJpegBufferSize_;
>
> - int64_t lastTimestamp_;
> -
> CameraMetadata lastSettings_;
> };
>
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list