[libcamera-devel,v2,1/2] libcamera: camera_sensor: Clear camera flips after opening the device

15279 diff mbox series

We clear the V4L2_CID_HFLIP and V4L2_CID_VFLIP controls immediately
after opening the camera device. This means the camera's Bayer format
and mbus codes will be in the sensor's "native" order, and we document
this to be the case so that it can be relied upon.

Clearing the flips is harmless where sensor flips do not affect the
Bayer order.

This also fixes a bug in the Raspberry Pi pipeline handler where the
native Bayer order was being computed wrongly, but the new behaviour
here will be helpful to other pipeline handlers too. A subsequent
commit will tidy up the Raspberry Pi pipeline handler in this area as
it can now be simplified.

Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
Fixes: 83a512816189 (pipeline: raspberrypi: Convert the pipeline handler to use media controller)
---
 src/libcamera/camera_sensor.cpp | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

Quoting David Plowman (2022-01-13 14:15:57)
> We clear the V4L2_CID_HFLIP and V4L2_CID_VFLIP controls immediately
> after opening the camera device. This means the camera's Bayer format
> and mbus codes will be in the sensor's "native" order, and we document
> this to be the case so that it can be relied upon.
> 
> Clearing the flips is harmless where sensor flips do not affect the
> Bayer order.
> 
> This also fixes a bug in the Raspberry Pi pipeline handler where the
> native Bayer order was being computed wrongly, but the new behaviour
> here will be helpful to other pipeline handlers too. A subsequent
> commit will tidy up the Raspberry Pi pipeline handler in this area as
> it can now be simplified.
> 
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> Fixes: 83a512816189 (pipeline: raspberrypi: Convert the pipeline handler to use media controller)

Fixes: 83a512816189 ("pipeline: raspberrypi: Convert the pipeline handler to use media controller")

Fixes tags usually have the $SUBJECT of the fix in quotes I believe.

It's defined by the kernel at:

https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html#describe-your-changes

I don't think that needs a resubmit by itself though, just highlighting
for awareness, as I'm sure I've seen more fixes tags without quotes on
the list.


Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> ---
>  src/libcamera/camera_sensor.cpp | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index c3999d35..64f7f12c 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -107,6 +107,17 @@ int CameraSensor::init()
>         if (ret < 0)
>                 return ret;
>  
> +       /*
> +        * Clear any flips to be sure we get the "native" Bayer order. This is
> +        * harmless for sensors where the flips don't affect the Bayer order.
> +        */
> +       ControlList ctrls(subdev_->controls());
> +       if (subdev_->controls().find(V4L2_CID_HFLIP) != subdev_->controls().end())
> +               ctrls.set(V4L2_CID_HFLIP, 0);
> +       if (subdev_->controls().find(V4L2_CID_VFLIP) != subdev_->controls().end())
> +               ctrls.set(V4L2_CID_VFLIP, 0);
> +       subdev_->setControls(&ctrls);
> +
>         /* Enumerate, sort and cache media bus codes and sizes. */
>         formats_ = subdev_->formats(pad_);
>         if (formats_.empty()) {
> @@ -461,6 +472,11 @@ int CameraSensor::initProperties()
>  /**
>   * \fn CameraSensor::mbusCodes()
>   * \brief Retrieve the media bus codes supported by the camera sensor
> + *
> + * Any Bayer formats are listed using the sensor's native Bayer order,
> + * that is, with the effect of V4L2_CID_HFLIP and V4L2_CID_VFLIP undone
> + * (where these controls exist).
> + *
>   * \return The supported media bus codes sorted in increasing order
>   */
>  
> -- 
> 2.30.2
>

Hi David,

Thank you for your patch.

On Thu, 13 Jan 2022 at 14:16, David Plowman <david.plowman@raspberrypi.com>
wrote:

> We clear the V4L2_CID_HFLIP and V4L2_CID_VFLIP controls immediately
> after opening the camera device. This means the camera's Bayer format
> and mbus codes will be in the sensor's "native" order, and we document
> this to be the case so that it can be relied upon.
>
> Clearing the flips is harmless where sensor flips do not affect the
> Bayer order.
>
> This also fixes a bug in the Raspberry Pi pipeline handler where the
> native Bayer order was being computed wrongly, but the new behaviour
> here will be helpful to other pipeline handlers too. A subsequent
> commit will tidy up the Raspberry Pi pipeline handler in this area as
> it can now be simplified.
>
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> Fixes: 83a512816189 (pipeline: raspberrypi: Convert the pipeline handler
> to use media controller)
>

Reviewed-by: Naushir Patuck <naush@raspberrypi.com>

---
>  src/libcamera/camera_sensor.cpp | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/src/libcamera/camera_sensor.cpp
> b/src/libcamera/camera_sensor.cpp
> index c3999d35..64f7f12c 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -107,6 +107,17 @@ int CameraSensor::init()
>         if (ret < 0)
>                 return ret;
>
> +       /*
> +        * Clear any flips to be sure we get the "native" Bayer order.
> This is
> +        * harmless for sensors where the flips don't affect the Bayer
> order.
> +        */
> +       ControlList ctrls(subdev_->controls());
> +       if (subdev_->controls().find(V4L2_CID_HFLIP) !=
> subdev_->controls().end())
> +               ctrls.set(V4L2_CID_HFLIP, 0);
> +       if (subdev_->controls().find(V4L2_CID_VFLIP) !=
> subdev_->controls().end())
> +               ctrls.set(V4L2_CID_VFLIP, 0);
> +       subdev_->setControls(&ctrls);
> +
>         /* Enumerate, sort and cache media bus codes and sizes. */
>         formats_ = subdev_->formats(pad_);
>         if (formats_.empty()) {
> @@ -461,6 +472,11 @@ int CameraSensor::initProperties()
>  /**
>   * \fn CameraSensor::mbusCodes()
>   * \brief Retrieve the media bus codes supported by the camera sensor
> + *
> + * Any Bayer formats are listed using the sensor's native Bayer order,
> + * that is, with the effect of V4L2_CID_HFLIP and V4L2_CID_VFLIP undone
> + * (where these controls exist).
> + *
>   * \return The supported media bus codes sorted in increasing order
>   */
>
> --
> 2.30.2
>
>