[libcamera-devel] [PATCH v2 2/7] libcamera: pipeline: uvcvideo: Support the new AE controls

[Jacopo Mondi]

Hi Paul,

On Fri, Oct 01, 2021 at 07:33:20PM +0900, Paul Elder wrote:
> Add support for the new AE controls in the uvcvideo pipeline handler.
>
> Bug: https://bugs.libcamera.org/show_bug.cgi?id=42
> Bug: https://bugs.libcamera.org/show_bug.cgi?id=43
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
>
> ---
> Changes in v2:
> - fix the rebase error where some uvc stuff was in rasberrypi
> ---
>  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 36 +++++++++++++++++---
>  1 file changed, 32 insertions(+), 4 deletions(-)
>
> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> index 264f5370..3a9c3b8d 100644
> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> @@ -268,7 +268,9 @@ int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id,
>  		cid = V4L2_CID_CONTRAST;
>  	else if (id == controls::Saturation)
>  		cid = V4L2_CID_SATURATION;
> -	else if (id == controls::AeEnable)
> +	else if (id == controls::ExposureTimeMode)
> +		cid = V4L2_CID_EXPOSURE_AUTO;
> +	else if (id == controls::AnalogueGainMode)
>  		cid = V4L2_CID_EXPOSURE_AUTO;

Shouldn't AnalogueGainMode map to V4L2_CID_AUTOGAIN ?

>  	else if (id == controls::ExposureTime)
>  		cid = V4L2_CID_EXPOSURE_ABSOLUTE;
> @@ -302,9 +304,33 @@ int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id,
>  	}
>
>  	case V4L2_CID_EXPOSURE_AUTO: {
> -		int32_t ivalue = value.get<bool>()
> +		bool exposureSet = controls->contains(V4L2_CID_EXPOSURE_AUTO);

Uh, I don't get this. We add V4L2_CID_EXPOSURE_AUTO to controls at the
end of the function, don't we ?
> +
> +		/* \todo Make this nicer. */

I don't get what happens down here, maybe a most explicative comment
might help ?

> +		int32_t ivalue;
> +		if (id == controls::ExposureTimeMode && exposureSet) {
> +			int32_t exposureMode = controls->get(V4L2_CID_EXPOSURE_AUTO).get<int32_t>();
> +			ivalue = value.get<int32_t>() == controls::ExposureTimeModeAuto
> +			       ? (exposureMode == V4L2_EXPOSURE_SHUTTER_PRIORITY
> +				  ? V4L2_EXPOSURE_AUTO
> +				  : V4L2_EXPOSURE_APERTURE_PRIORITY)
> +			       : V4L2_EXPOSURE_MANUAL;
> +		} else if (id == controls::ExposureTimeMode && !exposureSet) {
> +			ivalue = value.get<int32_t>() == controls::ExposureTimeModeAuto
>  			       ? V4L2_EXPOSURE_APERTURE_PRIORITY
>  			       : V4L2_EXPOSURE_MANUAL;
> +		} else if (id == controls::AnalogueGainMode && exposureSet) {
> +			int32_t exposureMode = controls->get(V4L2_CID_EXPOSURE_AUTO).get<int32_t>();
> +			ivalue = value.get<int32_t>() == controls::AnalogueGainModeAuto
> +			       ? (exposureMode == V4L2_EXPOSURE_APERTURE_PRIORITY
> +				  ? V4L2_EXPOSURE_AUTO
> +				  : V4L2_EXPOSURE_SHUTTER_PRIORITY)
> +			       : V4L2_EXPOSURE_MANUAL;
> +		} else if (id == controls::AnalogueGainMode && !exposureSet) {
> +			ivalue = value.get<int32_t>() == controls::AnalogueGainModeAuto
> +			       ? V4L2_EXPOSURE_SHUTTER_PRIORITY
> +			       : V4L2_EXPOSURE_MANUAL;
> +		}
>  		controls->set(V4L2_CID_EXPOSURE_AUTO, ivalue);
>  		break;
>  	}
> @@ -559,7 +585,7 @@ void UVCCameraData::addControl(uint32_t cid, const ControlInfo &v4l2Info,
>  		id = &controls::Saturation;
>  		break;
>  	case V4L2_CID_EXPOSURE_AUTO:
> -		id = &controls::AeEnable;
> +		id = &controls::ExposureTimeMode;
>  		break;
>  	case V4L2_CID_EXPOSURE_ABSOLUTE:
>  		id = &controls::ExposureTime;
> @@ -610,7 +636,9 @@ void UVCCameraData::addControl(uint32_t cid, const ControlInfo &v4l2Info,
>  		break;
>
>  	case V4L2_CID_EXPOSURE_AUTO:
> -		info = ControlInfo{ false, true, true };
> +		info = ControlInfo{ { controls::ExposureTimeModeAuto,
> +				      controls::ExposureTimeModeDisabled },
> +				    controls::ExposureTimeModeDisabled };

>  		break;
>
>  	case V4L2_CID_EXPOSURE_ABSOLUTE:
> --
> 2.27.0
>