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

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Fri, Oct 09, 2020 at 02:51:12PM +0200, Jacopo Mondi wrote:
> On Fri, Oct 09, 2020 at 01:36:57PM +0100, Kieran Bingham wrote:
> > 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>

Acked-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Starting the discussion to prepare for a non-draft implementation of
this, down the line, would you envision this being a stream-level
metadata ?

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

-- 
Regards,

Laurent Pinchart