| Message ID | 20210419131433.22920-5-jacopo@jmondi.org |
|---|---|
| State | Accepted |
| Delegated to: | Jacopo Mondi |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, On 19/04/2021 14:14, Jacopo Mondi wrote: > This commit applies to the RkISP1 pipeline handler the same change > applied to IPU3 in commit 13a7ed7b1f1f ("libcamera: ipu3: Do not > over-write metadata"). > > When a Request is completed upon receiving the IPA produced metadata, > the metadata associated with the Request are over-written, deleting > the information set at output buffer completion, such as the > SensorTimestamp. > > Fixes: 0eb65e14e18d ("libcamera: pipeline: rkisp1: Attach to an IPA") > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index 549f4a4e61a8..925c8c6372e0 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -362,7 +362,9 @@ void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &meta > if (!info) > return; > > - info->request->metadata() = metadata; > + for (const auto &ctrl : metadata) > + info->request->metadata().set(ctrl.first, ctrl.second); This screams to me 'ControlList->merge(&other)' ? But even without extending the API of ControlList, this fixes an issue so: Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > + > info->metadataProcessed = true; > > pipe->tryCompleteRequest(info->request); >
Hi Kieran, On Mon, Apr 19, 2021 at 02:54:38PM +0100, Kieran Bingham wrote: > Hi Jacopo, > > On 19/04/2021 14:14, Jacopo Mondi wrote: > > This commit applies to the RkISP1 pipeline handler the same change > > applied to IPU3 in commit 13a7ed7b1f1f ("libcamera: ipu3: Do not > > over-write metadata"). > > > > When a Request is completed upon receiving the IPA produced metadata, > > the metadata associated with the Request are over-written, deleting > > the information set at output buffer completion, such as the > > SensorTimestamp. > > > > Fixes: 0eb65e14e18d ("libcamera: pipeline: rkisp1: Attach to an IPA") > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > --- > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > index 549f4a4e61a8..925c8c6372e0 100644 > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > @@ -362,7 +362,9 @@ void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &meta > > if (!info) > > return; > > > > - info->request->metadata() = metadata; > > + for (const auto &ctrl : metadata) > > + info->request->metadata().set(ctrl.first, ctrl.second); > > This screams to me 'ControlList->merge(&other)' ? I considered operator+= too But I wonder if having this sequence explicit in ph that needs to do so isn't better... > > But even without extending the API of ControlList, this fixes an issue so: > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > + > > info->metadataProcessed = true; > > > > pipe->tryCompleteRequest(info->request); > > > > -- > Regards > -- > Kieran
On Mon, Apr 19, 2021 at 02:54:38PM +0100, Kieran Bingham wrote: > Hi Jacopo, > > On 19/04/2021 14:14, Jacopo Mondi wrote: > > This commit applies to the RkISP1 pipeline handler the same change > > applied to IPU3 in commit 13a7ed7b1f1f ("libcamera: ipu3: Do not > > over-write metadata"). > > > > When a Request is completed upon receiving the IPA produced metadata, > > the metadata associated with the Request are over-written, deleting > > the information set at output buffer completion, such as the > > SensorTimestamp. > > > > Fixes: 0eb65e14e18d ("libcamera: pipeline: rkisp1: Attach to an IPA") > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > --- > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > index 549f4a4e61a8..925c8c6372e0 100644 > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > @@ -362,7 +362,9 @@ void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &meta > > if (!info) > > return; > > > > - info->request->metadata() = metadata; > > + for (const auto &ctrl : metadata) > > + info->request->metadata().set(ctrl.first, ctrl.second); > > This screams to me 'ControlList->merge(&other)' ? https://patchwork.libcamera.org/patch/11364/ This may be a good occasion to test that patch ? > But even without extending the API of ControlList, this fixes an issue so: > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > + > > info->metadataProcessed = true; > > > > pipe->tryCompleteRequest(info->request);
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 549f4a4e61a8..925c8c6372e0 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -362,7 +362,9 @@ void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &meta if (!info) return; - info->request->metadata() = metadata; + for (const auto &ctrl : metadata) + info->request->metadata().set(ctrl.first, ctrl.second); + info->metadataProcessed = true; pipe->tryCompleteRequest(info->request);
This commit applies to the RkISP1 pipeline handler the same change applied to IPU3 in commit 13a7ed7b1f1f ("libcamera: ipu3: Do not over-write metadata"). When a Request is completed upon receiving the IPA produced metadata, the metadata associated with the Request are over-written, deleting the information set at output buffer completion, such as the SensorTimestamp. Fixes: 0eb65e14e18d ("libcamera: pipeline: rkisp1: Attach to an IPA") Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)