[libcamera-devel] [PATCH v2 5/7] libcamera: camera_sensor: Collect camera properties
Niklas Söderlund
niklas.soderlund at ragnatech.se
Wed Aug 28 13:27:58 CEST 2019
Hi Jacopo,
Thanks for your work.
On 2019-08-27 11:50:05 +0200, Jacopo Mondi wrote:
> Store the camera sensor location and rotation by parsing the
> associated V4L2 controls.
>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
> src/libcamera/camera_sensor.cpp | 39 +++++++++++++++++++++++++++
> src/libcamera/include/camera_sensor.h | 4 +++
> 2 files changed, 43 insertions(+)
>
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index a7670b449b31..fc7fdcdcaf5b 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -89,6 +89,45 @@ int CameraSensor::init()
> if (ret < 0)
> return ret;
>
> + /* Retrieve and store the camera sensor properties. */
> + V4L2ControlList controls({
> + V4L2_CID_CAMERA_SENSOR_LOCATION,
> + V4L2_CID_CAMERA_SENSOR_ROTATION,
> + });
> + ret = subdev_->getControls(&controls);
> + if (ret) {
> + LOG(CameraSensor, Error)
> + << "Failed to get camera sensor controls: " << ret;
> + return ret;
> + }
I still feel this is a bit harsh for the moment. If the sensor don't
support V4L2_CID_CAMERA_SENSOR_LOCATION and
V4L2_CID_CAMERA_SENSOR_ROTATION this will fail and the camera is not
registered with the system. I see two problems with this,
1. If the camera location is EXTERNAL is rotation really a mandatory
property?
2. I agree that in the future when the kernel support for
SENSOR_LOCATION have taken hold we can make it mandatory. For now
would it not be a good idea to set location to EXTERNAL if it's not
present and print a warning?
I fear we will block this series for a very long time without easing
the constraints for a while.
> +
> + V4L2Control *control = controls[V4L2_CID_CAMERA_SENSOR_LOCATION];
> + int64_t value = control->value();
> + switch (value) {
> + case V4L2_LOCATION_EXTERNAL:
> + location_ = CAMERA_LOCATION_EXTERNAL;
> + break;
> + case V4L2_LOCATION_FRONT:
> + location_ = CAMERA_LOCATION_FRONT;
> + break;
> + case V4L2_LOCATION_BACK:
> + location_ = CAMERA_LOCATION_BACK;
> + break;
> + default:
> + LOG(CameraSensor, Error)
> + << "Unsupported camera location: " << value;
> + return -EINVAL;
> + }
> +
> + control = controls[V4L2_CID_CAMERA_SENSOR_ROTATION];
> + value = control->value();
> + if (value < 0 || value > 360) {
> + LOG(CameraSensor, Error)
> + << "Unsupported camera rotation: " << value;
> + return -EINVAL;
> + }
> + rotation_ = value;
> +
> /* Enumerate and cache media bus codes and sizes. */
> const ImageFormats formats = subdev_->formats(0);
> if (formats.isEmpty()) {
> diff --git a/src/libcamera/include/camera_sensor.h b/src/libcamera/include/camera_sensor.h
> index fe033fb374c1..32d39127b275 100644
> --- a/src/libcamera/include/camera_sensor.h
> +++ b/src/libcamera/include/camera_sensor.h
> @@ -10,6 +10,7 @@
> #include <string>
> #include <vector>
>
> +#include <libcamera/control_ids.h>
> #include <libcamera/geometry.h>
>
> #include "log.h"
> @@ -55,6 +56,9 @@ private:
>
> std::vector<unsigned int> mbusCodes_;
> std::vector<Size> sizes_;
> +
> + CameraLocation location_;
> + unsigned int rotation_;
> };
>
> } /* namespace libcamera */
> --
> 2.23.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list