| Message ID | 20210430160026.190724-8-jacopo@jmondi.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, On 30/04/2021 17:00, Jacopo Mondi wrote: > 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. > > This commit applies to the RkISP1 pipeline handler the same change > applied to IPU3 in commit 13a7ed7b1f1f ("libcamera: ipu3: Do not > over-write metadata") but compared to that commit it uses the newly > introduced ControlList::merge() function. Looks much better to me ;-) I'm sure the documentation updates in 01/16 will get sorted, and this is more self-documenting so: Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index c75666391222..26f708242523 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -365,7 +365,7 @@ void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &meta > if (!info) > return; > > - info->request->metadata() = metadata; > + info->request->metadata().merge(metadata); > info->metadataProcessed = true; > > pipe->tryCompleteRequest(info->request); >
Hi Jacopo, On 30/04/2021 17:00, Jacopo Mondi wrote: > 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. > > This commit applies to the RkISP1 pipeline handler the same change > applied to IPU3 in commit 13a7ed7b1f1f ("libcamera: ipu3: Do not > over-write metadata") but compared to that commit it uses the newly > introduced ControlList::merge() function. It sounds like 13a7ed7b1f1f ("libcamera: ipu3: Do not over-write metadata") has added a todo which was waiting for a .merge() to become available to solve this in the same way. With this series adding the .merge() Can/Should we update the IPU3 in the same way either as an extra patch on this series or make sure we do it on top? -- Kieran > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index c75666391222..26f708242523 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -365,7 +365,7 @@ void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &meta > if (!info) > return; > > - info->request->metadata() = metadata; > + info->request->metadata().merge(metadata); > info->metadataProcessed = true; > > pipe->tryCompleteRequest(info->request); >
Hi Jacopo, Thanks for fixing this long overdue todo :-) On 2021-04-30 18:00:17 +0200, Jacopo Mondi wrote: > 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. > > This commit applies to the RkISP1 pipeline handler the same change > applied to IPU3 in commit 13a7ed7b1f1f ("libcamera: ipu3: Do not > over-write metadata") but compared to that commit it uses the newly > introduced ControlList::merge() function. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index c75666391222..26f708242523 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -365,7 +365,7 @@ void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &meta > if (!info) > return; > > - info->request->metadata() = metadata; > + info->request->metadata().merge(metadata); > info->metadataProcessed = true; > > pipe->tryCompleteRequest(info->request); > -- > 2.31.1 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Kieran, On Fri, Apr 30, 2021 at 08:10:40PM +0100, Kieran Bingham wrote: > Hi Jacopo, > > On 30/04/2021 17:00, Jacopo Mondi wrote: > > 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. > > > > This commit applies to the RkISP1 pipeline handler the same change > > applied to IPU3 in commit 13a7ed7b1f1f ("libcamera: ipu3: Do not > > over-write metadata") but compared to that commit it uses the newly > > introduced ControlList::merge() function. > > It sounds like 13a7ed7b1f1f ("libcamera: ipu3: Do not over-write > metadata") has added a todo which was waiting for a .merge() to become > available to solve this in the same way. > > With this series adding the .merge() Can/Should we update the IPU3 in > the same way either as an extra patch on this series or make sure we do > it on top? Am I wrong or the IPA does not produce metadata at the moment ? > > -- > Kieran > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > --- > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > index c75666391222..26f708242523 100644 > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > @@ -365,7 +365,7 @@ void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &meta > > if (!info) > > return; > > > > - info->request->metadata() = metadata; > > + info->request->metadata().merge(metadata); > > info->metadataProcessed = true; > > > > pipe->tryCompleteRequest(info->request); > > > > -- > Regards > -- > Kieran
Hi Jacopo, On 01/05/2021 08:56, Jacopo Mondi wrote: > Hi Kieran, > > On Fri, Apr 30, 2021 at 08:10:40PM +0100, Kieran Bingham wrote: >> Hi Jacopo, >> >> On 30/04/2021 17:00, Jacopo Mondi wrote: >>> 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. >>> >>> This commit applies to the RkISP1 pipeline handler the same change >>> applied to IPU3 in commit 13a7ed7b1f1f ("libcamera: ipu3: Do not >>> over-write metadata") but compared to that commit it uses the newly >>> introduced ControlList::merge() function. >> >> It sounds like 13a7ed7b1f1f ("libcamera: ipu3: Do not over-write >> metadata") has added a todo which was waiting for a .merge() to become >> available to solve this in the same way. >> >> With this series adding the .merge() Can/Should we update the IPU3 in >> the same way either as an extra patch on this series or make sure we do >> it on top? > > Am I wrong or the IPA does not produce metadata at the moment ? (I haven't even checked...) Is that relevant? If or when it does - it will be affected in the same way as the other IPAs. Seems better to make things all consistent in the same way at the same time - otherwise, when we add metadata - we'll hit the 'bug' and have to re-investigate it... -- Kieran > >> >> -- >> Kieran >> >>> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> >>> --- >>> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp >>> index c75666391222..26f708242523 100644 >>> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp >>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp >>> @@ -365,7 +365,7 @@ void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &meta >>> if (!info) >>> return; >>> >>> - info->request->metadata() = metadata; >>> + info->request->metadata().merge(metadata); >>> info->metadataProcessed = true; >>> >>> pipe->tryCompleteRequest(info->request); >>> >> >> -- >> Regards >> -- >> Kieran
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index c75666391222..26f708242523 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -365,7 +365,7 @@ void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &meta if (!info) return; - info->request->metadata() = metadata; + info->request->metadata().merge(metadata); info->metadataProcessed = true; pipe->tryCompleteRequest(info->request);
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. This commit applies to the RkISP1 pipeline handler the same change applied to IPU3 in commit 13a7ed7b1f1f ("libcamera: ipu3: Do not over-write metadata") but compared to that commit it uses the newly introduced ControlList::merge() function. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)