[libcamera-devel] [PATCH 04/13] libcamera: rkisp1: Do not over-write metadata

[Jacopo Mondi]

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 at 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 at ideasonboard.com>
>
> > +
> >  	info->metadataProcessed = true;
> >
> >  	pipe->tryCompleteRequest(info->request);
> >
>
> --
> Regards
> --
> Kieran