[v1,32/33] libcamera: rkisp1: Implement LensDewarpEnable control

24531 diff mbox series

Implement the LensDewarpEnable control for the rkisp1 pipeline using the
dw100 converter.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
---
 src/libcamera/pipeline/rkisp1/rkisp1.cpp | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Quoting Stefan Klug (2025-09-30 13:26:53)
> Implement the LensDewarpEnable control for the rkisp1 pipeline using the
> dw100 converter.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 13d82c9957bc..98521c804d84 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -1514,12 +1514,13 @@ int PipelineHandlerRkISP1::updateControls(RkISP1CameraData *data)
>                                                                       maxCrop);
>                 }
>  
> -

Lets squash that down to where it was added ?

>                 if (dewarper_->supportsRequests()) {
>                         controls[&controls::draft::Dw100Scale] = ControlInfo(0.2f, 8.0f, 1.0f);
>                         controls[&controls::draft::Dw100Rotation] = ControlInfo(-180.0f, 180.0f, 0.0f);
>                         controls[&controls::draft::Dw100Offset] = ControlInfo(Point(-10000, -10000), Point(10000, 10000), Point(0, 0));
>                         controls[&controls::draft::Dw100ScaleMode] = ControlInfo(controls::draft::Dw100ScaleModeValues, controls::draft::Fill);
> +                       if (data->dewarpParams_.has_value())
> +                               controls[&controls::LensDewarpEnable] = ControlInfo(false, true, true);

So - if we have a dewarp parameter set - we expose a control to allow it
to be enabled or disabled...

We default to true. Great :-)

>                 } else {
>                         LOG(RkISP1, Warning)
>                                 << "dw100 kernel driver has no requests support."
> @@ -1873,6 +1874,9 @@ void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer)
>         meta.set(controls::draft::Dw100Rotation, vertexMap.rotation());
>         meta.set(controls::draft::Dw100Offset, vertexMap.effectiveOffset());
>         meta.set(controls::ScalerCrop, vertexMap.effectiveScalerCrop());
> +
> +       if (vertexMap.dewarpParamsValid())
> +               meta.set(controls::LensDewarpEnable, vertexMap.lensDewarpEnable());

And we report if it was enabled...

But does anything parse LensDewarpEnable to allow it to be disabled ?

>  }
>  
>  void PipelineHandlerRkISP1::dewarpRequestReady(V4L2Request *request)
> -- 
> 2.48.1
>

Hi Kieran,

Thank you for the review.

Quoting Kieran Bingham (2025-10-02 23:22:18)
> Quoting Stefan Klug (2025-09-30 13:26:53)
> > Implement the LensDewarpEnable control for the rkisp1 pipeline using the
> > dw100 converter.
> > 
> > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> > ---
> >  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > index 13d82c9957bc..98521c804d84 100644
> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > @@ -1514,12 +1514,13 @@ int PipelineHandlerRkISP1::updateControls(RkISP1CameraData *data)
> >                                                                       maxCrop);
> >                 }
> >  
> > -
> 
> Lets squash that down to where it was added ?

Sure, no problem.

> 
> >                 if (dewarper_->supportsRequests()) {
> >                         controls[&controls::draft::Dw100Scale] = ControlInfo(0.2f, 8.0f, 1.0f);
> >                         controls[&controls::draft::Dw100Rotation] = ControlInfo(-180.0f, 180.0f, 0.0f);
> >                         controls[&controls::draft::Dw100Offset] = ControlInfo(Point(-10000, -10000), Point(10000, 10000), Point(0, 0));
> >                         controls[&controls::draft::Dw100ScaleMode] = ControlInfo(controls::draft::Dw100ScaleModeValues, controls::draft::Fill);
> > +                       if (data->dewarpParams_.has_value())
> > +                               controls[&controls::LensDewarpEnable] = ControlInfo(false, true, true);
> 
> So - if we have a dewarp parameter set - we expose a control to allow it
> to be enabled or disabled...
> 
> We default to true. Great :-)
> 
> >                 } else {
> >                         LOG(RkISP1, Warning)
> >                                 << "dw100 kernel driver has no requests support."
> > @@ -1873,6 +1874,9 @@ void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer)
> >         meta.set(controls::draft::Dw100Rotation, vertexMap.rotation());
> >         meta.set(controls::draft::Dw100Offset, vertexMap.effectiveOffset());
> >         meta.set(controls::ScalerCrop, vertexMap.effectiveScalerCrop());
> > +
> > +       if (vertexMap.dewarpParamsValid())
> > +               meta.set(controls::LensDewarpEnable, vertexMap.lensDewarpEnable());
> 
> And we report if it was enabled...
> 
> But does anything parse LensDewarpEnable to allow it to be disabled ?

Ouch where did that get lost... it was still there a while ago. Thanks
for spotting this.

Best regards,
Stefan

> 
> >  }
> >  
> >  void PipelineHandlerRkISP1::dewarpRequestReady(V4L2Request *request)
> > -- 
> > 2.48.1
> >