| Message ID | 20201230230603.123486-4-jacopo@jmondi.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, Thanks for your work. On 2020-12-31 00:06:00 +0100, Jacopo Mondi wrote: > Support for the V4L2 selection API is currently optional in the > CameraSensor class. Properties registered by using values read through > that API are defaulted in several different places (the Android camera > HAL or the CameraSensor class). > > In future support for the selection API will be made mandatory, but to > give time to sensor drivers in all test platforms to be updated, provide > a default for the sensor pixel array properties and cache them as > class member variables. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > include/libcamera/internal/camera_sensor.h | 3 ++ > src/libcamera/camera_sensor.cpp | 61 +++++++++------------- > 2 files changed, 28 insertions(+), 36 deletions(-) > > diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h > index aee10aa6e3c7..86902b85ada8 100644 > --- a/include/libcamera/internal/camera_sensor.h > +++ b/include/libcamera/internal/camera_sensor.h > @@ -84,6 +84,9 @@ private: > std::vector<unsigned int> mbusCodes_; > std::vector<Size> sizes_; > > + Size pixelArraySize_; > + Rectangle activeArea_; > + > ControlList properties_; > }; > > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp > index 048fcd6a1fae..bf0d6b06ae20 100644 > --- a/src/libcamera/camera_sensor.cpp > +++ b/src/libcamera/camera_sensor.cpp > @@ -31,6 +31,9 @@ namespace libcamera { > > LOG_DEFINE_CATEGORY(CameraSensor) > > +static constexpr Size defaultPixelArraySize = { 2592, 1944 }; > +static constexpr Rectangle defaultActiveArea = { 16, 12, 2560, 1920 }; > + > /** > * \struct CameraSensorInfo > * \brief Report the image sensor characteristics > @@ -269,15 +272,22 @@ int CameraSensor::validateSensorDriver() > Rectangle rect; > int ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_BOUNDS, &rect); > if (ret) { > + rect.width = defaultPixelArraySize.width; > + rect.height = defaultPixelArraySize.height; > LOG(CameraSensor, Warning) > - << "Failed to retrieve the readable pixel array size"; > + << "The PixelArraySize property has been defaulted to " > + << rect.toString(); > err = -EINVAL; > } > + pixelArraySize_.width = rect.width; > + pixelArraySize_.height = rect.height; > > - ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_DEFAULT, &rect); > + ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_DEFAULT, &activeArea_); > if (ret) { > + activeArea_ = defaultActiveArea; > LOG(CameraSensor, Warning) > - << "Failed to retrieve the active pixel array size"; > + << "The PixelArrayActiveAreas property has been defaulted to: " > + << activeArea_.toString(); > err = -EINVAL; > } > > @@ -373,24 +383,8 @@ int CameraSensor::initProperties() > propertyValue = 0; > properties_.set(properties::Rotation, propertyValue); > > - Rectangle bounds; > - ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_BOUNDS, &bounds); > - if (!ret) > - properties_.set(properties::PixelArraySize, bounds.size()); > - > - Rectangle crop; > - ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_DEFAULT, &crop); > - if (!ret) { > - /* > - * V4L2_SEL_TGT_CROP_DEFAULT and V4L2_SEL_TGT_CROP_BOUNDS are > - * defined relatively to the sensor full pixel array size, > - * while properties::PixelArrayActiveAreas is defined relatively > - * to properties::PixelArraySize. Adjust it. > - */ > - crop.x -= bounds.x; > - crop.y -= bounds.y; > - properties_.set(properties::PixelArrayActiveAreas, { crop }); > - } > + properties_.set(properties::PixelArraySize, pixelArraySize_); > + properties_.set(properties::PixelArrayActiveAreas, { activeArea_ }); > > /* Color filter array pattern, register only for RAW sensors. */ > for (const auto &format : formats_) { > @@ -646,20 +640,15 @@ int CameraSensor::sensorInfo(CameraSensorInfo *info) const > { > info->model = model(); > > - /* Get the active area size. */ > - Rectangle rect; > - int ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_DEFAULT, &rect); > - if (ret) { > - LOG(CameraSensor, Error) > - << "Failed to construct camera sensor info: " > - << "the camera sensor does not report the active area"; > - > - return ret; > - } > - info->activeAreaSize = { rect.width, rect.height }; > + /* > + * The active area size is a static property, while the crop > + * rectangle needs to be re-read as it depends on the sensor > + * configuration. > + */ > + info->activeAreaSize = { activeArea_.width, activeArea_.height }; > > /* It's mandatory for the subdevice to report its crop rectangle. */ > - ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP, &info->analogCrop); > + int ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP, &info->analogCrop); > if (ret) { > LOG(CameraSensor, Error) > << "Failed to construct camera sensor info: " > @@ -672,10 +661,10 @@ int CameraSensor::sensorInfo(CameraSensorInfo *info) const > * are defined relatively to the active pixel area, while V4L2's > * TGT_CROP target is defined in respect to the full pixel array. > * > - * Compensate it by subtracting the active areas offset. > + * Compensate it by subtracting the active area offset. > */ > - info->analogCrop.x -= rect.x; > - info->analogCrop.y -= rect.y; > + info->analogCrop.x -= activeArea_.x; > + info->analogCrop.y -= activeArea_.y; > > /* The bit depth and image size depend on the currently applied format. */ > V4L2SubdeviceFormat format{}; > -- > 2.29.2 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h index aee10aa6e3c7..86902b85ada8 100644 --- a/include/libcamera/internal/camera_sensor.h +++ b/include/libcamera/internal/camera_sensor.h @@ -84,6 +84,9 @@ private: std::vector<unsigned int> mbusCodes_; std::vector<Size> sizes_; + Size pixelArraySize_; + Rectangle activeArea_; + ControlList properties_; }; diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp index 048fcd6a1fae..bf0d6b06ae20 100644 --- a/src/libcamera/camera_sensor.cpp +++ b/src/libcamera/camera_sensor.cpp @@ -31,6 +31,9 @@ namespace libcamera { LOG_DEFINE_CATEGORY(CameraSensor) +static constexpr Size defaultPixelArraySize = { 2592, 1944 }; +static constexpr Rectangle defaultActiveArea = { 16, 12, 2560, 1920 }; + /** * \struct CameraSensorInfo * \brief Report the image sensor characteristics @@ -269,15 +272,22 @@ int CameraSensor::validateSensorDriver() Rectangle rect; int ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_BOUNDS, &rect); if (ret) { + rect.width = defaultPixelArraySize.width; + rect.height = defaultPixelArraySize.height; LOG(CameraSensor, Warning) - << "Failed to retrieve the readable pixel array size"; + << "The PixelArraySize property has been defaulted to " + << rect.toString(); err = -EINVAL; } + pixelArraySize_.width = rect.width; + pixelArraySize_.height = rect.height; - ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_DEFAULT, &rect); + ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_DEFAULT, &activeArea_); if (ret) { + activeArea_ = defaultActiveArea; LOG(CameraSensor, Warning) - << "Failed to retrieve the active pixel array size"; + << "The PixelArrayActiveAreas property has been defaulted to: " + << activeArea_.toString(); err = -EINVAL; } @@ -373,24 +383,8 @@ int CameraSensor::initProperties() propertyValue = 0; properties_.set(properties::Rotation, propertyValue); - Rectangle bounds; - ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_BOUNDS, &bounds); - if (!ret) - properties_.set(properties::PixelArraySize, bounds.size()); - - Rectangle crop; - ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_DEFAULT, &crop); - if (!ret) { - /* - * V4L2_SEL_TGT_CROP_DEFAULT and V4L2_SEL_TGT_CROP_BOUNDS are - * defined relatively to the sensor full pixel array size, - * while properties::PixelArrayActiveAreas is defined relatively - * to properties::PixelArraySize. Adjust it. - */ - crop.x -= bounds.x; - crop.y -= bounds.y; - properties_.set(properties::PixelArrayActiveAreas, { crop }); - } + properties_.set(properties::PixelArraySize, pixelArraySize_); + properties_.set(properties::PixelArrayActiveAreas, { activeArea_ }); /* Color filter array pattern, register only for RAW sensors. */ for (const auto &format : formats_) { @@ -646,20 +640,15 @@ int CameraSensor::sensorInfo(CameraSensorInfo *info) const { info->model = model(); - /* Get the active area size. */ - Rectangle rect; - int ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_DEFAULT, &rect); - if (ret) { - LOG(CameraSensor, Error) - << "Failed to construct camera sensor info: " - << "the camera sensor does not report the active area"; - - return ret; - } - info->activeAreaSize = { rect.width, rect.height }; + /* + * The active area size is a static property, while the crop + * rectangle needs to be re-read as it depends on the sensor + * configuration. + */ + info->activeAreaSize = { activeArea_.width, activeArea_.height }; /* It's mandatory for the subdevice to report its crop rectangle. */ - ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP, &info->analogCrop); + int ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP, &info->analogCrop); if (ret) { LOG(CameraSensor, Error) << "Failed to construct camera sensor info: " @@ -672,10 +661,10 @@ int CameraSensor::sensorInfo(CameraSensorInfo *info) const * are defined relatively to the active pixel area, while V4L2's * TGT_CROP target is defined in respect to the full pixel array. * - * Compensate it by subtracting the active areas offset. + * Compensate it by subtracting the active area offset. */ - info->analogCrop.x -= rect.x; - info->analogCrop.y -= rect.y; + info->analogCrop.x -= activeArea_.x; + info->analogCrop.y -= activeArea_.y; /* The bit depth and image size depend on the currently applied format. */ V4L2SubdeviceFormat format{};