| Message ID | 20210422094102.371772-13-paul.elder@ideasonboard.com |
|---|---|
| State | Superseded |
| Delegated to: | Paul Elder |
| Headers | show |
| Series |
|
| Related | show |
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@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_; > }; >
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_; };
Set the result metadata required for FULL hardware level based on the metadata returned from libcamera. Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> --- src/android/camera_device.cpp | 98 ++++++++++++++++------------------- src/android/camera_device.h | 2 - 2 files changed, 46 insertions(+), 54 deletions(-)