| Message ID | 20240301212121.9072-17-laurent.pinchart@ideasonboard.com |
|---|---|
| State | RFC |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent On Fri, Mar 01, 2024 at 11:21:05PM +0200, Laurent Pinchart wrote: > The CameraSensor class has grown a lot since its creation, with many > functions added for different types of purposes. They are not grouped by > categories in the class definition, generating confusion when reading > the header file. Improve readability by sorting functions by category: > > - Getters for static data (model, entity, focus lens, ...) > - Format and sensor configuration accessors > - Properties and controls (including test pattern mode) > > Update the .cpp file accordingly. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > include/libcamera/internal/camera_sensor.h | 29 +- > src/libcamera/sensor/camera_sensor.cpp | 328 ++++++++++----------- > 2 files changed, 179 insertions(+), 178 deletions(-) > > diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h > index b2f077b9cc75..750d6d729cac 100644 > --- a/include/libcamera/internal/camera_sensor.h > +++ b/include/libcamera/internal/camera_sensor.h > @@ -46,15 +46,15 @@ public: > > const std::string &model() const { return model_; } > const std::string &id() const { return id_; } > + > const MediaEntity *entity() const { return entity_; } > + V4L2Subdevice *device() { return subdev_.get(); } > + > + CameraLens *focusLens() { return focusLens_.get(); } > + Should you move const ControlList &properties() const { return properties_; } const ControlInfoMap &controls() const; here ? (I understand if you want to keep them grouped though) This can be fast-tracked too! Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > const std::vector<unsigned int> &mbusCodes() const { return mbusCodes_; } > std::vector<Size> sizes(unsigned int mbusCode) const; > Size resolution() const; > - const std::vector<controls::draft::TestPatternModeEnum> &testPatternModes() const > - { > - return testPatternModes_; > - } > - int setTestPatternMode(controls::draft::TestPatternModeEnum mode); > > V4L2SubdeviceFormat getFormat(const std::vector<unsigned int> &mbusCodes, > const Size &size) const; > @@ -66,18 +66,19 @@ public: > Transform transform = Transform::Identity, > V4L2SubdeviceFormat *sensorFormat = nullptr); > > + const ControlList &properties() const { return properties_; } > + int sensorInfo(IPACameraSensorInfo *info) const; > + Transform computeTransform(Orientation *orientation) const; > + > const ControlInfoMap &controls() const; > ControlList getControls(const std::vector<uint32_t> &ids); > int setControls(ControlList *ctrls); > > - V4L2Subdevice *device() { return subdev_.get(); } > - > - const ControlList &properties() const { return properties_; } > - int sensorInfo(IPACameraSensorInfo *info) const; > - > - CameraLens *focusLens() { return focusLens_.get(); } > - > - Transform computeTransform(Orientation *orientation) const; > + const std::vector<controls::draft::TestPatternModeEnum> &testPatternModes() const > + { > + return testPatternModes_; > + } > + int setTestPatternMode(controls::draft::TestPatternModeEnum mode); > > protected: > std::string logPrefix() const override; > @@ -91,8 +92,8 @@ private: > void initStaticProperties(); > void initTestPatternModes(); > int initProperties(); > - int applyTestPatternMode(controls::draft::TestPatternModeEnum mode); > int discoverAncillaryDevices(); > + int applyTestPatternMode(controls::draft::TestPatternModeEnum mode); > > const MediaEntity *entity_; > std::unique_ptr<V4L2Subdevice> subdev_; > diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp > index 545f89d036df..86ad9a85371c 100644 > --- a/src/libcamera/sensor/camera_sensor.cpp > +++ b/src/libcamera/sensor/camera_sensor.cpp > @@ -215,6 +215,30 @@ int CameraSensor::init() > return applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff); > } > > +int CameraSensor::generateId() > +{ > + const std::string devPath = subdev_->devicePath(); > + > + /* Try to get ID from firmware description. */ > + id_ = sysfs::firmwareNodePath(devPath); > + if (!id_.empty()) > + return 0; > + > + /* > + * Virtual sensors not described in firmware > + * > + * Verify it's a platform device and construct ID from the device path > + * and model of sensor. > + */ > + if (devPath.find("/sys/devices/platform/", 0) == 0) { > + id_ = devPath.substr(strlen("/sys/devices/")) + " " + model(); > + return 0; > + } > + > + LOG(CameraSensor, Error) << "Can't generate sensor ID"; > + return -EINVAL; > +} > + > int CameraSensor::validateSensorDriver() > { > int err = 0; > @@ -580,6 +604,21 @@ int CameraSensor::discoverAncillaryDevices() > * \return The sensor media entity > */ > > +/** > + * \fn CameraSensor::device() > + * \brief Retrieve the camera sensor device > + * \todo Remove this function by integrating DelayedControl with CameraSensor > + * \return The camera sensor device > + */ > + > +/** > + * \fn CameraSensor::focusLens() > + * \brief Retrieve the focus lens controller > + * > + * \return The focus lens controller. nullptr if no focus lens controller is > + * connected to the sensor > + */ > + > /** > * \fn CameraSensor::mbusCodes() > * \brief Retrieve the media bus codes supported by the camera sensor > @@ -632,64 +671,6 @@ Size CameraSensor::resolution() const > return std::min(sizes_.back(), activeArea_.size()); > } > > -/** > - * \fn CameraSensor::testPatternModes() > - * \brief Retrieve all the supported test pattern modes of the camera sensor > - * The test pattern mode values correspond to the controls::TestPattern control. > - * > - * \return The list of test pattern modes > - */ > - > -/** > - * \brief Set the test pattern mode for the camera sensor > - * \param[in] mode The test pattern mode > - * > - * The new \a mode is applied to the sensor if it differs from the active test > - * pattern mode. Otherwise, this function is a no-op. Setting the same test > - * pattern mode for every frame thus incurs no performance penalty. > - */ > -int CameraSensor::setTestPatternMode(controls::draft::TestPatternModeEnum mode) > -{ > - if (testPatternMode_ == mode) > - return 0; > - > - if (testPatternModes_.empty()) { > - LOG(CameraSensor, Error) > - << "Camera sensor does not support test pattern modes."; > - return -EINVAL; > - } > - > - return applyTestPatternMode(mode); > -} > - > -int CameraSensor::applyTestPatternMode(controls::draft::TestPatternModeEnum mode) > -{ > - if (testPatternModes_.empty()) > - return 0; > - > - auto it = std::find(testPatternModes_.begin(), testPatternModes_.end(), > - mode); > - if (it == testPatternModes_.end()) { > - LOG(CameraSensor, Error) << "Unsupported test pattern mode " > - << mode; > - return -EINVAL; > - } > - > - LOG(CameraSensor, Debug) << "Apply test pattern mode " << mode; > - > - int32_t index = staticProps_->testPatternModes.at(mode); > - ControlList ctrls{ controls() }; > - ctrls.set(V4L2_CID_TEST_PATTERN, index); > - > - int ret = setControls(&ctrls); > - if (ret) > - return ret; > - > - testPatternMode_ = mode; > - > - return 0; > -} > - > /** > * \brief Retrieve the best sensor format for a desired output > * \param[in] mbusCodes The list of acceptable media bus codes > @@ -919,80 +900,6 @@ int CameraSensor::applyConfiguration(const SensorConfiguration &config, > return 0; > } > > -/** > - * \brief Retrieve the supported V4L2 controls and their information > - * > - * Control information is updated automatically to reflect the current sensor > - * configuration when the setFormat() function is called, without invalidating > - * any iterator on the ControlInfoMap. > - * > - * \return A map of the V4L2 controls supported by the sensor > - */ > -const ControlInfoMap &CameraSensor::controls() const > -{ > - return subdev_->controls(); > -} > - > -/** > - * \brief Read V4L2 controls from the sensor > - * \param[in] ids The list of controls to read, specified by their ID > - * > - * This function reads the value of all controls contained in \a ids, and > - * returns their values as a ControlList. The control identifiers are defined by > - * the V4L2 specification (V4L2_CID_*). > - * > - * If any control in \a ids is not supported by the device, is disabled (i.e. > - * has the V4L2_CTRL_FLAG_DISABLED flag set), or if any other error occurs > - * during validation of the requested controls, no control is read and this > - * function returns an empty control list. > - * > - * \sa V4L2Device::getControls() > - * > - * \return The control values in a ControlList on success, or an empty list on > - * error > - */ > -ControlList CameraSensor::getControls(const std::vector<uint32_t> &ids) > -{ > - return subdev_->getControls(ids); > -} > - > -/** > - * \brief Write V4L2 controls to the sensor > - * \param[in] ctrls The list of controls to write > - * > - * This function writes the value of all controls contained in \a ctrls, and > - * stores the values actually applied to the device in the corresponding \a > - * ctrls entry. The control identifiers are defined by the V4L2 specification > - * (V4L2_CID_*). > - * > - * If any control in \a ctrls is not supported by the device, is disabled (i.e. > - * has the V4L2_CTRL_FLAG_DISABLED flag set), is read-only, or if any other > - * error occurs during validation of the requested controls, no control is > - * written and this function returns -EINVAL. > - * > - * If an error occurs while writing the controls, the index of the first > - * control that couldn't be written is returned. All controls below that index > - * are written and their values are updated in \a ctrls, while all other > - * controls are not written and their values are not changed. > - * > - * \sa V4L2Device::setControls() > - * > - * \return 0 on success or an error code otherwise > - * \retval -EINVAL One of the control is not supported or not accessible > - * \retval i The index of the control that failed > - */ > -int CameraSensor::setControls(ControlList *ctrls) > -{ > - return subdev_->setControls(ctrls); > -} > - > -/** > - * \fn CameraSensor::device() > - * \brief Retrieve the camera sensor device > - * \todo Remove this function by integrating DelayedControl with CameraSensor > - * \return The camera sensor device > - */ > - > /** > * \fn CameraSensor::properties() > * \brief Retrieve the camera sensor properties > @@ -1088,14 +995,6 @@ int CameraSensor::sensorInfo(IPACameraSensorInfo *info) const > return 0; > } > > -/** > - * \fn CameraSensor::focusLens() > - * \brief Retrieve the focus lens controller > - * > - * \return The focus lens controller. nullptr if no focus lens controller is > - * connected to the sensor > - */ > - > /** > * \brief Compute the Transform that gives the requested \a orientation > * \param[inout] orientation The desired image orientation > @@ -1155,33 +1054,134 @@ Transform CameraSensor::computeTransform(Orientation *orientation) const > return transform; > } > > +/** > + * \brief Retrieve the supported V4L2 controls and their information > + * > + * Control information is updated automatically to reflect the current sensor > + * configuration when the setFormat() function is called, without invalidating > + * any iterator on the ControlInfoMap. > + * > + * \return A map of the V4L2 controls supported by the sensor > + */ > +const ControlInfoMap &CameraSensor::controls() const > +{ > + return subdev_->controls(); > +} > + > +/** > + * \brief Read V4L2 controls from the sensor > + * \param[in] ids The list of controls to read, specified by their ID > + * > + * This function reads the value of all controls contained in \a ids, and > + * returns their values as a ControlList. The control identifiers are defined by > + * the V4L2 specification (V4L2_CID_*). > + * > + * If any control in \a ids is not supported by the device, is disabled (i.e. > + * has the V4L2_CTRL_FLAG_DISABLED flag set), or if any other error occurs > + * during validation of the requested controls, no control is read and this > + * function returns an empty control list. > + * > + * \sa V4L2Device::getControls() > + * > + * \return The control values in a ControlList on success, or an empty list on > + * error > + */ > +ControlList CameraSensor::getControls(const std::vector<uint32_t> &ids) > +{ > + return subdev_->getControls(ids); > +} > + > +/** > + * \brief Write V4L2 controls to the sensor > + * \param[in] ctrls The list of controls to write > + * > + * This function writes the value of all controls contained in \a ctrls, and > + * stores the values actually applied to the device in the corresponding \a > + * ctrls entry. The control identifiers are defined by the V4L2 specification > + * (V4L2_CID_*). > + * > + * If any control in \a ctrls is not supported by the device, is disabled (i.e. > + * has the V4L2_CTRL_FLAG_DISABLED flag set), is read-only, or if any other > + * error occurs during validation of the requested controls, no control is > + * written and this function returns -EINVAL. > + * > + * If an error occurs while writing the controls, the index of the first > + * control that couldn't be written is returned. All controls below that index > + * are written and their values are updated in \a ctrls, while all other > + * controls are not written and their values are not changed. > + * > + * \sa V4L2Device::setControls() > + * > + * \return 0 on success or an error code otherwise > + * \retval -EINVAL One of the control is not supported or not accessible > + * \retval i The index of the control that failed > + */ > +int CameraSensor::setControls(ControlList *ctrls) > +{ > + return subdev_->setControls(ctrls); > +} > + > +/** > + * \fn CameraSensor::testPatternModes() > + * \brief Retrieve all the supported test pattern modes of the camera sensor > + * The test pattern mode values correspond to the controls::TestPattern control. > + * > + * \return The list of test pattern modes > + */ > + > +/** > + * \brief Set the test pattern mode for the camera sensor > + * \param[in] mode The test pattern mode > + * > + * The new \a mode is applied to the sensor if it differs from the active test > + * pattern mode. Otherwise, this function is a no-op. Setting the same test > + * pattern mode for every frame thus incurs no performance penalty. > + */ > +int CameraSensor::setTestPatternMode(controls::draft::TestPatternModeEnum mode) > +{ > + if (testPatternMode_ == mode) > + return 0; > + > + if (testPatternModes_.empty()) { > + LOG(CameraSensor, Error) > + << "Camera sensor does not support test pattern modes."; > + return -EINVAL; > + } > + > + return applyTestPatternMode(mode); > +} > + > +int CameraSensor::applyTestPatternMode(controls::draft::TestPatternModeEnum mode) > +{ > + if (testPatternModes_.empty()) > + return 0; > + > + auto it = std::find(testPatternModes_.begin(), testPatternModes_.end(), > + mode); > + if (it == testPatternModes_.end()) { > + LOG(CameraSensor, Error) << "Unsupported test pattern mode " > + << mode; > + return -EINVAL; > + } > + > + LOG(CameraSensor, Debug) << "Apply test pattern mode " << mode; > + > + int32_t index = staticProps_->testPatternModes.at(mode); > + ControlList ctrls{ controls() }; > + ctrls.set(V4L2_CID_TEST_PATTERN, index); > + > + int ret = setControls(&ctrls); > + if (ret) > + return ret; > + > + testPatternMode_ = mode; > + > + return 0; > +} > + > std::string CameraSensor::logPrefix() const > { > return "'" + entity_->name() + "'"; > } > > -int CameraSensor::generateId() > -{ > - const std::string devPath = subdev_->devicePath(); > - > - /* Try to get ID from firmware description. */ > - id_ = sysfs::firmwareNodePath(devPath); > - if (!id_.empty()) > - return 0; > - > - /* > - * Virtual sensors not described in firmware > - * > - * Verify it's a platform device and construct ID from the device path > - * and model of sensor. > - */ > - if (devPath.find("/sys/devices/platform/", 0) == 0) { > - id_ = devPath.substr(strlen("/sys/devices/")) + " " + model(); > - return 0; > - } > - > - LOG(CameraSensor, Error) << "Can't generate sensor ID"; > - return -EINVAL; > -} > - > } /* namespace libcamera */ > -- > Regards, > > Laurent Pinchart >
On Mon, Mar 04, 2024 at 06:37:15PM +0100, Jacopo Mondi wrote: > Hi Laurent > > On Fri, Mar 01, 2024 at 11:21:05PM +0200, Laurent Pinchart wrote: > > The CameraSensor class has grown a lot since its creation, with many > > functions added for different types of purposes. They are not grouped by > > categories in the class definition, generating confusion when reading > > the header file. Improve readability by sorting functions by category: > > > > - Getters for static data (model, entity, focus lens, ...) > > - Format and sensor configuration accessors > > - Properties and controls (including test pattern mode) > > > > Update the .cpp file accordingly. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > include/libcamera/internal/camera_sensor.h | 29 +- > > src/libcamera/sensor/camera_sensor.cpp | 328 ++++++++++----------- > > 2 files changed, 179 insertions(+), 178 deletions(-) > > > > diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h > > index b2f077b9cc75..750d6d729cac 100644 > > --- a/include/libcamera/internal/camera_sensor.h > > +++ b/include/libcamera/internal/camera_sensor.h > > @@ -46,15 +46,15 @@ public: > > > > const std::string &model() const { return model_; } > > const std::string &id() const { return id_; } > > + > > const MediaEntity *entity() const { return entity_; } > > + V4L2Subdevice *device() { return subdev_.get(); } > > + > > + CameraLens *focusLens() { return focusLens_.get(); } > > + > > Should you move > const ControlList &properties() const { return properties_; } > const ControlInfoMap &controls() const; > > here ? > > (I understand if you want to keep them grouped though) I specifically decided to put the properties just before the controls :-) > This can be fast-tracked too! > > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > > const std::vector<unsigned int> &mbusCodes() const { return mbusCodes_; } > > std::vector<Size> sizes(unsigned int mbusCode) const; > > Size resolution() const; > > - const std::vector<controls::draft::TestPatternModeEnum> &testPatternModes() const > > - { > > - return testPatternModes_; > > - } > > - int setTestPatternMode(controls::draft::TestPatternModeEnum mode); > > > > V4L2SubdeviceFormat getFormat(const std::vector<unsigned int> &mbusCodes, > > const Size &size) const; > > @@ -66,18 +66,19 @@ public: > > Transform transform = Transform::Identity, > > V4L2SubdeviceFormat *sensorFormat = nullptr); > > > > + const ControlList &properties() const { return properties_; } > > + int sensorInfo(IPACameraSensorInfo *info) const; > > + Transform computeTransform(Orientation *orientation) const; > > + > > const ControlInfoMap &controls() const; > > ControlList getControls(const std::vector<uint32_t> &ids); > > int setControls(ControlList *ctrls); > > > > - V4L2Subdevice *device() { return subdev_.get(); } > > - > > - const ControlList &properties() const { return properties_; } > > - int sensorInfo(IPACameraSensorInfo *info) const; > > - > > - CameraLens *focusLens() { return focusLens_.get(); } > > - > > - Transform computeTransform(Orientation *orientation) const; > > + const std::vector<controls::draft::TestPatternModeEnum> &testPatternModes() const > > + { > > + return testPatternModes_; > > + } > > + int setTestPatternMode(controls::draft::TestPatternModeEnum mode); > > > > protected: > > std::string logPrefix() const override; > > @@ -91,8 +92,8 @@ private: > > void initStaticProperties(); > > void initTestPatternModes(); > > int initProperties(); > > - int applyTestPatternMode(controls::draft::TestPatternModeEnum mode); > > int discoverAncillaryDevices(); > > + int applyTestPatternMode(controls::draft::TestPatternModeEnum mode); > > > > const MediaEntity *entity_; > > std::unique_ptr<V4L2Subdevice> subdev_; > > diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp > > index 545f89d036df..86ad9a85371c 100644 > > --- a/src/libcamera/sensor/camera_sensor.cpp > > +++ b/src/libcamera/sensor/camera_sensor.cpp > > @@ -215,6 +215,30 @@ int CameraSensor::init() > > return applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff); > > } > > > > +int CameraSensor::generateId() > > +{ > > + const std::string devPath = subdev_->devicePath(); > > + > > + /* Try to get ID from firmware description. */ > > + id_ = sysfs::firmwareNodePath(devPath); > > + if (!id_.empty()) > > + return 0; > > + > > + /* > > + * Virtual sensors not described in firmware > > + * > > + * Verify it's a platform device and construct ID from the device path > > + * and model of sensor. > > + */ > > + if (devPath.find("/sys/devices/platform/", 0) == 0) { > > + id_ = devPath.substr(strlen("/sys/devices/")) + " " + model(); > > + return 0; > > + } > > + > > + LOG(CameraSensor, Error) << "Can't generate sensor ID"; > > + return -EINVAL; > > +} > > + > > int CameraSensor::validateSensorDriver() > > { > > int err = 0; > > @@ -580,6 +604,21 @@ int CameraSensor::discoverAncillaryDevices() > > * \return The sensor media entity > > */ > > > > +/** > > + * \fn CameraSensor::device() > > + * \brief Retrieve the camera sensor device > > + * \todo Remove this function by integrating DelayedControl with CameraSensor > > + * \return The camera sensor device > > + */ > > + > > +/** > > + * \fn CameraSensor::focusLens() > > + * \brief Retrieve the focus lens controller > > + * > > + * \return The focus lens controller. nullptr if no focus lens controller is > > + * connected to the sensor > > + */ > > + > > /** > > * \fn CameraSensor::mbusCodes() > > * \brief Retrieve the media bus codes supported by the camera sensor > > @@ -632,64 +671,6 @@ Size CameraSensor::resolution() const > > return std::min(sizes_.back(), activeArea_.size()); > > } > > > > -/** > > - * \fn CameraSensor::testPatternModes() > > - * \brief Retrieve all the supported test pattern modes of the camera sensor > > - * The test pattern mode values correspond to the controls::TestPattern control. > > - * > > - * \return The list of test pattern modes > > - */ > > - > > -/** > > - * \brief Set the test pattern mode for the camera sensor > > - * \param[in] mode The test pattern mode > > - * > > - * The new \a mode is applied to the sensor if it differs from the active test > > - * pattern mode. Otherwise, this function is a no-op. Setting the same test > > - * pattern mode for every frame thus incurs no performance penalty. > > - */ > > -int CameraSensor::setTestPatternMode(controls::draft::TestPatternModeEnum mode) > > -{ > > - if (testPatternMode_ == mode) > > - return 0; > > - > > - if (testPatternModes_.empty()) { > > - LOG(CameraSensor, Error) > > - << "Camera sensor does not support test pattern modes."; > > - return -EINVAL; > > - } > > - > > - return applyTestPatternMode(mode); > > -} > > - > > -int CameraSensor::applyTestPatternMode(controls::draft::TestPatternModeEnum mode) > > -{ > > - if (testPatternModes_.empty()) > > - return 0; > > - > > - auto it = std::find(testPatternModes_.begin(), testPatternModes_.end(), > > - mode); > > - if (it == testPatternModes_.end()) { > > - LOG(CameraSensor, Error) << "Unsupported test pattern mode " > > - << mode; > > - return -EINVAL; > > - } > > - > > - LOG(CameraSensor, Debug) << "Apply test pattern mode " << mode; > > - > > - int32_t index = staticProps_->testPatternModes.at(mode); > > - ControlList ctrls{ controls() }; > > - ctrls.set(V4L2_CID_TEST_PATTERN, index); > > - > > - int ret = setControls(&ctrls); > > - if (ret) > > - return ret; > > - > > - testPatternMode_ = mode; > > - > > - return 0; > > -} > > - > > /** > > * \brief Retrieve the best sensor format for a desired output > > * \param[in] mbusCodes The list of acceptable media bus codes > > @@ -919,80 +900,6 @@ int CameraSensor::applyConfiguration(const SensorConfiguration &config, > > return 0; > > } > > > > -/** > > - * \brief Retrieve the supported V4L2 controls and their information > > - * > > - * Control information is updated automatically to reflect the current sensor > > - * configuration when the setFormat() function is called, without invalidating > > - * any iterator on the ControlInfoMap. > > - * > > - * \return A map of the V4L2 controls supported by the sensor > > - */ > > -const ControlInfoMap &CameraSensor::controls() const > > -{ > > - return subdev_->controls(); > > -} > > - > > -/** > > - * \brief Read V4L2 controls from the sensor > > - * \param[in] ids The list of controls to read, specified by their ID > > - * > > - * This function reads the value of all controls contained in \a ids, and > > - * returns their values as a ControlList. The control identifiers are defined by > > - * the V4L2 specification (V4L2_CID_*). > > - * > > - * If any control in \a ids is not supported by the device, is disabled (i.e. > > - * has the V4L2_CTRL_FLAG_DISABLED flag set), or if any other error occurs > > - * during validation of the requested controls, no control is read and this > > - * function returns an empty control list. > > - * > > - * \sa V4L2Device::getControls() > > - * > > - * \return The control values in a ControlList on success, or an empty list on > > - * error > > - */ > > -ControlList CameraSensor::getControls(const std::vector<uint32_t> &ids) > > -{ > > - return subdev_->getControls(ids); > > -} > > - > > -/** > > - * \brief Write V4L2 controls to the sensor > > - * \param[in] ctrls The list of controls to write > > - * > > - * This function writes the value of all controls contained in \a ctrls, and > > - * stores the values actually applied to the device in the corresponding \a > > - * ctrls entry. The control identifiers are defined by the V4L2 specification > > - * (V4L2_CID_*). > > - * > > - * If any control in \a ctrls is not supported by the device, is disabled (i.e. > > - * has the V4L2_CTRL_FLAG_DISABLED flag set), is read-only, or if any other > > - * error occurs during validation of the requested controls, no control is > > - * written and this function returns -EINVAL. > > - * > > - * If an error occurs while writing the controls, the index of the first > > - * control that couldn't be written is returned. All controls below that index > > - * are written and their values are updated in \a ctrls, while all other > > - * controls are not written and their values are not changed. > > - * > > - * \sa V4L2Device::setControls() > > - * > > - * \return 0 on success or an error code otherwise > > - * \retval -EINVAL One of the control is not supported or not accessible > > - * \retval i The index of the control that failed > > - */ > > -int CameraSensor::setControls(ControlList *ctrls) > > -{ > > - return subdev_->setControls(ctrls); > > -} > > - > > -/** > > - * \fn CameraSensor::device() > > - * \brief Retrieve the camera sensor device > > - * \todo Remove this function by integrating DelayedControl with CameraSensor > > - * \return The camera sensor device > > - */ > > - > > /** > > * \fn CameraSensor::properties() > > * \brief Retrieve the camera sensor properties > > @@ -1088,14 +995,6 @@ int CameraSensor::sensorInfo(IPACameraSensorInfo *info) const > > return 0; > > } > > > > -/** > > - * \fn CameraSensor::focusLens() > > - * \brief Retrieve the focus lens controller > > - * > > - * \return The focus lens controller. nullptr if no focus lens controller is > > - * connected to the sensor > > - */ > > - > > /** > > * \brief Compute the Transform that gives the requested \a orientation > > * \param[inout] orientation The desired image orientation > > @@ -1155,33 +1054,134 @@ Transform CameraSensor::computeTransform(Orientation *orientation) const > > return transform; > > } > > > > +/** > > + * \brief Retrieve the supported V4L2 controls and their information > > + * > > + * Control information is updated automatically to reflect the current sensor > > + * configuration when the setFormat() function is called, without invalidating > > + * any iterator on the ControlInfoMap. > > + * > > + * \return A map of the V4L2 controls supported by the sensor > > + */ > > +const ControlInfoMap &CameraSensor::controls() const > > +{ > > + return subdev_->controls(); > > +} > > + > > +/** > > + * \brief Read V4L2 controls from the sensor > > + * \param[in] ids The list of controls to read, specified by their ID > > + * > > + * This function reads the value of all controls contained in \a ids, and > > + * returns their values as a ControlList. The control identifiers are defined by > > + * the V4L2 specification (V4L2_CID_*). > > + * > > + * If any control in \a ids is not supported by the device, is disabled (i.e. > > + * has the V4L2_CTRL_FLAG_DISABLED flag set), or if any other error occurs > > + * during validation of the requested controls, no control is read and this > > + * function returns an empty control list. > > + * > > + * \sa V4L2Device::getControls() > > + * > > + * \return The control values in a ControlList on success, or an empty list on > > + * error > > + */ > > +ControlList CameraSensor::getControls(const std::vector<uint32_t> &ids) > > +{ > > + return subdev_->getControls(ids); > > +} > > + > > +/** > > + * \brief Write V4L2 controls to the sensor > > + * \param[in] ctrls The list of controls to write > > + * > > + * This function writes the value of all controls contained in \a ctrls, and > > + * stores the values actually applied to the device in the corresponding \a > > + * ctrls entry. The control identifiers are defined by the V4L2 specification > > + * (V4L2_CID_*). > > + * > > + * If any control in \a ctrls is not supported by the device, is disabled (i.e. > > + * has the V4L2_CTRL_FLAG_DISABLED flag set), is read-only, or if any other > > + * error occurs during validation of the requested controls, no control is > > + * written and this function returns -EINVAL. > > + * > > + * If an error occurs while writing the controls, the index of the first > > + * control that couldn't be written is returned. All controls below that index > > + * are written and their values are updated in \a ctrls, while all other > > + * controls are not written and their values are not changed. > > + * > > + * \sa V4L2Device::setControls() > > + * > > + * \return 0 on success or an error code otherwise > > + * \retval -EINVAL One of the control is not supported or not accessible > > + * \retval i The index of the control that failed > > + */ > > +int CameraSensor::setControls(ControlList *ctrls) > > +{ > > + return subdev_->setControls(ctrls); > > +} > > + > > +/** > > + * \fn CameraSensor::testPatternModes() > > + * \brief Retrieve all the supported test pattern modes of the camera sensor > > + * The test pattern mode values correspond to the controls::TestPattern control. > > + * > > + * \return The list of test pattern modes > > + */ > > + > > +/** > > + * \brief Set the test pattern mode for the camera sensor > > + * \param[in] mode The test pattern mode > > + * > > + * The new \a mode is applied to the sensor if it differs from the active test > > + * pattern mode. Otherwise, this function is a no-op. Setting the same test > > + * pattern mode for every frame thus incurs no performance penalty. > > + */ > > +int CameraSensor::setTestPatternMode(controls::draft::TestPatternModeEnum mode) > > +{ > > + if (testPatternMode_ == mode) > > + return 0; > > + > > + if (testPatternModes_.empty()) { > > + LOG(CameraSensor, Error) > > + << "Camera sensor does not support test pattern modes."; > > + return -EINVAL; > > + } > > + > > + return applyTestPatternMode(mode); > > +} > > + > > +int CameraSensor::applyTestPatternMode(controls::draft::TestPatternModeEnum mode) > > +{ > > + if (testPatternModes_.empty()) > > + return 0; > > + > > + auto it = std::find(testPatternModes_.begin(), testPatternModes_.end(), > > + mode); > > + if (it == testPatternModes_.end()) { > > + LOG(CameraSensor, Error) << "Unsupported test pattern mode " > > + << mode; > > + return -EINVAL; > > + } > > + > > + LOG(CameraSensor, Debug) << "Apply test pattern mode " << mode; > > + > > + int32_t index = staticProps_->testPatternModes.at(mode); > > + ControlList ctrls{ controls() }; > > + ctrls.set(V4L2_CID_TEST_PATTERN, index); > > + > > + int ret = setControls(&ctrls); > > + if (ret) > > + return ret; > > + > > + testPatternMode_ = mode; > > + > > + return 0; > > +} > > + > > std::string CameraSensor::logPrefix() const > > { > > return "'" + entity_->name() + "'"; > > } > > > > -int CameraSensor::generateId() > > -{ > > - const std::string devPath = subdev_->devicePath(); > > - > > - /* Try to get ID from firmware description. */ > > - id_ = sysfs::firmwareNodePath(devPath); > > - if (!id_.empty()) > > - return 0; > > - > > - /* > > - * Virtual sensors not described in firmware > > - * > > - * Verify it's a platform device and construct ID from the device path > > - * and model of sensor. > > - */ > > - if (devPath.find("/sys/devices/platform/", 0) == 0) { > > - id_ = devPath.substr(strlen("/sys/devices/")) + " " + model(); > > - return 0; > > - } > > - > > - LOG(CameraSensor, Error) << "Can't generate sensor ID"; > > - return -EINVAL; > > -} > > - > > } /* namespace libcamera */
Hi Laurent, On Fri, Mar 01, 2024 at 11:21:05PM +0200, Laurent Pinchart wrote: > The CameraSensor class has grown a lot since its creation, with many > functions added for different types of purposes. They are not grouped by > categories in the class definition, generating confusion when reading > the header file. Improve readability by sorting functions by category: > > - Getters for static data (model, entity, focus lens, ...) > - Format and sensor configuration accessors > - Properties and controls (including test pattern mode) > > Update the .cpp file accordingly. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com> Cheers, Stefan > --- > include/libcamera/internal/camera_sensor.h | 29 +- > src/libcamera/sensor/camera_sensor.cpp | 328 ++++++++++----------- > 2 files changed, 179 insertions(+), 178 deletions(-) > > diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h > index b2f077b9cc75..750d6d729cac 100644 > --- a/include/libcamera/internal/camera_sensor.h > +++ b/include/libcamera/internal/camera_sensor.h > @@ -46,15 +46,15 @@ public: > > const std::string &model() const { return model_; } > const std::string &id() const { return id_; } > + > const MediaEntity *entity() const { return entity_; } > + V4L2Subdevice *device() { return subdev_.get(); } > + > + CameraLens *focusLens() { return focusLens_.get(); } > + > const std::vector<unsigned int> &mbusCodes() const { return mbusCodes_; } > std::vector<Size> sizes(unsigned int mbusCode) const; > Size resolution() const; > - const std::vector<controls::draft::TestPatternModeEnum> &testPatternModes() const > - { > - return testPatternModes_; > - } > - int setTestPatternMode(controls::draft::TestPatternModeEnum mode); > > V4L2SubdeviceFormat getFormat(const std::vector<unsigned int> &mbusCodes, > const Size &size) const; > @@ -66,18 +66,19 @@ public: > Transform transform = Transform::Identity, > V4L2SubdeviceFormat *sensorFormat = nullptr); > > + const ControlList &properties() const { return properties_; } > + int sensorInfo(IPACameraSensorInfo *info) const; > + Transform computeTransform(Orientation *orientation) const; > + > const ControlInfoMap &controls() const; > ControlList getControls(const std::vector<uint32_t> &ids); > int setControls(ControlList *ctrls); > > - V4L2Subdevice *device() { return subdev_.get(); } > - > - const ControlList &properties() const { return properties_; } > - int sensorInfo(IPACameraSensorInfo *info) const; > - > - CameraLens *focusLens() { return focusLens_.get(); } > - > - Transform computeTransform(Orientation *orientation) const; > + const std::vector<controls::draft::TestPatternModeEnum> &testPatternModes() const > + { > + return testPatternModes_; > + } > + int setTestPatternMode(controls::draft::TestPatternModeEnum mode); > > protected: > std::string logPrefix() const override; > @@ -91,8 +92,8 @@ private: > void initStaticProperties(); > void initTestPatternModes(); > int initProperties(); > - int applyTestPatternMode(controls::draft::TestPatternModeEnum mode); > int discoverAncillaryDevices(); > + int applyTestPatternMode(controls::draft::TestPatternModeEnum mode); > > const MediaEntity *entity_; > std::unique_ptr<V4L2Subdevice> subdev_; > diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp > index 545f89d036df..86ad9a85371c 100644 > --- a/src/libcamera/sensor/camera_sensor.cpp > +++ b/src/libcamera/sensor/camera_sensor.cpp > @@ -215,6 +215,30 @@ int CameraSensor::init() > return applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff); > } > > +int CameraSensor::generateId() > +{ > + const std::string devPath = subdev_->devicePath(); > + > + /* Try to get ID from firmware description. */ > + id_ = sysfs::firmwareNodePath(devPath); > + if (!id_.empty()) > + return 0; > + > + /* > + * Virtual sensors not described in firmware > + * > + * Verify it's a platform device and construct ID from the device path > + * and model of sensor. > + */ > + if (devPath.find("/sys/devices/platform/", 0) == 0) { > + id_ = devPath.substr(strlen("/sys/devices/")) + " " + model(); > + return 0; > + } > + > + LOG(CameraSensor, Error) << "Can't generate sensor ID"; > + return -EINVAL; > +} > + > int CameraSensor::validateSensorDriver() > { > int err = 0; > @@ -580,6 +604,21 @@ int CameraSensor::discoverAncillaryDevices() > * \return The sensor media entity > */ > > +/** > + * \fn CameraSensor::device() > + * \brief Retrieve the camera sensor device > + * \todo Remove this function by integrating DelayedControl with CameraSensor > + * \return The camera sensor device > + */ > + > +/** > + * \fn CameraSensor::focusLens() > + * \brief Retrieve the focus lens controller > + * > + * \return The focus lens controller. nullptr if no focus lens controller is > + * connected to the sensor > + */ > + > /** > * \fn CameraSensor::mbusCodes() > * \brief Retrieve the media bus codes supported by the camera sensor > @@ -632,64 +671,6 @@ Size CameraSensor::resolution() const > return std::min(sizes_.back(), activeArea_.size()); > } > > -/** > - * \fn CameraSensor::testPatternModes() > - * \brief Retrieve all the supported test pattern modes of the camera sensor > - * The test pattern mode values correspond to the controls::TestPattern control. > - * > - * \return The list of test pattern modes > - */ > - > -/** > - * \brief Set the test pattern mode for the camera sensor > - * \param[in] mode The test pattern mode > - * > - * The new \a mode is applied to the sensor if it differs from the active test > - * pattern mode. Otherwise, this function is a no-op. Setting the same test > - * pattern mode for every frame thus incurs no performance penalty. > - */ > -int CameraSensor::setTestPatternMode(controls::draft::TestPatternModeEnum mode) > -{ > - if (testPatternMode_ == mode) > - return 0; > - > - if (testPatternModes_.empty()) { > - LOG(CameraSensor, Error) > - << "Camera sensor does not support test pattern modes."; > - return -EINVAL; > - } > - > - return applyTestPatternMode(mode); > -} > - > -int CameraSensor::applyTestPatternMode(controls::draft::TestPatternModeEnum mode) > -{ > - if (testPatternModes_.empty()) > - return 0; > - > - auto it = std::find(testPatternModes_.begin(), testPatternModes_.end(), > - mode); > - if (it == testPatternModes_.end()) { > - LOG(CameraSensor, Error) << "Unsupported test pattern mode " > - << mode; > - return -EINVAL; > - } > - > - LOG(CameraSensor, Debug) << "Apply test pattern mode " << mode; > - > - int32_t index = staticProps_->testPatternModes.at(mode); > - ControlList ctrls{ controls() }; > - ctrls.set(V4L2_CID_TEST_PATTERN, index); > - > - int ret = setControls(&ctrls); > - if (ret) > - return ret; > - > - testPatternMode_ = mode; > - > - return 0; > -} > - > /** > * \brief Retrieve the best sensor format for a desired output > * \param[in] mbusCodes The list of acceptable media bus codes > @@ -919,80 +900,6 @@ int CameraSensor::applyConfiguration(const SensorConfiguration &config, > return 0; > } > > -/** > - * \brief Retrieve the supported V4L2 controls and their information > - * > - * Control information is updated automatically to reflect the current sensor > - * configuration when the setFormat() function is called, without invalidating > - * any iterator on the ControlInfoMap. > - * > - * \return A map of the V4L2 controls supported by the sensor > - */ > -const ControlInfoMap &CameraSensor::controls() const > -{ > - return subdev_->controls(); > -} > - > -/** > - * \brief Read V4L2 controls from the sensor > - * \param[in] ids The list of controls to read, specified by their ID > - * > - * This function reads the value of all controls contained in \a ids, and > - * returns their values as a ControlList. The control identifiers are defined by > - * the V4L2 specification (V4L2_CID_*). > - * > - * If any control in \a ids is not supported by the device, is disabled (i.e. > - * has the V4L2_CTRL_FLAG_DISABLED flag set), or if any other error occurs > - * during validation of the requested controls, no control is read and this > - * function returns an empty control list. > - * > - * \sa V4L2Device::getControls() > - * > - * \return The control values in a ControlList on success, or an empty list on > - * error > - */ > -ControlList CameraSensor::getControls(const std::vector<uint32_t> &ids) > -{ > - return subdev_->getControls(ids); > -} > - > -/** > - * \brief Write V4L2 controls to the sensor > - * \param[in] ctrls The list of controls to write > - * > - * This function writes the value of all controls contained in \a ctrls, and > - * stores the values actually applied to the device in the corresponding \a > - * ctrls entry. The control identifiers are defined by the V4L2 specification > - * (V4L2_CID_*). > - * > - * If any control in \a ctrls is not supported by the device, is disabled (i.e. > - * has the V4L2_CTRL_FLAG_DISABLED flag set), is read-only, or if any other > - * error occurs during validation of the requested controls, no control is > - * written and this function returns -EINVAL. > - * > - * If an error occurs while writing the controls, the index of the first > - * control that couldn't be written is returned. All controls below that index > - * are written and their values are updated in \a ctrls, while all other > - * controls are not written and their values are not changed. > - * > - * \sa V4L2Device::setControls() > - * > - * \return 0 on success or an error code otherwise > - * \retval -EINVAL One of the control is not supported or not accessible > - * \retval i The index of the control that failed > - */ > -int CameraSensor::setControls(ControlList *ctrls) > -{ > - return subdev_->setControls(ctrls); > -} > - > -/** > - * \fn CameraSensor::device() > - * \brief Retrieve the camera sensor device > - * \todo Remove this function by integrating DelayedControl with CameraSensor > - * \return The camera sensor device > - */ > - > /** > * \fn CameraSensor::properties() > * \brief Retrieve the camera sensor properties > @@ -1088,14 +995,6 @@ int CameraSensor::sensorInfo(IPACameraSensorInfo *info) const > return 0; > } > > -/** > - * \fn CameraSensor::focusLens() > - * \brief Retrieve the focus lens controller > - * > - * \return The focus lens controller. nullptr if no focus lens controller is > - * connected to the sensor > - */ > - > /** > * \brief Compute the Transform that gives the requested \a orientation > * \param[inout] orientation The desired image orientation > @@ -1155,33 +1054,134 @@ Transform CameraSensor::computeTransform(Orientation *orientation) const > return transform; > } > > +/** > + * \brief Retrieve the supported V4L2 controls and their information > + * > + * Control information is updated automatically to reflect the current sensor > + * configuration when the setFormat() function is called, without invalidating > + * any iterator on the ControlInfoMap. > + * > + * \return A map of the V4L2 controls supported by the sensor > + */ > +const ControlInfoMap &CameraSensor::controls() const > +{ > + return subdev_->controls(); > +} > + > +/** > + * \brief Read V4L2 controls from the sensor > + * \param[in] ids The list of controls to read, specified by their ID > + * > + * This function reads the value of all controls contained in \a ids, and > + * returns their values as a ControlList. The control identifiers are defined by > + * the V4L2 specification (V4L2_CID_*). > + * > + * If any control in \a ids is not supported by the device, is disabled (i.e. > + * has the V4L2_CTRL_FLAG_DISABLED flag set), or if any other error occurs > + * during validation of the requested controls, no control is read and this > + * function returns an empty control list. > + * > + * \sa V4L2Device::getControls() > + * > + * \return The control values in a ControlList on success, or an empty list on > + * error > + */ > +ControlList CameraSensor::getControls(const std::vector<uint32_t> &ids) > +{ > + return subdev_->getControls(ids); > +} > + > +/** > + * \brief Write V4L2 controls to the sensor > + * \param[in] ctrls The list of controls to write > + * > + * This function writes the value of all controls contained in \a ctrls, and > + * stores the values actually applied to the device in the corresponding \a > + * ctrls entry. The control identifiers are defined by the V4L2 specification > + * (V4L2_CID_*). > + * > + * If any control in \a ctrls is not supported by the device, is disabled (i.e. > + * has the V4L2_CTRL_FLAG_DISABLED flag set), is read-only, or if any other > + * error occurs during validation of the requested controls, no control is > + * written and this function returns -EINVAL. > + * > + * If an error occurs while writing the controls, the index of the first > + * control that couldn't be written is returned. All controls below that index > + * are written and their values are updated in \a ctrls, while all other > + * controls are not written and their values are not changed. > + * > + * \sa V4L2Device::setControls() > + * > + * \return 0 on success or an error code otherwise > + * \retval -EINVAL One of the control is not supported or not accessible > + * \retval i The index of the control that failed > + */ > +int CameraSensor::setControls(ControlList *ctrls) > +{ > + return subdev_->setControls(ctrls); > +} > + > +/** > + * \fn CameraSensor::testPatternModes() > + * \brief Retrieve all the supported test pattern modes of the camera sensor > + * The test pattern mode values correspond to the controls::TestPattern control. > + * > + * \return The list of test pattern modes > + */ > + > +/** > + * \brief Set the test pattern mode for the camera sensor > + * \param[in] mode The test pattern mode > + * > + * The new \a mode is applied to the sensor if it differs from the active test > + * pattern mode. Otherwise, this function is a no-op. Setting the same test > + * pattern mode for every frame thus incurs no performance penalty. > + */ > +int CameraSensor::setTestPatternMode(controls::draft::TestPatternModeEnum mode) > +{ > + if (testPatternMode_ == mode) > + return 0; > + > + if (testPatternModes_.empty()) { > + LOG(CameraSensor, Error) > + << "Camera sensor does not support test pattern modes."; > + return -EINVAL; > + } > + > + return applyTestPatternMode(mode); > +} > + > +int CameraSensor::applyTestPatternMode(controls::draft::TestPatternModeEnum mode) > +{ > + if (testPatternModes_.empty()) > + return 0; > + > + auto it = std::find(testPatternModes_.begin(), testPatternModes_.end(), > + mode); > + if (it == testPatternModes_.end()) { > + LOG(CameraSensor, Error) << "Unsupported test pattern mode " > + << mode; > + return -EINVAL; > + } > + > + LOG(CameraSensor, Debug) << "Apply test pattern mode " << mode; > + > + int32_t index = staticProps_->testPatternModes.at(mode); > + ControlList ctrls{ controls() }; > + ctrls.set(V4L2_CID_TEST_PATTERN, index); > + > + int ret = setControls(&ctrls); > + if (ret) > + return ret; > + > + testPatternMode_ = mode; > + > + return 0; > +} > + > std::string CameraSensor::logPrefix() const > { > return "'" + entity_->name() + "'"; > } > > -int CameraSensor::generateId() > -{ > - const std::string devPath = subdev_->devicePath(); > - > - /* Try to get ID from firmware description. */ > - id_ = sysfs::firmwareNodePath(devPath); > - if (!id_.empty()) > - return 0; > - > - /* > - * Virtual sensors not described in firmware > - * > - * Verify it's a platform device and construct ID from the device path > - * and model of sensor. > - */ > - if (devPath.find("/sys/devices/platform/", 0) == 0) { > - id_ = devPath.substr(strlen("/sys/devices/")) + " " + model(); > - return 0; > - } > - > - LOG(CameraSensor, Error) << "Can't generate sensor ID"; > - return -EINVAL; > -} > - > } /* namespace libcamera */ > -- > Regards, > > Laurent Pinchart >
diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h index b2f077b9cc75..750d6d729cac 100644 --- a/include/libcamera/internal/camera_sensor.h +++ b/include/libcamera/internal/camera_sensor.h @@ -46,15 +46,15 @@ public: const std::string &model() const { return model_; } const std::string &id() const { return id_; } + const MediaEntity *entity() const { return entity_; } + V4L2Subdevice *device() { return subdev_.get(); } + + CameraLens *focusLens() { return focusLens_.get(); } + const std::vector<unsigned int> &mbusCodes() const { return mbusCodes_; } std::vector<Size> sizes(unsigned int mbusCode) const; Size resolution() const; - const std::vector<controls::draft::TestPatternModeEnum> &testPatternModes() const - { - return testPatternModes_; - } - int setTestPatternMode(controls::draft::TestPatternModeEnum mode); V4L2SubdeviceFormat getFormat(const std::vector<unsigned int> &mbusCodes, const Size &size) const; @@ -66,18 +66,19 @@ public: Transform transform = Transform::Identity, V4L2SubdeviceFormat *sensorFormat = nullptr); + const ControlList &properties() const { return properties_; } + int sensorInfo(IPACameraSensorInfo *info) const; + Transform computeTransform(Orientation *orientation) const; + const ControlInfoMap &controls() const; ControlList getControls(const std::vector<uint32_t> &ids); int setControls(ControlList *ctrls); - V4L2Subdevice *device() { return subdev_.get(); } - - const ControlList &properties() const { return properties_; } - int sensorInfo(IPACameraSensorInfo *info) const; - - CameraLens *focusLens() { return focusLens_.get(); } - - Transform computeTransform(Orientation *orientation) const; + const std::vector<controls::draft::TestPatternModeEnum> &testPatternModes() const + { + return testPatternModes_; + } + int setTestPatternMode(controls::draft::TestPatternModeEnum mode); protected: std::string logPrefix() const override; @@ -91,8 +92,8 @@ private: void initStaticProperties(); void initTestPatternModes(); int initProperties(); - int applyTestPatternMode(controls::draft::TestPatternModeEnum mode); int discoverAncillaryDevices(); + int applyTestPatternMode(controls::draft::TestPatternModeEnum mode); const MediaEntity *entity_; std::unique_ptr<V4L2Subdevice> subdev_; diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp index 545f89d036df..86ad9a85371c 100644 --- a/src/libcamera/sensor/camera_sensor.cpp +++ b/src/libcamera/sensor/camera_sensor.cpp @@ -215,6 +215,30 @@ int CameraSensor::init() return applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff); } +int CameraSensor::generateId() +{ + const std::string devPath = subdev_->devicePath(); + + /* Try to get ID from firmware description. */ + id_ = sysfs::firmwareNodePath(devPath); + if (!id_.empty()) + return 0; + + /* + * Virtual sensors not described in firmware + * + * Verify it's a platform device and construct ID from the device path + * and model of sensor. + */ + if (devPath.find("/sys/devices/platform/", 0) == 0) { + id_ = devPath.substr(strlen("/sys/devices/")) + " " + model(); + return 0; + } + + LOG(CameraSensor, Error) << "Can't generate sensor ID"; + return -EINVAL; +} + int CameraSensor::validateSensorDriver() { int err = 0; @@ -580,6 +604,21 @@ int CameraSensor::discoverAncillaryDevices() * \return The sensor media entity */ +/** + * \fn CameraSensor::device() + * \brief Retrieve the camera sensor device + * \todo Remove this function by integrating DelayedControl with CameraSensor + * \return The camera sensor device + */ + +/** + * \fn CameraSensor::focusLens() + * \brief Retrieve the focus lens controller + * + * \return The focus lens controller. nullptr if no focus lens controller is + * connected to the sensor + */ + /** * \fn CameraSensor::mbusCodes() * \brief Retrieve the media bus codes supported by the camera sensor @@ -632,64 +671,6 @@ Size CameraSensor::resolution() const return std::min(sizes_.back(), activeArea_.size()); } -/** - * \fn CameraSensor::testPatternModes() - * \brief Retrieve all the supported test pattern modes of the camera sensor - * The test pattern mode values correspond to the controls::TestPattern control. - * - * \return The list of test pattern modes - */ - -/** - * \brief Set the test pattern mode for the camera sensor - * \param[in] mode The test pattern mode - * - * The new \a mode is applied to the sensor if it differs from the active test - * pattern mode. Otherwise, this function is a no-op. Setting the same test - * pattern mode for every frame thus incurs no performance penalty. - */ -int CameraSensor::setTestPatternMode(controls::draft::TestPatternModeEnum mode) -{ - if (testPatternMode_ == mode) - return 0; - - if (testPatternModes_.empty()) { - LOG(CameraSensor, Error) - << "Camera sensor does not support test pattern modes."; - return -EINVAL; - } - - return applyTestPatternMode(mode); -} - -int CameraSensor::applyTestPatternMode(controls::draft::TestPatternModeEnum mode) -{ - if (testPatternModes_.empty()) - return 0; - - auto it = std::find(testPatternModes_.begin(), testPatternModes_.end(), - mode); - if (it == testPatternModes_.end()) { - LOG(CameraSensor, Error) << "Unsupported test pattern mode " - << mode; - return -EINVAL; - } - - LOG(CameraSensor, Debug) << "Apply test pattern mode " << mode; - - int32_t index = staticProps_->testPatternModes.at(mode); - ControlList ctrls{ controls() }; - ctrls.set(V4L2_CID_TEST_PATTERN, index); - - int ret = setControls(&ctrls); - if (ret) - return ret; - - testPatternMode_ = mode; - - return 0; -} - /** * \brief Retrieve the best sensor format for a desired output * \param[in] mbusCodes The list of acceptable media bus codes @@ -919,80 +900,6 @@ int CameraSensor::applyConfiguration(const SensorConfiguration &config, return 0; } -/** - * \brief Retrieve the supported V4L2 controls and their information - * - * Control information is updated automatically to reflect the current sensor - * configuration when the setFormat() function is called, without invalidating - * any iterator on the ControlInfoMap. - * - * \return A map of the V4L2 controls supported by the sensor - */ -const ControlInfoMap &CameraSensor::controls() const -{ - return subdev_->controls(); -} - -/** - * \brief Read V4L2 controls from the sensor - * \param[in] ids The list of controls to read, specified by their ID - * - * This function reads the value of all controls contained in \a ids, and - * returns their values as a ControlList. The control identifiers are defined by - * the V4L2 specification (V4L2_CID_*). - * - * If any control in \a ids is not supported by the device, is disabled (i.e. - * has the V4L2_CTRL_FLAG_DISABLED flag set), or if any other error occurs - * during validation of the requested controls, no control is read and this - * function returns an empty control list. - * - * \sa V4L2Device::getControls() - * - * \return The control values in a ControlList on success, or an empty list on - * error - */ -ControlList CameraSensor::getControls(const std::vector<uint32_t> &ids) -{ - return subdev_->getControls(ids); -} - -/** - * \brief Write V4L2 controls to the sensor - * \param[in] ctrls The list of controls to write - * - * This function writes the value of all controls contained in \a ctrls, and - * stores the values actually applied to the device in the corresponding \a - * ctrls entry. The control identifiers are defined by the V4L2 specification - * (V4L2_CID_*). - * - * If any control in \a ctrls is not supported by the device, is disabled (i.e. - * has the V4L2_CTRL_FLAG_DISABLED flag set), is read-only, or if any other - * error occurs during validation of the requested controls, no control is - * written and this function returns -EINVAL. - * - * If an error occurs while writing the controls, the index of the first - * control that couldn't be written is returned. All controls below that index - * are written and their values are updated in \a ctrls, while all other - * controls are not written and their values are not changed. - * - * \sa V4L2Device::setControls() - * - * \return 0 on success or an error code otherwise - * \retval -EINVAL One of the control is not supported or not accessible - * \retval i The index of the control that failed - */ -int CameraSensor::setControls(ControlList *ctrls) -{ - return subdev_->setControls(ctrls); -} - -/** - * \fn CameraSensor::device() - * \brief Retrieve the camera sensor device - * \todo Remove this function by integrating DelayedControl with CameraSensor - * \return The camera sensor device - */ - /** * \fn CameraSensor::properties() * \brief Retrieve the camera sensor properties @@ -1088,14 +995,6 @@ int CameraSensor::sensorInfo(IPACameraSensorInfo *info) const return 0; } -/** - * \fn CameraSensor::focusLens() - * \brief Retrieve the focus lens controller - * - * \return The focus lens controller. nullptr if no focus lens controller is - * connected to the sensor - */ - /** * \brief Compute the Transform that gives the requested \a orientation * \param[inout] orientation The desired image orientation @@ -1155,33 +1054,134 @@ Transform CameraSensor::computeTransform(Orientation *orientation) const return transform; } +/** + * \brief Retrieve the supported V4L2 controls and their information + * + * Control information is updated automatically to reflect the current sensor + * configuration when the setFormat() function is called, without invalidating + * any iterator on the ControlInfoMap. + * + * \return A map of the V4L2 controls supported by the sensor + */ +const ControlInfoMap &CameraSensor::controls() const +{ + return subdev_->controls(); +} + +/** + * \brief Read V4L2 controls from the sensor + * \param[in] ids The list of controls to read, specified by their ID + * + * This function reads the value of all controls contained in \a ids, and + * returns their values as a ControlList. The control identifiers are defined by + * the V4L2 specification (V4L2_CID_*). + * + * If any control in \a ids is not supported by the device, is disabled (i.e. + * has the V4L2_CTRL_FLAG_DISABLED flag set), or if any other error occurs + * during validation of the requested controls, no control is read and this + * function returns an empty control list. + * + * \sa V4L2Device::getControls() + * + * \return The control values in a ControlList on success, or an empty list on + * error + */ +ControlList CameraSensor::getControls(const std::vector<uint32_t> &ids) +{ + return subdev_->getControls(ids); +} + +/** + * \brief Write V4L2 controls to the sensor + * \param[in] ctrls The list of controls to write + * + * This function writes the value of all controls contained in \a ctrls, and + * stores the values actually applied to the device in the corresponding \a + * ctrls entry. The control identifiers are defined by the V4L2 specification + * (V4L2_CID_*). + * + * If any control in \a ctrls is not supported by the device, is disabled (i.e. + * has the V4L2_CTRL_FLAG_DISABLED flag set), is read-only, or if any other + * error occurs during validation of the requested controls, no control is + * written and this function returns -EINVAL. + * + * If an error occurs while writing the controls, the index of the first + * control that couldn't be written is returned. All controls below that index + * are written and their values are updated in \a ctrls, while all other + * controls are not written and their values are not changed. + * + * \sa V4L2Device::setControls() + * + * \return 0 on success or an error code otherwise + * \retval -EINVAL One of the control is not supported or not accessible + * \retval i The index of the control that failed + */ +int CameraSensor::setControls(ControlList *ctrls) +{ + return subdev_->setControls(ctrls); +} + +/** + * \fn CameraSensor::testPatternModes() + * \brief Retrieve all the supported test pattern modes of the camera sensor + * The test pattern mode values correspond to the controls::TestPattern control. + * + * \return The list of test pattern modes + */ + +/** + * \brief Set the test pattern mode for the camera sensor + * \param[in] mode The test pattern mode + * + * The new \a mode is applied to the sensor if it differs from the active test + * pattern mode. Otherwise, this function is a no-op. Setting the same test + * pattern mode for every frame thus incurs no performance penalty. + */ +int CameraSensor::setTestPatternMode(controls::draft::TestPatternModeEnum mode) +{ + if (testPatternMode_ == mode) + return 0; + + if (testPatternModes_.empty()) { + LOG(CameraSensor, Error) + << "Camera sensor does not support test pattern modes."; + return -EINVAL; + } + + return applyTestPatternMode(mode); +} + +int CameraSensor::applyTestPatternMode(controls::draft::TestPatternModeEnum mode) +{ + if (testPatternModes_.empty()) + return 0; + + auto it = std::find(testPatternModes_.begin(), testPatternModes_.end(), + mode); + if (it == testPatternModes_.end()) { + LOG(CameraSensor, Error) << "Unsupported test pattern mode " + << mode; + return -EINVAL; + } + + LOG(CameraSensor, Debug) << "Apply test pattern mode " << mode; + + int32_t index = staticProps_->testPatternModes.at(mode); + ControlList ctrls{ controls() }; + ctrls.set(V4L2_CID_TEST_PATTERN, index); + + int ret = setControls(&ctrls); + if (ret) + return ret; + + testPatternMode_ = mode; + + return 0; +} + std::string CameraSensor::logPrefix() const { return "'" + entity_->name() + "'"; } -int CameraSensor::generateId() -{ - const std::string devPath = subdev_->devicePath(); - - /* Try to get ID from firmware description. */ - id_ = sysfs::firmwareNodePath(devPath); - if (!id_.empty()) - return 0; - - /* - * Virtual sensors not described in firmware - * - * Verify it's a platform device and construct ID from the device path - * and model of sensor. - */ - if (devPath.find("/sys/devices/platform/", 0) == 0) { - id_ = devPath.substr(strlen("/sys/devices/")) + " " + model(); - return 0; - } - - LOG(CameraSensor, Error) << "Can't generate sensor ID"; - return -EINVAL; -} - } /* namespace libcamera */
The CameraSensor class has grown a lot since its creation, with many functions added for different types of purposes. They are not grouped by categories in the class definition, generating confusion when reading the header file. Improve readability by sorting functions by category: - Getters for static data (model, entity, focus lens, ...) - Format and sensor configuration accessors - Properties and controls (including test pattern mode) Update the .cpp file accordingly. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- include/libcamera/internal/camera_sensor.h | 29 +- src/libcamera/sensor/camera_sensor.cpp | 328 ++++++++++----------- 2 files changed, 179 insertions(+), 178 deletions(-)