[libcamera-devel] [RFC PATCH v2 11/12] pipeline: ipu3: Set request metadata for FULL compliance

[Laurent Pinchart]

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 at 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) {

-- 
Regards,

Laurent Pinchart