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

[Jacopo Mondi]

Hi Laurent,

On Mon, Oct 12, 2020 at 04:07:52AM +0300, Laurent Pinchart wrote:
> 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 ?
>

To really answer this question I should have a better understanding of
how this property is used by application.

Strictly speaking, yes, each stream has its own number of processing
steps applied, but maybe what really matters is the total applied to a
request ?

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