[libcamera-devel] [PATCH v2 1/4] ipa: ipu3: Extend ipu3 ipa interface for sensor and lens controls

[Hanlin Chen]

Hi Jean-Michel,
I'm sorry for the late reply.

On Thu, Nov 4, 2021 at 7:40 PM Jean-Michel Hautbois
<jeanmichel.hautbois at ideasonboard.com> wrote:
>
> Hi Han-Lin,
>
> Thanks for the patch.
>
> On 29/10/2021 13:59, Han-Lin Chen wrote:
> > IPU3Event and IPU3Action use single ControlList for both libcamera and V4L2
> > controls, and it's content could be either one based on the context.
> > Extend IPU3Event and IPU3Action for sensor and lens V4L2 controls, and preserve
> > the original one for only libcamera Controls to make the content of an event
> > more specific.
> >
> > Signed-off-by: Han-Lin Chen <hanlinchen at chromium.org>
> > ---
> >   include/libcamera/ipa/ipu3.mojom     | 4 ++++
> >   src/ipa/ipu3/ipu3.cpp                | 2 +-
> >   src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +-
> >   3 files changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/libcamera/ipa/ipu3.mojom b/include/libcamera/ipa/ipu3.mojom
> > index 2f254ed4..cc0d822f 100644
> > --- a/include/libcamera/ipa/ipu3.mojom
> > +++ b/include/libcamera/ipa/ipu3.mojom
> > @@ -23,11 +23,15 @@ struct IPU3Event {
> >       int64 frameTimestamp;
> >       uint32 bufferId;
> >       libcamera.ControlList controls;
> > +     libcamera.ControlList sensorControls;
> > +     libcamera.ControlList lensControls;
>
> I would split sensorControls adding and lensControls adding in two patch
> series, as the first one could be merged quickly, and could be useful
> also for the open IPA :-). One series would be "add sensorControls to
> use effective sensor values" and the second (biggest one) would then add
> (on top of it) the lens controls.
Thanks! The patches looks great to me.
>
> >   };
> >
> >   struct IPU3Action {
> >       IPU3Operations op;
> >       libcamera.ControlList controls;
> > +     libcamera.ControlList sensorControls;
> > +     libcamera.ControlList lensControls;
> >   };
> >
> >   struct IPAConfigInfo {
> > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> > index 5c51607d..6775570e 100644
> > --- a/src/ipa/ipu3/ipu3.cpp
> > +++ b/src/ipa/ipu3/ipu3.cpp
> > @@ -646,7 +646,7 @@ void IPAIPU3::setControls(unsigned int frame)
> >       ControlList ctrls(ctrls_);
> >       ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure_));
> >       ctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain_));
> > -     op.controls = ctrls;
> > +     op.sensorControls = ctrls;
> >
> >       queueFrameAction.emit(frame, op);
> >   }
> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > index eb714aa6..8816efc5 100644
> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > @@ -1248,7 +1248,7 @@ void IPU3CameraData::queueFrameAction(unsigned int id,
> >   {
> >       switch (action.op) {
> >       case ipa::ipu3::ActionSetSensorControls: {
> > -             const ControlList &controls = action.controls;
> > +             const ControlList &controls = action.sensorControls;
> >               delayedCtrls_->push(controls);
> >               break;
> >       }
> >