[libcamera-devel] [PATCH v1 6/6] libcamera: pipeline: ipu3: Pass request metadata to IPA

[Laurent Pinchart]

Hi Kieran,

On Fri, May 14, 2021 at 11:38:45AM +0100, Kieran Bingham wrote:
> On 14/05/2021 08:58, Umang Jain wrote:
> > From: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > 
> > Pass the request metadata control list to the IPA when setting
> > passing the statistics buffers, or after the raw buffer completion.
> > 
> > This allows us to pass in the timestamp and other data from the request
> > which will be used by the IPA.

Metadata are reported by the camera to applications through the Request
class. While the pipeline handler may store information in
Request::metadata() that would be of interest to the IPA, I'd rather
pass those pieces of information explicitly than passing
Request::metadata(), as conceptually speaking, Request::metadata() is
filled by the IPA, not passed to the IPA.

> > Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > ---
> >  src/libcamera/pipeline/ipu3/ipu3.cpp | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > index 5b15ca90..38f66919 100644
> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > @@ -1285,6 +1285,7 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)
> >  	ev.op = ipa::ipu3::EventFillParams;
> >  	ev.frame = info->id;
> >  	ev.bufferId = info->paramBuffer->cookie();
> > +	ev.controls = request->metadata();
> >  	ipa_->processEvent(ev);
> >  }
> >  
> > @@ -1314,6 +1315,8 @@ void IPU3CameraData::statBufferReady(FrameBuffer *buffer)
> >  	if (!info)
> >  		return;
> >  
> > +	Request *request = info->request;
> > +
> >  	if (buffer->metadata().status == FrameMetadata::FrameCancelled) {
> >  		info->metadataProcessed = true;
> >  
> > @@ -1321,7 +1324,6 @@ void IPU3CameraData::statBufferReady(FrameBuffer *buffer)
> >  		* tryComplete() will delete info if it completes the IPU3Frame.
> >  		* In that event, we must have obtained the Request before hand.
> >  		*/
> > -		Request *request = info->request;
> >  
> >  		if (frameInfos_.tryComplete(info))
> >  			pipe_->completeRequest(request);
> > @@ -1333,6 +1335,7 @@ void IPU3CameraData::statBufferReady(FrameBuffer *buffer)
> >  	ev.op = ipa::ipu3::EventStatReady;
> >  	ev.frame = info->id;
> >  	ev.bufferId = info->statBuffer->cookie();
> > +	ev.controls = request->metadata();
> 
> I only wonder if we'll ever need to send both controls and metadata as
> separate lists. And if so, I suspect we should have a distinct separate
> metadata or properties entry in the IPA/IPC events.
> 
> But I quite dislike that we have 'controls' 'metadata' and 'properties'
> which are all quite ambiguous as to which is which.
> 
> Are properties always metadata? Is metadata always properties? or
> sometimes the value of a control as it was set?

If you're talking about the Camera properties and the Request controls
and metadata, I think it's fairly well-defined (but if it's not
well-documented, patches are welcome :-)). Properties are "semi-static"
information about a camera (they used to be static, but we've allowed
them to change in response to Camera::configure(), which I don't like
much and would like to revisit at some point). Controls are passed by
the application to the camera, and metadata are reported by the camera
to the application. property_ids.yaml lists the possibly properties,
while control_ids.yaml lists the possible controls and metadata. I
don't like the split that much, and I've considered merging the files
together, with an additional YAML property for each entry to tell
whether a given entry can be used as a property, a control and/or
metadata.

> Are the controls libcamera controls? or V4L2 controls?

Which controls ?

> >  	ipa_->processEvent(ev);
> >  }

-- 
Regards,

Laurent Pinchart