[libcamera-devel] [PATCH v3 05/16] ipa: ipu3: Update camera controls in configure()
Jacopo Mondi
jacopo at jmondi.org
Fri Oct 15 10:01:14 CEST 2021
Hi Jean-Michel
On Fri, Oct 15, 2021 at 09:26:10AM +0200, Jean-Michel Hautbois wrote:
> Hi Jacopo,
>
> On 11/10/2021 17:11, Jacopo Mondi wrote:
> > When a new CameraConfiguration is applied to the Camera the IPA is
> > configured as well, using the newly applied sensor configuration and its
> > updated V4L2 controls.
> >
> > Also update the Camera controls at IPA::configure() time by re-computing
> > the controls::ExposureTime and controls::FrameDurationLimits limits and
> > update the controls on the pipeline handler side after having configured
> > the IPA.
> >
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
> > Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> > include/libcamera/ipa/ipu3.mojom | 3 +-
> > src/ipa/ipu3/ipu3.cpp | 82 +++++++++++++++++++---------
> > src/libcamera/pipeline/ipu3/ipu3.cpp | 4 +-
> > 3 files changed, 60 insertions(+), 29 deletions(-)
> >
> > diff --git a/include/libcamera/ipa/ipu3.mojom b/include/libcamera/ipa/ipu3.mojom
> > index 2045ce909a88..2f254ed4193e 100644
> > --- a/include/libcamera/ipa/ipu3.mojom
> > +++ b/include/libcamera/ipa/ipu3.mojom
> > @@ -45,7 +45,8 @@ interface IPAIPU3Interface {
> > start() => (int32 ret);
> > stop();
> >
> > - configure(IPAConfigInfo configInfo) => (int32 ret);
> > + configure(IPAConfigInfo configInfo)
> > + => (int32 ret, libcamera.ControlInfoMap ipaControls);
> >
> > mapBuffers(array<libcamera.IPABuffer> buffers);
> > unmapBuffers(array<uint32> ids);
> > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> > index 6d9bbf391cb2..388f1902bcab 100644
> > --- a/src/ipa/ipu3/ipu3.cpp
> > +++ b/src/ipa/ipu3/ipu3.cpp
> > @@ -171,13 +171,17 @@ public:
> > int start() override;
> > void stop() override {}
> >
> > - int configure(const IPAConfigInfo &configInfo) override;
> > + int configure(const IPAConfigInfo &configInfo,
> > + ControlInfoMap *ipaControls) override;
> >
> > void mapBuffers(const std::vector<IPABuffer> &buffers) override;
> > void unmapBuffers(const std::vector<unsigned int> &ids) override;
> > void processEvent(const IPU3Event &event) override;
> >
> > private:
> > + void updateControls(const IPACameraSensorInfo &sensorInfo,
> > + const ControlInfoMap &sensorControls,
> > + ControlInfoMap *ipaControls);
> > void processControls(unsigned int frame, const ControlList &controls);
> > void fillParams(unsigned int frame, ipu3_uapi_params *params);
> > void parseStatistics(unsigned int frame,
> > @@ -212,36 +216,30 @@ private:
> > struct IPAContext context_;
> > };
> >
> > -/**
> > - * Initialize the IPA module and its controls.
> > +
> > +/*
> > + * Compute camera controls using the sensor information and the sensor
> > + * v4l2 controls.
> > *
> > - * This function receives the camera sensor information from the pipeline
> > - * handler, computes the limits of the controls it handles and returns
> > - * them in the \a ipaControls output parameter.
> > + * Some of the camera controls are computed by the pipeline handler, some others
> > + * by the IPA module which is in charge of handling, for example, the exposure
> > + * time and the frame duration.
> > + *
> > + * This function computes:
> > + * - controls::ExposureTime
> > + * - controls::FrameDurationLimits
> > */
> > -int IPAIPU3::init(const IPASettings &settings,
> > - const IPACameraSensorInfo &sensorInfo,
> > - const ControlInfoMap &sensorControls,
> > - ControlInfoMap *ipaControls)
> > +void IPAIPU3::updateControls(const IPACameraSensorInfo &sensorInfo,
> > + const ControlInfoMap &sensorControls,
> > + ControlInfoMap *ipaControls)
> > {
> > - camHelper_ = CameraSensorHelperFactory::create(settings.sensorModel);
> > - if (camHelper_ == nullptr) {
> > - LOG(IPAIPU3, Error)
> > - << "Failed to create camera sensor helper for "
> > - << settings.sensorModel;
> > - return -ENODEV;
> > - }
> > -
> > - /* Initialize Controls. */
> > ControlInfoMap::Map controls{};
> >
> > /*
> > - * Compute exposure time limits.
> > - *
> > - * Initialize the control using the line length and pixel rate of the
> > - * current configuration converted to microseconds. Use the
> > - * V4L2_CID_EXPOSURE control to get exposure min, max and default and
> > - * convert it from lines to microseconds.
> > + * Compute exposure time limits by using line length and pixel rate
> > + * converted to microseconds. Use the V4L2_CID_EXPOSURE control to get
> > + * exposure min, max and default and convert it from lines to
> > + * microseconds.
> > */
> > double lineDuration = sensorInfo.lineLength / (sensorInfo.pixelRate / 1e6);
> > const ControlInfo &v4l2Exposure = sensorControls.find(V4L2_CID_EXPOSURE)->second;
> > @@ -279,6 +277,27 @@ int IPAIPU3::init(const IPASettings &settings,
> > frameDurations[2]);
> >
> > *ipaControls = ControlInfoMap(std::move(controls), controls::controls);
> > +}
> > +
> > +/**
> > + * Initialize the IPA module and its controls.
> > + *
> > + * This function receives the camera sensor information from the pipeline
> > + * handler, computes the limits of the controls it handles and returns
> > + * them in the \a ipaControls output parameter.
> > + */
> > +int IPAIPU3::init(const IPASettings &settings,
> > + const IPACameraSensorInfo &sensorInfo,
> > + const ControlInfoMap &sensorControls,
> > + ControlInfoMap *ipaControls)
> > +{
> > + camHelper_ = CameraSensorHelperFactory::create(settings.sensorModel);
> > + if (camHelper_ == nullptr) {
> > + LOG(IPAIPU3, Error)
> > + << "Failed to create camera sensor helper for "
> > + << settings.sensorModel;
> > + return -ENODEV;
> > + }
> >
> > /* Construct our Algorithms */
> > algorithms_.push_back(std::make_unique<algorithms::Agc>());
> > @@ -286,6 +305,9 @@ int IPAIPU3::init(const IPASettings &settings,
> > algorithms_.push_back(std::make_unique<algorithms::BlackLevelCorrection>());
> > algorithms_.push_back(std::make_unique<algorithms::ToneMapping>());
> >
> > + /* Initialize controls. */
> > + updateControls(sensorInfo, sensorControls, ipaControls);
> > +
>
> Thanks, I had a patch almost doing the same, I did not notice you
> already did that... :-).
>
> > return 0;
> > }
> >
> > @@ -365,7 +387,8 @@ void IPAIPU3::calculateBdsGrid(const Size &bdsOutputSize)
> > << (int)bdsGrid.height << " << " << (int)bdsGrid.block_height_log2 << ")";
> > }
> >
> > -int IPAIPU3::configure(const IPAConfigInfo &configInfo)
> > +int IPAIPU3::configure(const IPAConfigInfo &configInfo,
> > + ControlInfoMap *ipaControls)
> > {
> > if (configInfo.sensorControls.empty()) {
> > LOG(IPAIPU3, Error) << "No sensor controls provided";
> > @@ -374,6 +397,10 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo)
> >
> > sensorInfo_ = configInfo.sensorInfo;
> >
> > + /*
> > + * Compute the sensor V4L2 controls to be used by the algorithms and
> > + * to be set on the sensor.
> > + */
> > ctrls_ = configInfo.sensorControls;
> >
> > const auto itExp = ctrls_.find(V4L2_CID_EXPOSURE);
> > @@ -415,6 +442,9 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo)
> > return ret;
> > }
> >
> > + /* Update the camera controls using the new sensor settings. */
> > + updateControls(sensorInfo_, ctrls_, ipaControls);
> > +
>
> I would have put that call before the algorithm->configure() calls, as
> we may (wait for it) need to pass the updated values to some of them
> (AGC?) through the IPAContext.
I see, updateControls() uses the sensor controls part of configInfo
to compute the Camera's controls::controls, and it seems to me that
algo->configure() use the same sensor controls, and does not require
the newly compute controls::controls. Will this change soon ? I have
no problem moving it, but I would do so when required.
Thanks
j
>
> Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
>
> > return 0;
> > }
> >
> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > index 1033153360b8..e0b61deb9b47 100644
> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > @@ -659,14 +659,14 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
> > configInfo.bdsOutputSize = config->imguConfig().bds;
> > configInfo.iif = config->imguConfig().iif;
> >
> > - ret = data->ipa_->configure(configInfo);
> > + ret = data->ipa_->configure(configInfo, &data->ipaControls_);
> > if (ret) {
> > LOG(IPU3, Error) << "Failed to configure IPA: "
> > << strerror(-ret);
> > return ret;
> > }
> >
> > - return 0;
> > + return updateControls(data);
> > }
> >
> > int PipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream,
> >
More information about the libcamera-devel
mailing list