Message ID | 20210317100211.1067585-8-naush@raspberrypi.com |
---|---|
State | Changes Requested |
Headers | show |
Series |
|
Related | show |
Hi Naush, Thank you for the patch. On Wed, Mar 17, 2021 at 10:02:11AM +0000, Naushir Patuck wrote: > Rename ConfigInput to IPAConfig to be more consistent with the naming, > and remove ConfigInput::op, as it is never used. > > Replace ConfigOutput with a ControlList type, as that is the only return > type from ipa::configure(). > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > --- > include/libcamera/ipa/raspberrypi.mojom | 16 +++++----------- > src/ipa/raspberrypi/raspberrypi.cpp | 13 ++++++------- > .../pipeline/raspberrypi/raspberrypi.cpp | 13 +++++-------- > 3 files changed, 16 insertions(+), 26 deletions(-) > > diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom > index 7549397a27c4..f38c22611cc4 100644 > --- a/include/libcamera/ipa/raspberrypi.mojom > +++ b/include/libcamera/ipa/raspberrypi.mojom > @@ -29,17 +29,11 @@ struct ISPConfig { > ControlList controls; > }; > > -struct ConfigInput { > - uint32 op; > +struct IPAConfig { > uint32 transform; > FileDescriptor lsTableHandle; > }; > > -struct ConfigOutput { > - uint32 params; Removal of params could be squashed with 2/7. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > - ControlList controls; > -}; > - > struct StartConfig { > ControlList controls; > int32 dropFrameCount; > @@ -57,7 +51,7 @@ interface IPARPiInterface { > * \param[in] streamConfig Configuration of all active streams > * \param[in] entityControls Controls provided by the pipeline entities > * \param[in] ipaConfig Pipeline-handler-specific configuration data > - * \param[out] results Pipeline-handler-specific configuration result > + * \param[out] controls Controls to apply by the pipeline entity > * > * This method shall be called when the camera is configured to inform > * the IPA of the camera's streams and the sensor settings. > @@ -65,14 +59,14 @@ interface IPARPiInterface { > * The \a sensorInfo conveys information about the camera sensor settings that > * the pipeline handler has selected for the configuration. > * > - * The \a ipaConfig and \a results parameters carry data passed by the > + * The \a ipaConfig and \a controls parameters carry data passed by the > * pipeline handler to the IPA and back. > */ > configure(CameraSensorInfo sensorInfo, > map<uint32, IPAStream> streamConfig, > map<uint32, ControlInfoMap> entityControls, > - ConfigInput ipaConfig) > - => (int32 ret, ConfigOutput results); > + IPAConfig ipaConfig) > + => (int32 ret, ControlList controls); > > /** > * \fn mapBuffers() > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp > index 417a7922c3bd..1c928b72fbd0 100644 > --- a/src/ipa/raspberrypi/raspberrypi.cpp > +++ b/src/ipa/raspberrypi/raspberrypi.cpp > @@ -85,8 +85,8 @@ public: > int configure(const CameraSensorInfo &sensorInfo, > const std::map<unsigned int, IPAStream> &streamConfig, > const std::map<unsigned int, ControlInfoMap> &entityControls, > - const ipa::RPi::ConfigInput &data, > - ipa::RPi::ConfigOutput *response) override; > + const ipa::RPi::IPAConfig &data, > + ControlList *controls) override; > void mapBuffers(const std::vector<IPABuffer> &buffers) override; > void unmapBuffers(const std::vector<unsigned int> &ids) override; > void signalStatReady(const uint32_t bufferId) override; > @@ -313,16 +313,14 @@ void IPARPi::setMode(const CameraSensorInfo &sensorInfo) > int IPARPi::configure(const CameraSensorInfo &sensorInfo, > [[maybe_unused]] const std::map<unsigned int, IPAStream> &streamConfig, > const std::map<unsigned int, ControlInfoMap> &entityControls, > - const ipa::RPi::ConfigInput &ipaConfig, > - ipa::RPi::ConfigOutput *result) > + const ipa::RPi::IPAConfig &ipaConfig, > + ControlList *controls) > { > if (entityControls.size() != 2) { > LOG(IPARPI, Error) << "No ISP or sensor controls found."; > return -1; > } > > - result->params = 0; > - > sensorCtrls_ = entityControls.at(0); > ispCtrls_ = entityControls.at(1); > > @@ -379,7 +377,8 @@ int IPARPi::configure(const CameraSensorInfo &sensorInfo, > agcStatus.analogue_gain = DefaultAnalogueGain; > applyAGC(&agcStatus, ctrls); > > - result->controls = std::move(ctrls); > + ASSERT(controls); > + *controls = std::move(ctrls); > } > > return 0; > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > index 16568d56f31c..1f8307a3c1c5 100644 > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > @@ -1240,7 +1240,7 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config) > > std::map<unsigned int, IPAStream> streamConfig; > std::map<unsigned int, ControlInfoMap> entityControls; > - ipa::RPi::ConfigInput ipaConfig; > + ipa::RPi::IPAConfig ipaConfig; > > /* Get the device format to pass to the IPA. */ > V4L2DeviceFormat sensorFormat; > @@ -1283,19 +1283,16 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config) > } > > /* Ready the IPA - it must know about the sensor resolution. */ > - ipa::RPi::ConfigOutput result; > - > + ControlList controls; > ret = ipa_->configure(sensorInfo_, streamConfig, entityControls, ipaConfig, > - &result); > + &controls); > if (ret < 0) { > LOG(RPI, Error) << "IPA configuration failed!"; > return -EPIPE; > } > > - if (!result.controls.empty()) { > - ControlList &ctrls = result.controls; > - unicam_[Unicam::Image].dev()->setControls(&ctrls); > - } > + if (!controls.empty()) > + unicam_[Unicam::Image].dev()->setControls(&controls); > > /* > * Configure the H/V flip controls based on the combination of
diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom index 7549397a27c4..f38c22611cc4 100644 --- a/include/libcamera/ipa/raspberrypi.mojom +++ b/include/libcamera/ipa/raspberrypi.mojom @@ -29,17 +29,11 @@ struct ISPConfig { ControlList controls; }; -struct ConfigInput { - uint32 op; +struct IPAConfig { uint32 transform; FileDescriptor lsTableHandle; }; -struct ConfigOutput { - uint32 params; - ControlList controls; -}; - struct StartConfig { ControlList controls; int32 dropFrameCount; @@ -57,7 +51,7 @@ interface IPARPiInterface { * \param[in] streamConfig Configuration of all active streams * \param[in] entityControls Controls provided by the pipeline entities * \param[in] ipaConfig Pipeline-handler-specific configuration data - * \param[out] results Pipeline-handler-specific configuration result + * \param[out] controls Controls to apply by the pipeline entity * * This method shall be called when the camera is configured to inform * the IPA of the camera's streams and the sensor settings. @@ -65,14 +59,14 @@ interface IPARPiInterface { * The \a sensorInfo conveys information about the camera sensor settings that * the pipeline handler has selected for the configuration. * - * The \a ipaConfig and \a results parameters carry data passed by the + * The \a ipaConfig and \a controls parameters carry data passed by the * pipeline handler to the IPA and back. */ configure(CameraSensorInfo sensorInfo, map<uint32, IPAStream> streamConfig, map<uint32, ControlInfoMap> entityControls, - ConfigInput ipaConfig) - => (int32 ret, ConfigOutput results); + IPAConfig ipaConfig) + => (int32 ret, ControlList controls); /** * \fn mapBuffers() diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp index 417a7922c3bd..1c928b72fbd0 100644 --- a/src/ipa/raspberrypi/raspberrypi.cpp +++ b/src/ipa/raspberrypi/raspberrypi.cpp @@ -85,8 +85,8 @@ public: int configure(const CameraSensorInfo &sensorInfo, const std::map<unsigned int, IPAStream> &streamConfig, const std::map<unsigned int, ControlInfoMap> &entityControls, - const ipa::RPi::ConfigInput &data, - ipa::RPi::ConfigOutput *response) override; + const ipa::RPi::IPAConfig &data, + ControlList *controls) override; void mapBuffers(const std::vector<IPABuffer> &buffers) override; void unmapBuffers(const std::vector<unsigned int> &ids) override; void signalStatReady(const uint32_t bufferId) override; @@ -313,16 +313,14 @@ void IPARPi::setMode(const CameraSensorInfo &sensorInfo) int IPARPi::configure(const CameraSensorInfo &sensorInfo, [[maybe_unused]] const std::map<unsigned int, IPAStream> &streamConfig, const std::map<unsigned int, ControlInfoMap> &entityControls, - const ipa::RPi::ConfigInput &ipaConfig, - ipa::RPi::ConfigOutput *result) + const ipa::RPi::IPAConfig &ipaConfig, + ControlList *controls) { if (entityControls.size() != 2) { LOG(IPARPI, Error) << "No ISP or sensor controls found."; return -1; } - result->params = 0; - sensorCtrls_ = entityControls.at(0); ispCtrls_ = entityControls.at(1); @@ -379,7 +377,8 @@ int IPARPi::configure(const CameraSensorInfo &sensorInfo, agcStatus.analogue_gain = DefaultAnalogueGain; applyAGC(&agcStatus, ctrls); - result->controls = std::move(ctrls); + ASSERT(controls); + *controls = std::move(ctrls); } return 0; diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index 16568d56f31c..1f8307a3c1c5 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -1240,7 +1240,7 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config) std::map<unsigned int, IPAStream> streamConfig; std::map<unsigned int, ControlInfoMap> entityControls; - ipa::RPi::ConfigInput ipaConfig; + ipa::RPi::IPAConfig ipaConfig; /* Get the device format to pass to the IPA. */ V4L2DeviceFormat sensorFormat; @@ -1283,19 +1283,16 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config) } /* Ready the IPA - it must know about the sensor resolution. */ - ipa::RPi::ConfigOutput result; - + ControlList controls; ret = ipa_->configure(sensorInfo_, streamConfig, entityControls, ipaConfig, - &result); + &controls); if (ret < 0) { LOG(RPI, Error) << "IPA configuration failed!"; return -EPIPE; } - if (!result.controls.empty()) { - ControlList &ctrls = result.controls; - unicam_[Unicam::Image].dev()->setControls(&ctrls); - } + if (!controls.empty()) + unicam_[Unicam::Image].dev()->setControls(&controls); /* * Configure the H/V flip controls based on the combination of
Rename ConfigInput to IPAConfig to be more consistent with the naming, and remove ConfigInput::op, as it is never used. Replace ConfigOutput with a ControlList type, as that is the only return type from ipa::configure(). Signed-off-by: Naushir Patuck <naush@raspberrypi.com> --- include/libcamera/ipa/raspberrypi.mojom | 16 +++++----------- src/ipa/raspberrypi/raspberrypi.cpp | 13 ++++++------- .../pipeline/raspberrypi/raspberrypi.cpp | 13 +++++-------- 3 files changed, 16 insertions(+), 26 deletions(-)