| Message ID | 20191218145001.22283-4-jacopo@jmondi.org |
|---|---|
| State | Superseded |
| Delegated to: | Jacopo Mondi |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, Thanks for your patch. On 2019-12-18 15:49:57 +0100, Jacopo Mondi wrote: > Factorize CameraSensor properties initialization to a dedicated virtual Factorize don't seem to match the context :-) I would here and in the subject s/Factorize/Refactor/ > function that dervied classes could subclass to register sensor-specific > properties. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > src/libcamera/camera_sensor.cpp | 94 ++++++++++++++++----------- > src/libcamera/include/camera_sensor.h | 5 +- > 2 files changed, 60 insertions(+), 39 deletions(-) > > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp > index d1c9c9bcd58f..9692895d9b7b 100644 > --- a/src/libcamera/camera_sensor.cpp > +++ b/src/libcamera/camera_sensor.cpp > @@ -77,7 +77,7 @@ CameraSensor *CameraSensorFactory::create(const MediaEntity *entity) > * Once constructed the instance must be initialized with init(). > */ > CameraSensor::CameraSensor(const MediaEntity *entity) > - : entity_(entity), properties_(properties::properties) > + : properties_(properties::properties), entity_(entity) > { > subdev_ = new V4L2Subdevice(entity); > } > @@ -119,42 +119,6 @@ int CameraSensor::init() > if (ret < 0) > return ret; > > - /* Retrieve and store the camera sensor properties. */ > - const ControlInfoMap &controls = subdev_->controls(); > - int32_t propertyValue; > - > - /* Camera Location: default is front location. */ > - const auto &locationControl = controls.find(V4L2_CID_CAMERA_SENSOR_LOCATION); > - if (locationControl != controls.end()) { > - int32_t v4l2Location = > - locationControl->second.def().get<int32_t>(); > - switch (v4l2Location) { > - case V4L2_LOCATION_EXTERNAL: > - propertyValue = properties::CameraLocationExternal; > - break; > - case V4L2_LOCATION_FRONT: > - propertyValue = properties::CameraLocationFront; > - break; > - case V4L2_LOCATION_BACK: > - propertyValue = properties::CameraLocationBack; > - break; > - default: > - LOG(CameraSensor, Error) > - << "Unsupported camera location: " << v4l2Location; > - return -EINVAL; > - } > - } else { > - propertyValue = properties::CameraLocationFront; > - } > - properties_.set(properties::Location, propertyValue); > - > - /* Camera Rotation: default is 0 degrees. */ > - propertyValue = 0; > - const auto &rotationControl = controls.find(V4L2_CID_CAMERA_SENSOR_ROTATION); > - if (rotationControl != controls.end()) > - propertyValue = rotationControl->second.def().get<int32_t>(); > - properties_.set(properties::Rotation, propertyValue); > - I Wonder if we should not refactor this into three functions, one virtual initProperties() as done already but go even further and add a two non-virtual functions initDefaultLocationProperty() and initDefaultRotationProperty(). The refactored initProperties() would then call the two new functions and allow subclasses to reuse a standard way to parse common controls/properties. > /* Enumerate and cache media bus codes and sizes. */ > const ImageFormats formats = subdev_->formats(0); > if (formats.isEmpty()) { > @@ -185,6 +149,62 @@ int CameraSensor::init() > std::sort(mbusCodes_.begin(), mbusCodes_.end()); > std::sort(sizes_.begin(), sizes_.end()); > > + return initProperties(subdev_->controls()); > +} > + > +/** > + * \brief Initialize the camera sensor properties > + * \param[in] controlMap The map of control information provided by the sensor > + * > + * This method initializes the camera sensor properties, by inspecting the > + * control information reported by the sensor media entity in \a controlMap. > + * For each supported standard V4L2 control reported by the sensor, a libcamera > + * property is created and registered in the list of properties supported by the > + * sensor. > + * > + * Derived classes are free to override this method to register sensor specific > + * properties as they like, by inspecting custom controls or by adding > + * properties with pre-defined values, and eventually call this base class > + * implementation to register standard ones. > + * > + * \return 0 on success, a negative error code otherwise > + */ > +int CameraSensor::initProperties(const ControlInfoMap &controlMap) > +{ > + int32_t propertyValue; > + > + /* Camera Location: default is front location. */ > + const auto &locationControl = controlMap.find(V4L2_CID_CAMERA_SENSOR_LOCATION); > + if (locationControl != controlMap.end()) { > + int32_t v4l2Location = > + locationControl->second.def().get<int32_t>(); > + switch (v4l2Location) { > + case V4L2_LOCATION_EXTERNAL: > + propertyValue = properties::CameraLocationExternal; > + break; > + case V4L2_LOCATION_FRONT: > + propertyValue = properties::CameraLocationFront; > + break; > + case V4L2_LOCATION_BACK: > + propertyValue = properties::CameraLocationBack; > + break; > + default: > + LOG(CameraSensor, Error) > + << "Unsupported camera location: " << v4l2Location; > + return -EINVAL; > + } > + } else { > + propertyValue = properties::CameraLocationFront; > + } > + properties_.set(properties::Location, propertyValue); > + > + /* Camera Rotation: default is 0 degrees. */ > + propertyValue = 0; > + const auto &rotationControl = controlMap.find(V4L2_CID_CAMERA_SENSOR_ROTATION); > + if (rotationControl != controlMap.end()) > + propertyValue = rotationControl->second.def().get<int32_t>(); > + properties_.set(properties::Rotation, propertyValue); > + > return 0; > } > > diff --git a/src/libcamera/include/camera_sensor.h b/src/libcamera/include/camera_sensor.h > index 2c09b1bdb61b..43748a6c8535 100644 > --- a/src/libcamera/include/camera_sensor.h > +++ b/src/libcamera/include/camera_sensor.h > @@ -38,6 +38,7 @@ public: > CameraSensor &operator=(const CameraSensor &) = delete; > > int init(); > + virtual int initProperties(const ControlInfoMap &controlMap); > > const MediaEntity *entity() const { return entity_; } > const std::vector<unsigned int> &mbusCodes() const { return mbusCodes_; } > @@ -55,6 +56,8 @@ public: > const ControlList &properties() const { return properties_; } > > protected: > + ControlList properties_; > + > friend class CameraSensorFactory; > explicit CameraSensor(const MediaEntity *entity); > std::string logPrefix() const; > @@ -65,8 +68,6 @@ private: > > std::vector<unsigned int> mbusCodes_; > std::vector<Size> sizes_; > - > - ControlList properties_; > }; > > } /* namespace libcamera */ > -- > 2.24.0 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Niklas, On Wed, Jan 15, 2020 at 09:03:21PM +0100, Niklas Söderlund wrote: > Hi Jacopo, > > Thanks for your patch. > > On 2019-12-18 15:49:57 +0100, Jacopo Mondi wrote: > > Factorize CameraSensor properties initialization to a dedicated virtual > > Factorize don't seem to match the context :-) > > I would here and in the subject s/Factorize/Refactor/ > > > function that dervied classes could subclass to register sensor-specific > > properties. > > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > --- > > src/libcamera/camera_sensor.cpp | 94 ++++++++++++++++----------- > > src/libcamera/include/camera_sensor.h | 5 +- > > 2 files changed, 60 insertions(+), 39 deletions(-) > > > > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp > > index d1c9c9bcd58f..9692895d9b7b 100644 > > --- a/src/libcamera/camera_sensor.cpp > > +++ b/src/libcamera/camera_sensor.cpp > > @@ -77,7 +77,7 @@ CameraSensor *CameraSensorFactory::create(const MediaEntity *entity) > > * Once constructed the instance must be initialized with init(). > > */ > > CameraSensor::CameraSensor(const MediaEntity *entity) > > - : entity_(entity), properties_(properties::properties) > > + : properties_(properties::properties), entity_(entity) > > { > > subdev_ = new V4L2Subdevice(entity); > > } > > @@ -119,42 +119,6 @@ int CameraSensor::init() > > if (ret < 0) > > return ret; > > > > - /* Retrieve and store the camera sensor properties. */ > > - const ControlInfoMap &controls = subdev_->controls(); > > - int32_t propertyValue; > > - > > - /* Camera Location: default is front location. */ > > - const auto &locationControl = controls.find(V4L2_CID_CAMERA_SENSOR_LOCATION); > > - if (locationControl != controls.end()) { > > - int32_t v4l2Location = > > - locationControl->second.def().get<int32_t>(); > > - switch (v4l2Location) { > > - case V4L2_LOCATION_EXTERNAL: > > - propertyValue = properties::CameraLocationExternal; > > - break; > > - case V4L2_LOCATION_FRONT: > > - propertyValue = properties::CameraLocationFront; > > - break; > > - case V4L2_LOCATION_BACK: > > - propertyValue = properties::CameraLocationBack; > > - break; > > - default: > > - LOG(CameraSensor, Error) > > - << "Unsupported camera location: " << v4l2Location; > > - return -EINVAL; > > - } > > - } else { > > - propertyValue = properties::CameraLocationFront; > > - } > > - properties_.set(properties::Location, propertyValue); > > - > > - /* Camera Rotation: default is 0 degrees. */ > > - propertyValue = 0; > > - const auto &rotationControl = controls.find(V4L2_CID_CAMERA_SENSOR_ROTATION); > > - if (rotationControl != controls.end()) > > - propertyValue = rotationControl->second.def().get<int32_t>(); > > - properties_.set(properties::Rotation, propertyValue); > > - > > I Wonder if we should not refactor this into three functions, one > virtual initProperties() as done already but go even further and add a > two non-virtual functions initDefaultLocationProperty() and > initDefaultRotationProperty(). > > The refactored initProperties() would then call the two new functions > and allow subclasses to reuse a standard way to parse common > controls/properties. > Would yousplit this up to the point of having one helper per property ? I'm concerned about code reuse as well, and my idea was that sub-classes would call the base class CameraSensor::initProperties() as documented.... > > /* Enumerate and cache media bus codes and sizes. */ > > const ImageFormats formats = subdev_->formats(0); > > if (formats.isEmpty()) { > > @@ -185,6 +149,62 @@ int CameraSensor::init() > > std::sort(mbusCodes_.begin(), mbusCodes_.end()); > > std::sort(sizes_.begin(), sizes_.end()); > > > > + return initProperties(subdev_->controls()); > > +} > > + > > +/** > > + * \brief Initialize the camera sensor properties > > + * \param[in] controlMap The map of control information provided by the sensor > > + * > > + * This method initializes the camera sensor properties, by inspecting the > > + * control information reported by the sensor media entity in \a controlMap. > > + * For each supported standard V4L2 control reported by the sensor, a libcamera > > + * property is created and registered in the list of properties supported by the > > + * sensor. > > + * > > + * Derived classes are free to override this method to register sensor specific > > + * properties as they like, by inspecting custom controls or by adding > > + * properties with pre-defined values, and eventually call this base class > > + * implementation to register standard ones. ... here to register the default ones. Would you really split it to one per property ? I would rather instrument the base class initProperty to register all default ones, except the ones already overridden by the derived class. Thanks j > > + * > > + * \return 0 on success, a negative error code otherwise > > + */ > > +int CameraSensor::initProperties(const ControlInfoMap &controlMap) > > +{ > > + int32_t propertyValue; > > + > > + /* Camera Location: default is front location. */ > > + const auto &locationControl = controlMap.find(V4L2_CID_CAMERA_SENSOR_LOCATION); > > + if (locationControl != controlMap.end()) { > > + int32_t v4l2Location = > > + locationControl->second.def().get<int32_t>(); > > + switch (v4l2Location) { > > + case V4L2_LOCATION_EXTERNAL: > > + propertyValue = properties::CameraLocationExternal; > > + break; > > + case V4L2_LOCATION_FRONT: > > + propertyValue = properties::CameraLocationFront; > > + break; > > + case V4L2_LOCATION_BACK: > > + propertyValue = properties::CameraLocationBack; > > + break; > > + default: > > + LOG(CameraSensor, Error) > > + << "Unsupported camera location: " << v4l2Location; > > + return -EINVAL; > > + } > > + } else { > > + propertyValue = properties::CameraLocationFront; > > + } > > + properties_.set(properties::Location, propertyValue); > > + > > + /* Camera Rotation: default is 0 degrees. */ > > + propertyValue = 0; > > + const auto &rotationControl = controlMap.find(V4L2_CID_CAMERA_SENSOR_ROTATION); > > + if (rotationControl != controlMap.end()) > > + propertyValue = rotationControl->second.def().get<int32_t>(); > > + properties_.set(properties::Rotation, propertyValue); > > + > > return 0; > > } > > > > diff --git a/src/libcamera/include/camera_sensor.h b/src/libcamera/include/camera_sensor.h > > index 2c09b1bdb61b..43748a6c8535 100644 > > --- a/src/libcamera/include/camera_sensor.h > > +++ b/src/libcamera/include/camera_sensor.h > > @@ -38,6 +38,7 @@ public: > > CameraSensor &operator=(const CameraSensor &) = delete; > > > > int init(); > > + virtual int initProperties(const ControlInfoMap &controlMap); > > > > const MediaEntity *entity() const { return entity_; } > > const std::vector<unsigned int> &mbusCodes() const { return mbusCodes_; } > > @@ -55,6 +56,8 @@ public: > > const ControlList &properties() const { return properties_; } > > > > protected: > > + ControlList properties_; > > + > > friend class CameraSensorFactory; > > explicit CameraSensor(const MediaEntity *entity); > > std::string logPrefix() const; > > @@ -65,8 +68,6 @@ private: > > > > std::vector<unsigned int> mbusCodes_; > > std::vector<Size> sizes_; > > - > > - ControlList properties_; > > }; > > > > } /* namespace libcamera */ > > -- > > 2.24.0 > > > > _______________________________________________ > > libcamera-devel mailing list > > libcamera-devel@lists.libcamera.org > > https://lists.libcamera.org/listinfo/libcamera-devel > > -- > Regards, > Niklas Söderlund
Hi Jacopo, On 2020-01-20 16:55:55 +0100, Jacopo Mondi wrote: > Hi Niklas, > > On Wed, Jan 15, 2020 at 09:03:21PM +0100, Niklas Söderlund wrote: > > Hi Jacopo, > > > > Thanks for your patch. > > > > On 2019-12-18 15:49:57 +0100, Jacopo Mondi wrote: > > > Factorize CameraSensor properties initialization to a dedicated virtual > > > > Factorize don't seem to match the context :-) > > > > I would here and in the subject s/Factorize/Refactor/ > > > > > function that dervied classes could subclass to register sensor-specific > > > properties. > > > > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > > --- > > > src/libcamera/camera_sensor.cpp | 94 ++++++++++++++++----------- > > > src/libcamera/include/camera_sensor.h | 5 +- > > > 2 files changed, 60 insertions(+), 39 deletions(-) > > > > > > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp > > > index d1c9c9bcd58f..9692895d9b7b 100644 > > > --- a/src/libcamera/camera_sensor.cpp > > > +++ b/src/libcamera/camera_sensor.cpp > > > @@ -77,7 +77,7 @@ CameraSensor *CameraSensorFactory::create(const MediaEntity *entity) > > > * Once constructed the instance must be initialized with init(). > > > */ > > > CameraSensor::CameraSensor(const MediaEntity *entity) > > > - : entity_(entity), properties_(properties::properties) > > > + : properties_(properties::properties), entity_(entity) > > > { > > > subdev_ = new V4L2Subdevice(entity); > > > } > > > @@ -119,42 +119,6 @@ int CameraSensor::init() > > > if (ret < 0) > > > return ret; > > > > > > - /* Retrieve and store the camera sensor properties. */ > > > - const ControlInfoMap &controls = subdev_->controls(); > > > - int32_t propertyValue; > > > - > > > - /* Camera Location: default is front location. */ > > > - const auto &locationControl = controls.find(V4L2_CID_CAMERA_SENSOR_LOCATION); > > > - if (locationControl != controls.end()) { > > > - int32_t v4l2Location = > > > - locationControl->second.def().get<int32_t>(); > > > - switch (v4l2Location) { > > > - case V4L2_LOCATION_EXTERNAL: > > > - propertyValue = properties::CameraLocationExternal; > > > - break; > > > - case V4L2_LOCATION_FRONT: > > > - propertyValue = properties::CameraLocationFront; > > > - break; > > > - case V4L2_LOCATION_BACK: > > > - propertyValue = properties::CameraLocationBack; > > > - break; > > > - default: > > > - LOG(CameraSensor, Error) > > > - << "Unsupported camera location: " << v4l2Location; > > > - return -EINVAL; > > > - } > > > - } else { > > > - propertyValue = properties::CameraLocationFront; > > > - } > > > - properties_.set(properties::Location, propertyValue); > > > - > > > - /* Camera Rotation: default is 0 degrees. */ > > > - propertyValue = 0; > > > - const auto &rotationControl = controls.find(V4L2_CID_CAMERA_SENSOR_ROTATION); > > > - if (rotationControl != controls.end()) > > > - propertyValue = rotationControl->second.def().get<int32_t>(); > > > - properties_.set(properties::Rotation, propertyValue); > > > - > > > > I Wonder if we should not refactor this into three functions, one > > virtual initProperties() as done already but go even further and add a > > two non-virtual functions initDefaultLocationProperty() and > > initDefaultRotationProperty(). > > > > The refactored initProperties() would then call the two new functions > > and allow subclasses to reuse a standard way to parse common > > controls/properties. > > > > Would yousplit this up to the point of having one helper per property ? Maybe or perhaps group them so maybe location and rotation could be parsed by the same helper. > > I'm concerned about code reuse as well, and my idea was that > sub-classes would call the base class CameraSensor::initProperties() > as documented.... > > > > /* Enumerate and cache media bus codes and sizes. */ > > > const ImageFormats formats = subdev_->formats(0); > > > if (formats.isEmpty()) { > > > @@ -185,6 +149,62 @@ int CameraSensor::init() > > > std::sort(mbusCodes_.begin(), mbusCodes_.end()); > > > std::sort(sizes_.begin(), sizes_.end()); > > > > > > + return initProperties(subdev_->controls()); > > > +} > > > + > > > +/** > > > + * \brief Initialize the camera sensor properties > > > + * \param[in] controlMap The map of control information provided by the sensor > > > + * > > > + * This method initializes the camera sensor properties, by inspecting the > > > + * control information reported by the sensor media entity in \a controlMap. > > > + * For each supported standard V4L2 control reported by the sensor, a libcamera > > > + * property is created and registered in the list of properties supported by the > > > + * sensor. > > > + * > > > + * Derived classes are free to override this method to register sensor specific > > > + * properties as they like, by inspecting custom controls or by adding > > > + * properties with pre-defined values, and eventually call this base class > > > + * implementation to register standard ones. > > ... here to register the default ones. Would you really split it to one per > property ? I would rather instrument the base class initProperty to > register all default ones, except the ones already overridden by the > derived class. I'm not a big fan of the design where the subclass shall overload the base and then call the base function it overloads. For me it's confusing to get a good overview of the logic and it can create ambiguity in implementations. Different subclasses can call the base implementation first, last or somewhere in-between which can limit what later can be added to the base. Sometimes the design is unavoidable but is this such a case? > > Thanks > j > > > > + * > > > + * \return 0 on success, a negative error code otherwise > > > + */ > > > +int CameraSensor::initProperties(const ControlInfoMap &controlMap) > > > +{ > > > + int32_t propertyValue; > > > + > > > + /* Camera Location: default is front location. */ > > > + const auto &locationControl = controlMap.find(V4L2_CID_CAMERA_SENSOR_LOCATION); > > > + if (locationControl != controlMap.end()) { > > > + int32_t v4l2Location = > > > + locationControl->second.def().get<int32_t>(); > > > + switch (v4l2Location) { > > > + case V4L2_LOCATION_EXTERNAL: > > > + propertyValue = properties::CameraLocationExternal; > > > + break; > > > + case V4L2_LOCATION_FRONT: > > > + propertyValue = properties::CameraLocationFront; > > > + break; > > > + case V4L2_LOCATION_BACK: > > > + propertyValue = properties::CameraLocationBack; > > > + break; > > > + default: > > > + LOG(CameraSensor, Error) > > > + << "Unsupported camera location: " << v4l2Location; > > > + return -EINVAL; > > > + } > > > + } else { > > > + propertyValue = properties::CameraLocationFront; > > > + } > > > + properties_.set(properties::Location, propertyValue); > > > + > > > + /* Camera Rotation: default is 0 degrees. */ > > > + propertyValue = 0; > > > + const auto &rotationControl = controlMap.find(V4L2_CID_CAMERA_SENSOR_ROTATION); > > > + if (rotationControl != controlMap.end()) > > > + propertyValue = rotationControl->second.def().get<int32_t>(); > > > + properties_.set(properties::Rotation, propertyValue); > > > + > > > return 0; > > > } > > > > > > diff --git a/src/libcamera/include/camera_sensor.h b/src/libcamera/include/camera_sensor.h > > > index 2c09b1bdb61b..43748a6c8535 100644 > > > --- a/src/libcamera/include/camera_sensor.h > > > +++ b/src/libcamera/include/camera_sensor.h > > > @@ -38,6 +38,7 @@ public: > > > CameraSensor &operator=(const CameraSensor &) = delete; > > > > > > int init(); > > > + virtual int initProperties(const ControlInfoMap &controlMap); > > > > > > const MediaEntity *entity() const { return entity_; } > > > const std::vector<unsigned int> &mbusCodes() const { return mbusCodes_; } > > > @@ -55,6 +56,8 @@ public: > > > const ControlList &properties() const { return properties_; } > > > > > > protected: > > > + ControlList properties_; > > > + > > > friend class CameraSensorFactory; > > > explicit CameraSensor(const MediaEntity *entity); > > > std::string logPrefix() const; > > > @@ -65,8 +68,6 @@ private: > > > > > > std::vector<unsigned int> mbusCodes_; > > > std::vector<Size> sizes_; > > > - > > > - ControlList properties_; > > > }; > > > > > > } /* namespace libcamera */ > > > -- > > > 2.24.0 > > > > > > _______________________________________________ > > > libcamera-devel mailing list > > > libcamera-devel@lists.libcamera.org > > > https://lists.libcamera.org/listinfo/libcamera-devel > > > > -- > > Regards, > > Niklas Söderlund
diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp index d1c9c9bcd58f..9692895d9b7b 100644 --- a/src/libcamera/camera_sensor.cpp +++ b/src/libcamera/camera_sensor.cpp @@ -77,7 +77,7 @@ CameraSensor *CameraSensorFactory::create(const MediaEntity *entity) * Once constructed the instance must be initialized with init(). */ CameraSensor::CameraSensor(const MediaEntity *entity) - : entity_(entity), properties_(properties::properties) + : properties_(properties::properties), entity_(entity) { subdev_ = new V4L2Subdevice(entity); } @@ -119,42 +119,6 @@ int CameraSensor::init() if (ret < 0) return ret; - /* Retrieve and store the camera sensor properties. */ - const ControlInfoMap &controls = subdev_->controls(); - int32_t propertyValue; - - /* Camera Location: default is front location. */ - const auto &locationControl = controls.find(V4L2_CID_CAMERA_SENSOR_LOCATION); - if (locationControl != controls.end()) { - int32_t v4l2Location = - locationControl->second.def().get<int32_t>(); - switch (v4l2Location) { - case V4L2_LOCATION_EXTERNAL: - propertyValue = properties::CameraLocationExternal; - break; - case V4L2_LOCATION_FRONT: - propertyValue = properties::CameraLocationFront; - break; - case V4L2_LOCATION_BACK: - propertyValue = properties::CameraLocationBack; - break; - default: - LOG(CameraSensor, Error) - << "Unsupported camera location: " << v4l2Location; - return -EINVAL; - } - } else { - propertyValue = properties::CameraLocationFront; - } - properties_.set(properties::Location, propertyValue); - - /* Camera Rotation: default is 0 degrees. */ - propertyValue = 0; - const auto &rotationControl = controls.find(V4L2_CID_CAMERA_SENSOR_ROTATION); - if (rotationControl != controls.end()) - propertyValue = rotationControl->second.def().get<int32_t>(); - properties_.set(properties::Rotation, propertyValue); - /* Enumerate and cache media bus codes and sizes. */ const ImageFormats formats = subdev_->formats(0); if (formats.isEmpty()) { @@ -185,6 +149,62 @@ int CameraSensor::init() std::sort(mbusCodes_.begin(), mbusCodes_.end()); std::sort(sizes_.begin(), sizes_.end()); + return initProperties(subdev_->controls()); +} + +/** + * \brief Initialize the camera sensor properties + * \param[in] controlMap The map of control information provided by the sensor + * + * This method initializes the camera sensor properties, by inspecting the + * control information reported by the sensor media entity in \a controlMap. + * For each supported standard V4L2 control reported by the sensor, a libcamera + * property is created and registered in the list of properties supported by the + * sensor. + * + * Derived classes are free to override this method to register sensor specific + * properties as they like, by inspecting custom controls or by adding + * properties with pre-defined values, and eventually call this base class + * implementation to register standard ones. + * + * \return 0 on success, a negative error code otherwise + */ +int CameraSensor::initProperties(const ControlInfoMap &controlMap) +{ + int32_t propertyValue; + + /* Camera Location: default is front location. */ + const auto &locationControl = controlMap.find(V4L2_CID_CAMERA_SENSOR_LOCATION); + if (locationControl != controlMap.end()) { + int32_t v4l2Location = + locationControl->second.def().get<int32_t>(); + switch (v4l2Location) { + case V4L2_LOCATION_EXTERNAL: + propertyValue = properties::CameraLocationExternal; + break; + case V4L2_LOCATION_FRONT: + propertyValue = properties::CameraLocationFront; + break; + case V4L2_LOCATION_BACK: + propertyValue = properties::CameraLocationBack; + break; + default: + LOG(CameraSensor, Error) + << "Unsupported camera location: " << v4l2Location; + return -EINVAL; + } + } else { + propertyValue = properties::CameraLocationFront; + } + properties_.set(properties::Location, propertyValue); + + /* Camera Rotation: default is 0 degrees. */ + propertyValue = 0; + const auto &rotationControl = controlMap.find(V4L2_CID_CAMERA_SENSOR_ROTATION); + if (rotationControl != controlMap.end()) + propertyValue = rotationControl->second.def().get<int32_t>(); + properties_.set(properties::Rotation, propertyValue); + return 0; } diff --git a/src/libcamera/include/camera_sensor.h b/src/libcamera/include/camera_sensor.h index 2c09b1bdb61b..43748a6c8535 100644 --- a/src/libcamera/include/camera_sensor.h +++ b/src/libcamera/include/camera_sensor.h @@ -38,6 +38,7 @@ public: CameraSensor &operator=(const CameraSensor &) = delete; int init(); + virtual int initProperties(const ControlInfoMap &controlMap); const MediaEntity *entity() const { return entity_; } const std::vector<unsigned int> &mbusCodes() const { return mbusCodes_; } @@ -55,6 +56,8 @@ public: const ControlList &properties() const { return properties_; } protected: + ControlList properties_; + friend class CameraSensorFactory; explicit CameraSensor(const MediaEntity *entity); std::string logPrefix() const; @@ -65,8 +68,6 @@ private: std::vector<unsigned int> mbusCodes_; std::vector<Size> sizes_; - - ControlList properties_; }; } /* namespace libcamera */
Factorize CameraSensor properties initialization to a dedicated virtual function that dervied classes could subclass to register sensor-specific properties. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- src/libcamera/camera_sensor.cpp | 94 ++++++++++++++++----------- src/libcamera/include/camera_sensor.h | 5 +- 2 files changed, 60 insertions(+), 39 deletions(-)