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

[Jacopo Mondi]

On Tue, Oct 13, 2020 at 10:57:08PM +0200, Jacopo Mondi wrote:
> 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 ?
>

I meant "maximum", not "total", sorry

> > > >
> > > > >  			pipe_->completeRequest(camera_, request);
> > > > >  			return;
> > > > >  		}
> >
> > --
> > Regards,
> >
> > Laurent Pinchart
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel