[libcamera-devel] [PATCH 06/10] libcamera: ipu3: Report pipeline depth

[Jacopo Mondi]

Hi Kieran,

On Fri, Oct 09, 2020 at 01:36:57PM +0100, Kieran Bingham wrote:
> Hi Jacopo,
>
> On 09/10/2020 13:20, Jacopo Mondi wrote:
> > Report for each request the pipeline depth describing the number of
> > processing steps the frames went through.
> >
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> >  src/libcamera/pipeline/ipu3/ipu3.cpp | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > index f7ade2a6d5f3..413c2b8b1f07 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>
> > @@ -841,6 +842,7 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)
> >  		return;
> >
> >  	/* Mark the request as complete. */
> > +	request->metadata().set(controls::draft::PipelineDepth, 3);
> >  	pipe_->completeRequest(camera_, request);
> >  }
> >
> > @@ -866,6 +868,7 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)
> >  	if (request->findBuffer(&rawStream_)) {
> >  		bool isComplete = pipe_->completeBuffer(camera_, request, buffer);
> >  		if (isComplete) {
> > +			request->metadata().set(controls::draft::PipelineDepth, 2);
>
> Will it be obvious to readers what the '2' stages are? Maybe later we'll
> somehow automatically handle it.
>
> I think hardcoding these values currently deserves an explanation of why
> it's only 2 (or 3 above) perhaps.
>
> But - eitherway - maybe that's overkill too.

Please don't forget this is pipeline code, not application facing
code. I expect reading the proeprty documentation and noticing RAW=2,
YUV=3 is pretty clear to someone poking with platform code.

I can add a comment to previous patch that add the maximum pipeline
depth though.

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

Thanks
  j

> >  			pipe_->completeRequest(camera_, request);
> >  			return;
> >  		}
> >
>
> --
> Regards
> --
> Kieran