Message ID | 20210514075808.282479-7-umang.jain@ideasonboard.com |
---|---|
State | Superseded |
Delegated to: | Umang Jain |
Headers | show |
Series |
|
Related | show |
Hi Me, On 14/05/2021 08:58, Umang Jain wrote: > From: Kieran Bingham <kieran.bingham@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. > > Signed-off-by: Kieran Bingham <kieran.bingham@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? Are the controls libcamera controls? or V4L2 controls? > ipa_->processEvent(ev); > } > >
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@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@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); > > }
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(); ipa_->processEvent(ev); }