[libcamera-devel] [PATCH v2 3/7] ipa: ipc: Rename CameraSensorInfo to IPACameraSensorInfo
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Fri May 21 11:35:28 CEST 2021
On Fri, May 21, 2021 at 11:33:05AM +0200, Jacopo Mondi wrote:
> On Wed, May 19, 2021 at 03:49:50PM +0530, Umang Jain wrote:
> > This matches the naming convention for IPA IPC.
>
> I wonder, now that we moved the stuct out of CameraSensor, would
> IPASensorInfo be a better fit ?
I'd ba careful about using "sensor" only, as we may end up interfacing
with other types of sensors in the future. It's possibly a bit unlikely
(gyroscopes and accelerometers would most likely be handled outside of
libcamera), but I'd prefer not ruling it out completely.
> > Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> > ---
> > Documentation/guides/ipa.rst | 8 ++--
> > include/libcamera/internal/camera_sensor.h | 2 +-
> > include/libcamera/ipa/core.mojom | 2 +-
> > include/libcamera/ipa/raspberrypi.mojom | 2 +-
> > include/libcamera/ipa/rkisp1.mojom | 2 +-
> > src/ipa/ipu3/ipu3_agc.cpp | 2 +-
> > src/ipa/raspberrypi/raspberrypi.cpp | 8 ++--
> > src/ipa/rkisp1/rkisp1.cpp | 6 +--
> > src/libcamera/camera_sensor.cpp | 6 +--
> > src/libcamera/ipa/core_ipa_interface.cpp | 38 +++++++++----------
> > src/libcamera/pipeline/ipu3/ipu3.cpp | 4 +-
> > .../pipeline/raspberrypi/raspberrypi.cpp | 4 +-
> > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +-
> > 13 files changed, 43 insertions(+), 43 deletions(-)
> >
> > diff --git a/Documentation/guides/ipa.rst b/Documentation/guides/ipa.rst
> > index cbffbd9a..6d460e13 100644
> > --- a/Documentation/guides/ipa.rst
> > +++ b/Documentation/guides/ipa.rst
> > @@ -74,7 +74,7 @@ serialization, any custom data containers must be defined with the mojo IDL.
> > The following list of libcamera objects are supported in the interface
> > definition, and may be used as function parameter types or struct field types:
> >
> > -- libcamera.CameraSensorInfo
> > +- libcamera.IPACameraSensorInfo
> > - libcamera.ControlInfoMap
> > - libcamera.ControlList
> > - libcamera.FileDescriptor
> > @@ -208,7 +208,7 @@ The following is an example of a main interface definition:
> > start() => (int32 ret);
> > stop();
> >
> > - configure(libcamera.CameraSensorInfo sensorInfo,
> > + configure(libcamera.IPACameraSensorInfo sensorInfo,
> > map<uint32, libcamera.IPAStream> streamConfig,
> > map<uint32, libcamera.ControlInfoMap> entityControls,
> > ConfigInput ipaConfig)
> > @@ -470,7 +470,7 @@ definition:
> >
> > .. code-block:: none
> >
> > - configure(libcamera.CameraSensorInfo sensorInfo,
> > + configure(libcamera.IPACameraSensorInfo sensorInfo,
> > uint32 exampleNumber,
> > map<uint32, libcamera.IPAStream> streamConfig,
> > map<uint32, libcamera.ControlInfoMap> entityControls,
> > @@ -481,7 +481,7 @@ We will need to implement a function with the following function signature:
> >
> > .. code-block:: C++
> >
> > - int configure(const CameraSensorInfo &sensorInfo,
> > + int configure(const IPACameraSensorInfo &sensorInfo,
> > uint32_t exampleNumber,
> > const std::map<unsigned int, IPAStream> &streamConfig,
> > const std::map<unsigned int, ControlInfoMap> &entityControls,
> > diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> > index 0905ebfa..cf6c1c1e 100644
> > --- a/include/libcamera/internal/camera_sensor.h
> > +++ b/include/libcamera/internal/camera_sensor.h
> > @@ -51,7 +51,7 @@ public:
> > V4L2Subdevice *device() { return subdev_.get(); }
> >
> > const ControlList &properties() const { return properties_; }
> > - int sensorInfo(CameraSensorInfo *info) const;
> > + int sensorInfo(IPACameraSensorInfo *info) const;
> >
> > void updateControlInfo();
> >
> > diff --git a/include/libcamera/ipa/core.mojom b/include/libcamera/ipa/core.mojom
> > index b95b3dc4..34a799c2 100644
> > --- a/include/libcamera/ipa/core.mojom
> > +++ b/include/libcamera/ipa/core.mojom
> > @@ -78,7 +78,7 @@ module libcamera;
> > uint32 height;
> > };
> >
> > -struct CameraSensorInfo {
> > +struct IPACameraSensorInfo {
> > string model;
> >
> > uint32 bitsPerPixel;
> > diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom
> > index 42321bee..0a21f453 100644
> > --- a/include/libcamera/ipa/raspberrypi.mojom
> > +++ b/include/libcamera/ipa/raspberrypi.mojom
> > @@ -63,7 +63,7 @@ interface IPARPiInterface {
> > * The \a ipaConfig and \a controls parameters carry data passed by the
> > * pipeline handler to the IPA and back.
> > */
> > - configure(libcamera.CameraSensorInfo sensorInfo,
> > + configure(libcamera.IPACameraSensorInfo sensorInfo,
> > map<uint32, libcamera.IPAStream> streamConfig,
> > map<uint32, libcamera.ControlInfoMap> entityControls,
> > IPAConfig ipaConfig)
> > diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom
> > index cca871a0..66a4607c 100644
> > --- a/include/libcamera/ipa/rkisp1.mojom
> > +++ b/include/libcamera/ipa/rkisp1.mojom
> > @@ -29,7 +29,7 @@ interface IPARkISP1Interface {
> > start() => (int32 ret);
> > stop();
> >
> > - configure(libcamera.CameraSensorInfo sensorInfo,
> > + configure(libcamera.IPACameraSensorInfo sensorInfo,
> > map<uint32, libcamera.IPAStream> streamConfig,
> > map<uint32, libcamera.ControlInfoMap> entityControls)
> > => (int32 ret);
> > diff --git a/src/ipa/ipu3/ipu3_agc.cpp b/src/ipa/ipu3/ipu3_agc.cpp
> > index ca54d89a..8bae423f 100644
> > --- a/src/ipa/ipu3/ipu3_agc.cpp
> > +++ b/src/ipa/ipu3/ipu3_agc.cpp
> > @@ -39,7 +39,7 @@ static constexpr uint32_t kMaxGain = kMaxISO / 100;
> > static constexpr uint32_t kMinExposure = 1;
> > static constexpr uint32_t kMaxExposure = 1976;
> >
> > -/* \todo those should be get from CameraSensorInfo ! */
> > +/* \todo those should be got from IPACameraSensorInfo ! */
> > /* line duration in microseconds */
> > static constexpr double kLineDuration = 16.8;
> > static constexpr double kMaxExposureTime = kMaxExposure * kLineDuration;
> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> > index 87774500..e5bb8159 100644
> > --- a/src/ipa/raspberrypi/raspberrypi.cpp
> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> > @@ -89,7 +89,7 @@ public:
> > void start(const ControlList &controls, ipa::RPi::StartConfig *startConfig) override;
> > void stop() override {}
> >
> > - int configure(const CameraSensorInfo &sensorInfo,
> > + int configure(const IPACameraSensorInfo &sensorInfo,
> > const std::map<unsigned int, IPAStream> &streamConfig,
> > const std::map<unsigned int, ControlInfoMap> &entityControls,
> > const ipa::RPi::IPAConfig &data,
> > @@ -101,7 +101,7 @@ public:
> > void signalIspPrepare(const ipa::RPi::ISPConfig &data) override;
> >
> > private:
> > - void setMode(const CameraSensorInfo &sensorInfo);
> > + void setMode(const IPACameraSensorInfo &sensorInfo);
> > bool validateSensorControls();
> > bool validateIspControls();
> > void queueRequest(const ControlList &controls);
> > @@ -278,7 +278,7 @@ void IPARPi::start(const ControlList &controls, ipa::RPi::StartConfig *startConf
> > lastRunTimestamp_ = 0;
> > }
> >
> > -void IPARPi::setMode(const CameraSensorInfo &sensorInfo)
> > +void IPARPi::setMode(const IPACameraSensorInfo &sensorInfo)
> > {
> > mode_.bitdepth = sensorInfo.bitsPerPixel;
> > mode_.width = sensorInfo.outputSize.width;
> > @@ -323,7 +323,7 @@ void IPARPi::setMode(const CameraSensorInfo &sensorInfo)
> > mode_.max_frame_length = sensorInfo.maxFrameLength;
> > }
> >
> > -int IPARPi::configure(const CameraSensorInfo &sensorInfo,
> > +int IPARPi::configure(const IPACameraSensorInfo &sensorInfo,
> > [[maybe_unused]] const std::map<unsigned int, IPAStream> &streamConfig,
> > const std::map<unsigned int, ControlInfoMap> &entityControls,
> > const ipa::RPi::IPAConfig &ipaConfig,
> > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> > index 6d45673c..b47ea324 100644
> > --- a/src/ipa/rkisp1/rkisp1.cpp
> > +++ b/src/ipa/rkisp1/rkisp1.cpp
> > @@ -37,7 +37,7 @@ public:
> > int start() override;
> > void stop() override {}
> >
> > - int configure(const CameraSensorInfo &info,
> > + int configure(const IPACameraSensorInfo &info,
> > const std::map<uint32_t, IPAStream> &streamConfig,
> > const std::map<uint32_t, ControlInfoMap> &entityControls) override;
> > void mapBuffers(const std::vector<IPABuffer> &buffers) override;
> > @@ -90,12 +90,12 @@ int IPARkISP1::start()
> > }
> >
> > /**
> > - * \todo The RkISP1 pipeline currently provides an empty CameraSensorInfo
> > + * \todo The RkISP1 pipeline currently provides an empty IPACameraSensorInfo
> > * if the connected sensor does not provide enough information to properly
> > * assemble one. Make sure the reported sensor information are relevant
> > * before accessing them.
> > */
> > -int IPARkISP1::configure([[maybe_unused]] const CameraSensorInfo &info,
> > +int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info,
> > [[maybe_unused]] const std::map<uint32_t, IPAStream> &streamConfig,
> > const std::map<uint32_t, ControlInfoMap> &entityControls)
> > {
> > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> > index 170de827..0fb8a258 100644
> > --- a/src/libcamera/camera_sensor.cpp
> > +++ b/src/libcamera/camera_sensor.cpp
> > @@ -205,7 +205,7 @@ int CameraSensor::validateSensorDriver()
> > *
> > * Failures in reading any of the targets are not deemed to be fatal,
> > * but some properties and features, like constructing a
> > - * CameraSensorInfo for the IPA module, won't be supported.
> > + * IPACameraSensorInfo for the IPA module, won't be supported.
> > *
> > * \todo Make support for selection targets mandatory as soon as all
> > * test platforms have been updated.
> > @@ -674,7 +674,7 @@ int CameraSensor::setControls(ControlList *ctrls)
> > *
> > * \return 0 on success, a negative error code otherwise
> > */
> > -int CameraSensor::sensorInfo(CameraSensorInfo *info) const
> > +int CameraSensor::sensorInfo(IPACameraSensorInfo *info) const
> > {
> > if (!bayerFormat_)
> > return -EINVAL;
> > @@ -701,7 +701,7 @@ int CameraSensor::sensorInfo(CameraSensorInfo *info) const
> > }
> >
> > /*
> > - * CameraSensorInfo::analogCrop::x and CameraSensorInfo::analogCrop::y
> > + * IPACameraSensorInfo::analogCrop::x and IPACameraSensorInfo::analogCrop::y
> > * are defined relatively to the active pixel area, while V4L2's
> > * TGT_CROP target is defined in respect to the full pixel array.
> > *
> > diff --git a/src/libcamera/ipa/core_ipa_interface.cpp b/src/libcamera/ipa/core_ipa_interface.cpp
> > index def8d184..15d61d91 100644
> > --- a/src/libcamera/ipa/core_ipa_interface.cpp
> > +++ b/src/libcamera/ipa/core_ipa_interface.cpp
> > @@ -103,7 +103,7 @@ namespace libcamera {
> > */
> >
> > /**
> > - * \struct CameraSensorInfo
> > + * \struct IPACameraSensorInfo
> > * \brief Report the image sensor characteristics
> > *
> > * The structure reports image sensor characteristics used by IPA modules to
> > @@ -121,15 +121,15 @@ namespace libcamera {
> > */
> >
> > /**
> > - * \fn CameraSensorInfo::CameraSensorInfo(const std::string &model,
> > - uint32_t bitsPerPixel,
> > - const Size &activeAreaSize,
> > - const Rectangle &analogCrop,
> > - const Size &outputSize,
> > - uint64_t pixelRate,
> > - uint32_t lineLength,
> > - uint32_t minFrameLength,
> > - uint32_t maxFrameLength)
> > + * \fn IPACameraSensorInfo::IPACameraSensorInfo(const std::string &model,
> > + uint32_t bitsPerPixel,
> > + const Size &activeAreaSize,
> > + const Rectangle &analogCrop,
> > + const Size &outputSize,
> > + uint64_t pixelRate,
> > + uint32_t lineLength,
> > + uint32_t minFrameLength,
> > + uint32_t maxFrameLength)
> > * \param[in] model
> > * \param[in] bitsPerPixel
> > * \param[in] activeAreaSize
> > @@ -142,7 +142,7 @@ namespace libcamera {
> > */
> >
> > /**
> > - * \var CameraSensorInfo::model
> > + * \var IPACameraSensorInfo::model
> > * \brief The image sensor model name
> > *
> > * The sensor model name is a free-formed string that uniquely identifies the
> > @@ -150,18 +150,18 @@ namespace libcamera {
> > */
> >
> > /**
> > - * \var CameraSensorInfo::bitsPerPixel
> > + * \var IPACameraSensorInfo::bitsPerPixel
> > * \brief The number of bits per pixel of the image format produced by the
> > * image sensor
> > */
> >
> > /**
> > - * \var CameraSensorInfo::activeAreaSize
> > + * \var IPACameraSensorInfo::activeAreaSize
> > * \brief The size of the pixel array active area of the sensor
> > */
> >
> > /**
> > - * \var CameraSensorInfo::analogCrop
> > + * \var IPACameraSensorInfo::analogCrop
> > * \brief The portion of the pixel array active area which is read-out and
> > * processed
> > *
> > @@ -174,7 +174,7 @@ namespace libcamera {
> > */
> >
> > /**
> > - * \var CameraSensorInfo::outputSize
> > + * \var IPACameraSensorInfo::outputSize
> > * \brief The size of the images produced by the camera sensor
> > *
> > * The output image size defines the horizontal and vertical sizes of the images
> > @@ -187,7 +187,7 @@ namespace libcamera {
> > */
> >
> > /**
> > - * \var CameraSensorInfo::pixelRate
> > + * \var IPACameraSensorInfo::pixelRate
> > * \brief The number of pixels produced in a second
> > *
> > * To obtain the read-out time in seconds of a full line:
> > @@ -198,14 +198,14 @@ namespace libcamera {
> > */
> >
> > /**
> > - * \var CameraSensorInfo::lineLength
> > + * \var IPACameraSensorInfo::lineLength
> > * \brief Total line length in pixels
> > *
> > * The total line length in pixel clock periods, including blanking.
> > */
> >
> > /**
> > - * \var CameraSensorInfo::minFrameLength
> > + * \var IPACameraSensorInfo::minFrameLength
> > * \brief The minimum allowable frame length in units of lines
> > *
> > * The sensor frame length comprises of active output lines and blanking lines
> > @@ -220,7 +220,7 @@ namespace libcamera {
> > */
> >
> > /**
> > - * \var CameraSensorInfo::maxFrameLength
> > + * \var IPACameraSensorInfo::maxFrameLength
> > * \brief The maximum allowable frame length in units of lines
> > *
> > * The sensor frame length comprises of active output lines and blanking lines
> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > index ade8ffbd..98c6160f 100644
> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > @@ -544,7 +544,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
> > if (ret)
> > return ret;
> >
> > - CameraSensorInfo sensorInfo;
> > + IPACameraSensorInfo sensorInfo;
> > cio2->sensor()->sensorInfo(&sensorInfo);
> > data->cropRegion_ = sensorInfo.analogCrop;
> >
> > @@ -883,7 +883,7 @@ int PipelineHandlerIPU3::initControls(IPU3CameraData *data)
> > if (ret)
> > return ret;
> >
> > - CameraSensorInfo sensorInfo{};
> > + IPACameraSensorInfo sensorInfo{};
> > ret = sensor->sensorInfo(&sensorInfo);
> > if (ret)
> > return ret;
> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > index 6fbdba04..f2a94dc0 100644
> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > @@ -211,7 +211,7 @@ public:
> > BayerFormat::Order nativeBayerOrder_;
> >
> > /* For handling digital zoom. */
> > - CameraSensorInfo sensorInfo_;
> > + IPACameraSensorInfo sensorInfo_;
> > Rectangle ispCrop_; /* crop in ISP (camera mode) pixels */
> > Rectangle scalerCrop_; /* crop in sensor native pixels */
> > bool updateScalerCrop_;
> > @@ -1275,7 +1275,7 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)
> > ipaConfig.lsTableHandle = lsTable_;
> > }
> >
> > - /* We store the CameraSensorInfo for digital zoom calculations. */
> > + /* We store the IPACameraSensorInfo for digital zoom calculations. */
> > int ret = sensor_->sensorInfo(&sensorInfo_);
> > if (ret) {
> > LOG(RPI, Error) << "Failed to retrieve camera sensor info";
> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > index adebe8b5..6699839c 100644
> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > @@ -648,7 +648,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
> > return ret;
> >
> > /* Inform IPA of stream configuration and sensor controls. */
> > - CameraSensorInfo sensorInfo = {};
> > + IPACameraSensorInfo sensorInfo = {};
> > ret = data->sensor_->sensorInfo(&sensorInfo);
> > if (ret) {
> > /* \todo Turn this into a hard failure. */
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list