| Message ID | 20210422094102.371772-12-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:01PM +0900, Paul Elder wrote: > Set the request metadata as required by FULL hardware level. Let's document that this isn't the final implementation, controls need to be plumbed to the IPA. > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > --- > src/libcamera/pipeline/ipu3/ipu3.cpp | 67 ++++++++++++++++++++++++++-- > 1 file changed, 64 insertions(+), 3 deletions(-) > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index 70a5e9ce..de90b9fe 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -75,7 +75,8 @@ class IPU3CameraData : public CameraData > { > public: > IPU3CameraData(PipelineHandler *pipe) > - : CameraData(pipe), exposureTime_(0), supportsFlips_(false) > + : CameraData(pipe), exposureTime_(0), supportsFlips_(false), > + lastTimestamp_(0) > { > } > > @@ -106,6 +107,8 @@ public: > private: > void queueFrameAction(unsigned int id, > const ipa::ipu3::IPU3Action &action); > + > + int64_t lastTimestamp_; > }; > > class IPU3CameraConfiguration : public CameraConfiguration > @@ -1249,12 +1252,65 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer) > > request->metadata().set(controls::draft::PipelineDepth, 3); > /* \todo Move the ExposureTime control to the IPA. */ > - request->metadata().set(controls::ExposureTime, exposureTime_); > + request->metadata().set(controls::ExposureTime, > + request->controls().contains(controls::ExposureTime) ? > + request->controls().get(controls::ExposureTime) : > + exposureTime_); > /* \todo Actually apply the scaler crop region to the ImgU. */ > if (request->controls().contains(controls::ScalerCrop)) > cropRegion_ = request->controls().get(controls::ScalerCrop); > request->metadata().set(controls::ScalerCrop, cropRegion_); > > + request->metadata().set(controls::draft::BlackLevelLocked, > + request->controls().contains(controls::draft::BlackLevelLocked) ? > + request->controls().get(controls::draft::BlackLevelLocked) : > + false); Lots of copy & paste boilerplate. It should ideally be replaced with a (template) helper function. As this will go away anyway I'm OK with it for the time being. > + > + request->metadata().set(controls::AeLocked, > + request->controls().contains(controls::AeLocked) ? > + request->controls().get(controls::AeLocked) : > + false); > + > + request->metadata().set(controls::draft::AePrecaptureTrigger, > + request->controls().contains(controls::draft::AePrecaptureTrigger) ? > + request->controls().get(controls::draft::AePrecaptureTrigger) : > + controls::draft::AePrecaptureTriggerIdle); > + > + request->metadata().set(controls::AwbMode, > + request->controls().contains(controls::AwbMode) ? > + request->controls().get(controls::AwbMode) : > + controls::AwbAuto); > + > + request->metadata().set(controls::AwbLocked, > + request->controls().contains(controls::AwbLocked) ? > + request->controls().get(controls::AwbLocked) : > + false); > + > + request->metadata().set(controls::draft::EdgeMode, > + request->controls().contains(controls::draft::EdgeMode) ? > + request->controls().get(controls::draft::EdgeMode) : > + (uint8_t)controls::draft::EdgeModeOff); > + > + request->metadata().set(controls::draft::NoiseReductionMode, > + request->controls().contains(controls::draft::NoiseReductionMode) ? > + request->controls().get(controls::draft::NoiseReductionMode) : > + controls::draft::NoiseReductionModeOff); > + > + request->metadata().set(controls::draft::SensorSensitivity, > + request->controls().contains(controls::draft::SensorSensitivity) ? > + request->controls().get(controls::draft::SensorSensitivity) : > + 32); > + > + if (request->metadata().get(controls::draft::FrameDuration) < > + request->metadata().get(controls::ExposureTime) * 1000) > + request->metadata().set(controls::draft::FrameDuration, > + request->metadata().get(controls::ExposureTime) * 1000); > + > + request->metadata().set(controls::draft::TonemapMode, > + request->controls().contains(controls::draft::TonemapMode) ? > + request->controls().get(controls::draft::TonemapMode) : > + (uint8_t)controls::draft::TonemapModeFast); > + > if (frameInfos_.tryComplete(info)) > pipe_->completeRequest(request); > } > @@ -1280,8 +1336,13 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer) > * \todo The sensor timestamp should be better estimated by connecting > * to the V4L2Device::frameStart signal. > */ > + int64_t timestamp = buffer->metadata().timestamp; > request->metadata().set(controls::SensorTimestamp, > - buffer->metadata().timestamp); > + timestamp); > + > + request->metadata().set(controls::draft::FrameDuration, > + timestamp - lastTimestamp_); > + lastTimestamp_ = timestamp; Same comment as before, this should not be measured. > > /* If the buffer is cancelled force a complete of the whole request. */ > if (buffer->metadata().status == FrameMetadata::FrameCancelled) {
Hi Paul, On Thu, Apr 22, 2021 at 06:41:01PM +0900, Paul Elder wrote: > Set the request metadata as required by FULL hardware level. > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > --- > src/libcamera/pipeline/ipu3/ipu3.cpp | 67 ++++++++++++++++++++++++++-- > 1 file changed, 64 insertions(+), 3 deletions(-) > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index 70a5e9ce..de90b9fe 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -75,7 +75,8 @@ class IPU3CameraData : public CameraData > { > public: > IPU3CameraData(PipelineHandler *pipe) > - : CameraData(pipe), exposureTime_(0), supportsFlips_(false) > + : CameraData(pipe), exposureTime_(0), supportsFlips_(false), > + lastTimestamp_(0) > { > } > > @@ -106,6 +107,8 @@ public: > private: > void queueFrameAction(unsigned int id, > const ipa::ipu3::IPU3Action &action); > + > + int64_t lastTimestamp_; > }; > > class IPU3CameraConfiguration : public CameraConfiguration > @@ -1249,12 +1252,65 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer) > > request->metadata().set(controls::draft::PipelineDepth, 3); > /* \todo Move the ExposureTime control to the IPA. */ > - request->metadata().set(controls::ExposureTime, exposureTime_); > + request->metadata().set(controls::ExposureTime, > + request->controls().contains(controls::ExposureTime) ? > + request->controls().get(controls::ExposureTime) : > + exposureTime_); > /* \todo Actually apply the scaler crop region to the ImgU. */ > if (request->controls().contains(controls::ScalerCrop)) > cropRegion_ = request->controls().get(controls::ScalerCrop); > request->metadata().set(controls::ScalerCrop, cropRegion_); > > + request->metadata().set(controls::draft::BlackLevelLocked, > + request->controls().contains(controls::draft::BlackLevelLocked) ? > + request->controls().get(controls::draft::BlackLevelLocked) : > + false); I feel like we're missing a policy here What is the meaning of: Frame Control Present? Value 1 BlackLevelLocked yes true 2 BlackLevelLocked no 3 BlackLevelLocked yes false Should the black level lock be held in frame 2) or should be removed ? Do we want application to -always- specify all the control values, or do we assume if a control is not there it has not changed value ? This has impacts on the IPA implementation too, let alone the HAL (would be interesting how gstreamer behaves, for example) Thanks j > + > + request->metadata().set(controls::AeLocked, > + request->controls().contains(controls::AeLocked) ? > + request->controls().get(controls::AeLocked) : > + false); > + > + request->metadata().set(controls::draft::AePrecaptureTrigger, > + request->controls().contains(controls::draft::AePrecaptureTrigger) ? > + request->controls().get(controls::draft::AePrecaptureTrigger) : > + controls::draft::AePrecaptureTriggerIdle); > + > + request->metadata().set(controls::AwbMode, > + request->controls().contains(controls::AwbMode) ? > + request->controls().get(controls::AwbMode) : > + controls::AwbAuto); > + > + request->metadata().set(controls::AwbLocked, > + request->controls().contains(controls::AwbLocked) ? > + request->controls().get(controls::AwbLocked) : > + false); > + > + request->metadata().set(controls::draft::EdgeMode, > + request->controls().contains(controls::draft::EdgeMode) ? > + request->controls().get(controls::draft::EdgeMode) : > + (uint8_t)controls::draft::EdgeModeOff); > + > + request->metadata().set(controls::draft::NoiseReductionMode, > + request->controls().contains(controls::draft::NoiseReductionMode) ? > + request->controls().get(controls::draft::NoiseReductionMode) : > + controls::draft::NoiseReductionModeOff); > + > + request->metadata().set(controls::draft::SensorSensitivity, > + request->controls().contains(controls::draft::SensorSensitivity) ? > + request->controls().get(controls::draft::SensorSensitivity) : > + 32); > + > + if (request->metadata().get(controls::draft::FrameDuration) < > + request->metadata().get(controls::ExposureTime) * 1000) > + request->metadata().set(controls::draft::FrameDuration, > + request->metadata().get(controls::ExposureTime) * 1000); > + > + request->metadata().set(controls::draft::TonemapMode, > + request->controls().contains(controls::draft::TonemapMode) ? > + request->controls().get(controls::draft::TonemapMode) : > + (uint8_t)controls::draft::TonemapModeFast); > + > if (frameInfos_.tryComplete(info)) > pipe_->completeRequest(request); > } > @@ -1280,8 +1336,13 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer) > * \todo The sensor timestamp should be better estimated by connecting > * to the V4L2Device::frameStart signal. > */ > + int64_t timestamp = buffer->metadata().timestamp; > request->metadata().set(controls::SensorTimestamp, > - buffer->metadata().timestamp); > + timestamp); > + > + request->metadata().set(controls::draft::FrameDuration, > + timestamp - lastTimestamp_); > + lastTimestamp_ = timestamp; > > /* If the buffer is cancelled force a complete of the whole request. */ > if (buffer->metadata().status == FrameMetadata::FrameCancelled) { > -- > 2.27.0 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 70a5e9ce..de90b9fe 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -75,7 +75,8 @@ class IPU3CameraData : public CameraData { public: IPU3CameraData(PipelineHandler *pipe) - : CameraData(pipe), exposureTime_(0), supportsFlips_(false) + : CameraData(pipe), exposureTime_(0), supportsFlips_(false), + lastTimestamp_(0) { } @@ -106,6 +107,8 @@ public: private: void queueFrameAction(unsigned int id, const ipa::ipu3::IPU3Action &action); + + int64_t lastTimestamp_; }; class IPU3CameraConfiguration : public CameraConfiguration @@ -1249,12 +1252,65 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer) request->metadata().set(controls::draft::PipelineDepth, 3); /* \todo Move the ExposureTime control to the IPA. */ - request->metadata().set(controls::ExposureTime, exposureTime_); + request->metadata().set(controls::ExposureTime, + request->controls().contains(controls::ExposureTime) ? + request->controls().get(controls::ExposureTime) : + exposureTime_); /* \todo Actually apply the scaler crop region to the ImgU. */ if (request->controls().contains(controls::ScalerCrop)) cropRegion_ = request->controls().get(controls::ScalerCrop); request->metadata().set(controls::ScalerCrop, cropRegion_); + request->metadata().set(controls::draft::BlackLevelLocked, + request->controls().contains(controls::draft::BlackLevelLocked) ? + request->controls().get(controls::draft::BlackLevelLocked) : + false); + + request->metadata().set(controls::AeLocked, + request->controls().contains(controls::AeLocked) ? + request->controls().get(controls::AeLocked) : + false); + + request->metadata().set(controls::draft::AePrecaptureTrigger, + request->controls().contains(controls::draft::AePrecaptureTrigger) ? + request->controls().get(controls::draft::AePrecaptureTrigger) : + controls::draft::AePrecaptureTriggerIdle); + + request->metadata().set(controls::AwbMode, + request->controls().contains(controls::AwbMode) ? + request->controls().get(controls::AwbMode) : + controls::AwbAuto); + + request->metadata().set(controls::AwbLocked, + request->controls().contains(controls::AwbLocked) ? + request->controls().get(controls::AwbLocked) : + false); + + request->metadata().set(controls::draft::EdgeMode, + request->controls().contains(controls::draft::EdgeMode) ? + request->controls().get(controls::draft::EdgeMode) : + (uint8_t)controls::draft::EdgeModeOff); + + request->metadata().set(controls::draft::NoiseReductionMode, + request->controls().contains(controls::draft::NoiseReductionMode) ? + request->controls().get(controls::draft::NoiseReductionMode) : + controls::draft::NoiseReductionModeOff); + + request->metadata().set(controls::draft::SensorSensitivity, + request->controls().contains(controls::draft::SensorSensitivity) ? + request->controls().get(controls::draft::SensorSensitivity) : + 32); + + if (request->metadata().get(controls::draft::FrameDuration) < + request->metadata().get(controls::ExposureTime) * 1000) + request->metadata().set(controls::draft::FrameDuration, + request->metadata().get(controls::ExposureTime) * 1000); + + request->metadata().set(controls::draft::TonemapMode, + request->controls().contains(controls::draft::TonemapMode) ? + request->controls().get(controls::draft::TonemapMode) : + (uint8_t)controls::draft::TonemapModeFast); + if (frameInfos_.tryComplete(info)) pipe_->completeRequest(request); } @@ -1280,8 +1336,13 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer) * \todo The sensor timestamp should be better estimated by connecting * to the V4L2Device::frameStart signal. */ + int64_t timestamp = buffer->metadata().timestamp; request->metadata().set(controls::SensorTimestamp, - buffer->metadata().timestamp); + timestamp); + + request->metadata().set(controls::draft::FrameDuration, + timestamp - lastTimestamp_); + lastTimestamp_ = timestamp; /* If the buffer is cancelled force a complete of the whole request. */ if (buffer->metadata().status == FrameMetadata::FrameCancelled) {
Set the request metadata as required by FULL hardware level. Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> --- src/libcamera/pipeline/ipu3/ipu3.cpp | 67 ++++++++++++++++++++++++++-- 1 file changed, 64 insertions(+), 3 deletions(-)