[libcamera-devel] [PATCH 4/6] libcamera: pipeline: uvcvideo: Set device's flip controls correctly

[Jacopo Mondi]

Hi David

On Thu, Nov 10, 2022 at 02:45:54PM +0000, David Plowman via libcamera-devel wrote:
> Set the horizontal and vertical flip controls correctly by
> calling the device's setTransform method now that this no longer
> happens in CameraSensor::init().
>
> Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> ---
>  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> index 277465b7..4a891c23 100644
> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> @@ -209,6 +209,9 @@ int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config)
>  	StreamConfiguration &cfg = config->at(0);
>  	int ret;
>
> +	/* Set up the video device's horizontal and vertical flips. */
> +	data->video_->setTransform(config->transform);
> +

Do we need this ? The validate() function does

	if (transform != Transform::Identity) {
		transform = Transform::Identity;
		status = Adjusted;
	}

Same for simple and rkisp1.

This opens a different question. What happens if another application
sets a flip before libcamera is started ? Should we here instead
reset all flips to their default values ? Using the newly introduced
setTrasform(Identity) wouldn't be enough as it would not clean up any
externally set flip...

What would the expected configuration from the application point of
view ? Whatever flip is externally set is kept, or all flips
are reset to their default ? Please note that "reset to their
defaults" != "clear any flip flag" as most drivers (and we discussed
this with Dave recently) do inizialize their flip controls to report
what is actually programmed in registers, and quite often sensors are
180 degrees flipped by default:
https://lore.kernel.org/all/CAPY8ntCCpMZtG-eJQ8cgd_GO06ZDP32oRK8mGrZKGMU2W401Dg@mail.gmail.com/
This is one more reason to remove the "set flips to 0" in init.

Do we need a V4L2Device::resetFlips() function instead ?

>  	V4L2DeviceFormat format;
>  	format.fourcc = data->video_->toV4L2PixelFormat(cfg.pixelFormat);
>  	format.size = cfg.size;
> --
> 2.30.2
>