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

[Jacopo Mondi]

Hi Laurent,

On Sat, Jul 04, 2020 at 03:52:23AM +0300, Laurent Pinchart 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));

mmm, I see in the IPA this, which is where I assume you got this from

		ctrls.set(V4L2_CID_HFLIP, (int32_t) !!(orientation & RPi::CamTransform_HFLIP));
		ctrls.set(V4L2_CID_VFLIP, (int32_t) !!(orientation & RPi::CamTransform_VFLIP));

Where the 'orientation' flag is retrieved from
		RPi::CamTransform orientation = helper_->GetOrientation();

and expressed through members of this enumeration
                static constexpr CamTransform CamTransform_IDENTITY = 0;
                static constexpr CamTransform CamTransform_HFLIP    = 1;
                static constexpr CamTransform CamTransform_VFLIP    = 2;

Now, 'rotation' is expressed in multiple of 90 degrees in the [0-360[
interval, shouldn't we:
        if (rotation == 180)
                HFLIP
        if (rotation == 90 || roation == 270)
                VFLIP
 ?


> +	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);

Do you think we can now remove the custom rotation handling from the
IPA helpers ?

>  		return;
>  	}
>
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel