| Message ID | 20210907194107.803730-5-jacopo@jmondi.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, Thank you for the patch. On Tue, Sep 07, 2021 at 09:40:54PM +0200, 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@jmondi.org> > Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > Reviewed-by: Umang Jain <umang.jain@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 d561c2244907..714f58ab8a42 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 c925cf642611..ec75b3ffa305 100644 > --- a/src/ipa/ipu3/ipu3.cpp > +++ b/src/ipa/ipu3/ipu3.cpp > @@ -156,13 +156,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, > @@ -197,36 +201,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, in example, the exposure s/in example/for example/ > + * time and the frame duration. > + * > + * This functions computes: s/functions/function/ Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + * - 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; > @@ -264,12 +262,36 @@ 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>()); > algorithms_.push_back(std::make_unique<algorithms::Awb>()); > algorithms_.push_back(std::make_unique<algorithms::ToneMapping>()); > > + /* Initialize controls. */ > + updateControls(sensorInfo, sensorControls, ipaControls); > + > return 0; > } > > @@ -335,7 +357,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.entityControls.empty()) { > LOG(IPAIPU3, Error) << "No controls provided"; > @@ -344,6 +367,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.entityControls.at(0); > > const auto itExp = ctrls_.find(V4L2_CID_EXPOSURE); > @@ -385,6 +412,9 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo) > return ret; > } > > + /* Update the camera controls using the new sensor settings. */ > + updateControls(sensorInfo_, ctrls_, ipaControls); > + > return 0; > } > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index 1c383336e45f..de0adffc965f 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -660,14 +660,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,
diff --git a/include/libcamera/ipa/ipu3.mojom b/include/libcamera/ipa/ipu3.mojom index d561c2244907..714f58ab8a42 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 c925cf642611..ec75b3ffa305 100644 --- a/src/ipa/ipu3/ipu3.cpp +++ b/src/ipa/ipu3/ipu3.cpp @@ -156,13 +156,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, @@ -197,36 +201,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, in example, the exposure + * time and the frame duration. + * + * This functions 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; @@ -264,12 +262,36 @@ 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>()); algorithms_.push_back(std::make_unique<algorithms::Awb>()); algorithms_.push_back(std::make_unique<algorithms::ToneMapping>()); + /* Initialize controls. */ + updateControls(sensorInfo, sensorControls, ipaControls); + return 0; } @@ -335,7 +357,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.entityControls.empty()) { LOG(IPAIPU3, Error) << "No controls provided"; @@ -344,6 +367,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.entityControls.at(0); const auto itExp = ctrls_.find(V4L2_CID_EXPOSURE); @@ -385,6 +412,9 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo) return ret; } + /* Update the camera controls using the new sensor settings. */ + updateControls(sensorInfo_, ctrls_, ipaControls); + return 0; } diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 1c383336e45f..de0adffc965f 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -660,14 +660,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,