[libcamera-devel] [PATCH v2 08/12] libcamera: pipeline: raspberrypi: Set sensor flip based on rotation

[Naushir Patuck]

Hi Laurent,

Thank you for your patch.

On Sat, 4 Jul 2020 at 01:52, Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> Instead of receiving sensor orientation configuration from the IPA,
> retrieve it from the CameraSensor Rotation property, and configure the
> HFLIP and VFLIP controls accordingly.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 74bee6895dcd..fda6831e6a7e 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -16,6 +16,7 @@
>  #include <libcamera/formats.h>
>  #include <libcamera/ipa/raspberrypi.h>
>  #include <libcamera/logging.h>
> +#include <libcamera/property_ids.h>
>  #include <libcamera/request.h>
>  #include <libcamera/stream.h>
>
> @@ -1174,6 +1175,13 @@ int RPiCameraData::configureIPA()
>         ipa_->configure(sensorInfo, streamConfig, entityControls, ipaConfig,
>                         nullptr);
>
> +       /* Configure the H/V flip controls based on the sensor rotation. */
> +       ControlList ctrls(unicam_[Unicam::Image].dev()->controls());
> +       int32_t rotation = sensor_->properties().get(properties::Rotation);
> +       ctrls.set(V4L2_CID_HFLIP, static_cast<int32_t>(!!rotation));
> +       ctrls.set(V4L2_CID_VFLIP, static_cast<int32_t>(!!rotation));

Are you missing a ">> 1" or similar in the above code?  Otherwise,
both H/V FLIP will be always set to the same value.

Regards,
Naush


> +       unicam_[Unicam::Image].dev()->setControls(&ctrls);
> +
>         return 0;
>  }
>
> @@ -1202,10 +1210,6 @@ void RPiCameraData::queueFrameAction(unsigned int frame, const IPAOperationData
>                                               { V4L2_CID_EXPOSURE, action.data[1] } });
>                         sensorMetadata_ = action.data[2];
>                 }
> -
> -               /* Set the sensor orientation here as well. */
> -               ControlList controls = action.controls[0];
> -               unicam_[Unicam::Image].dev()->setControls(&controls);
>                 return;
>         }
>
> --
> Regards,
>
> Laurent Pinchart
>