[libcamera-devel] [PATCH 4/8] libcamera: pipeline: ipu3: frames: Use the request sequence

[Laurent Pinchart]

Hi Kieran and Niklas,

On Sat, Mar 13, 2021 at 12:31:25AM +0100, Niklas Söderlund wrote:
> On 2021-03-12 06:11:27 +0000, Kieran Bingham wrote:
> > For all frame indexes, use the same sequence number as generated
> > by the Request object.
> > 
> > This allows clear matching of what operations occured to which request.
> 
> nit-pick: This will not be true if we need to queue buffers internally 
> (without a request) to feed the 3A right? I think this change may be a 
> step in the right direction just wanted to bring up why an internal id 
> was used.

Wouldn't we still have a request, even if it has only a raw buffer ?

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

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

> > ---
> >  src/libcamera/pipeline/ipu3/frames.cpp | 4 +---
> >  src/libcamera/pipeline/ipu3/frames.h   | 1 -
> >  2 files changed, 1 insertion(+), 4 deletions(-)
> > 
> > diff --git a/src/libcamera/pipeline/ipu3/frames.cpp b/src/libcamera/pipeline/ipu3/frames.cpp
> > index 151ebfe7ca5f..4198e2019f3f 100644
> > --- a/src/libcamera/pipeline/ipu3/frames.cpp
> > +++ b/src/libcamera/pipeline/ipu3/frames.cpp
> > @@ -18,7 +18,6 @@ namespace libcamera {
> >  LOG_DECLARE_CATEGORY(IPU3)
> >  
> >  IPU3Frames::IPU3Frames()
> > -	: nextId_(0)
> >  {
> >  }
> >  
> > @@ -31,7 +30,6 @@ void IPU3Frames::init(const std::vector<std::unique_ptr<FrameBuffer>> &paramBuff
> >  	for (const std::unique_ptr<FrameBuffer> &buffer : statBuffers)
> >  		availableStatBuffers_.push(buffer.get());
> >  
> > -	nextId_ = 0;
> >  	frameInfo_.clear();
> >  }
> >  
> > @@ -43,7 +41,7 @@ void IPU3Frames::clear()
> >  
> >  IPU3Frames::Info *IPU3Frames::create(Request *request)
> >  {
> > -	unsigned int id = nextId_++;
> > +	unsigned int id = request->sequence();
> >  
> >  	if (availableParamBuffers_.empty()) {
> >  		LOG(IPU3, Error) << "Parameters buffer underrun";
> > diff --git a/src/libcamera/pipeline/ipu3/frames.h b/src/libcamera/pipeline/ipu3/frames.h
> > index 106e5c15cc7a..4acdf48eca9d 100644
> > --- a/src/libcamera/pipeline/ipu3/frames.h
> > +++ b/src/libcamera/pipeline/ipu3/frames.h
> > @@ -53,7 +53,6 @@ private:
> >  	std::queue<FrameBuffer *> availableParamBuffers_;
> >  	std::queue<FrameBuffer *> availableStatBuffers_;
> >  
> > -	unsigned int nextId_;
> >  	std::map<unsigned int, std::unique_ptr<Info>> frameInfo_;
> >  };
> >  

-- 
Regards,

Laurent Pinchart