[libcamera-devel] [RFC 6/6] libcamera: ipu3: Report pipeline depth

[Kieran Bingham]

Hi Jacopo,

On 11/09/2020 17:20, Jacopo Mondi wrote:
> Report for each request the pipeline depth describing the number of
> processing steps the frames went into.
> 
> This change shows a limitation in the current implementation as the
> number of processing steps a frame has been run through is a property
> of each Stream. Currently we report the maximum depth among all the
> returned frames.

So do we need 'stream properties' ?


> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 9ce329a83f5d..26ea26430f8e 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -12,6 +12,7 @@
>  #include <vector>
>  
>  #include <libcamera/camera.h>
> +#include <libcamera/control_ids.h>
>  #include <libcamera/formats.h>
>  #include <libcamera/property_ids.h>
>  #include <libcamera/request.h>
> @@ -41,6 +42,10 @@ static constexpr unsigned int IMGU_OUTPUT_HEIGHT_ALIGN = 4;
>  static constexpr unsigned int IMGU_OUTPUT_WIDTH_MARGIN = 64;
>  static constexpr unsigned int IMGU_OUTPUT_HEIGHT_MARGIN = 32;
>  
> +static const ControlInfoMap IPU3Controls = {
> +	{ &controls::DraftPipelineDepth, ControlInfo(2, 3) }
> +};
> +

I presume we're going to need to expose controls that are read from the
V4L2 device too. Things like (given my recent discussion with David) the
DigitalGain perhaps, or other things which are controls of the ISP in
particular ?

This might need to be updated at load/runtime in that case, so I don't
think the static will be suitable - but that's a future problem.

I'm actually not worried about that here right now - though perhaps we
should add some sort of a \todo.


>  class IPU3CameraData : public CameraData
>  {
>  public:
> @@ -786,6 +791,8 @@ int PipelineHandlerIPU3::registerCameras()
>  		data->properties_.set(properties::DraftAvailableColorCorrectionAberrationModes,
>  				      { static_cast<int32_t>(properties::DRAFT_COLOR_CORRECTION_ABERRATION_OFF) });
>  
> +		data->controlInfo_ = IPU3Controls;
> +
>  		/**
>  		 * \todo Dynamically assign ImgU and output devices to each
>  		 * stream and camera; as of now, limit support to two cameras
> @@ -848,6 +855,7 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)
>  		return;
>  
>  	/* Mark the request as complete. */
> +	request->controls().set(controls::DraftPipelineDepth, 3);
>  	pipe_->completeRequest(camera_, request);
>  }
>  
> @@ -873,6 +881,7 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)
>  	if (request->findBuffer(&rawStream_)) {
>  		bool isComplete = pipe_->completeBuffer(camera_, request, buffer);
>  		if (isComplete) {
> +			request->controls().set(controls::DraftPipelineDepth, 2);
>  			pipe_->completeRequest(camera_, request);
>  			return;
>  		}
> 

Otherwise, this patch is actually simple enough that I think I can give
this already:

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

Although, I expect there to be some minimal changes in here as this
series develops anyway, but nothing that I foresee being troublesome

-- 
Regards
--
Kieran