[libcamera-devel] [PATCH v3 7/8] libcamera: camera_sensor: Initialize frame durations

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Fri, Jun 05, 2020 at 04:10:01PM +0200, Jacopo Mondi wrote:
> Calculate the camera minimum and maximum frame durations, and register
> the corresponding FrameDurationLimits property with those values.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/camera_sensor.cpp | 74 +++++++++++++++++++++++++++++++++
>  1 file changed, 74 insertions(+)
> 
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index b9428175c55e..b144940f047d 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -283,6 +283,80 @@ int CameraSensor::initProperties()
>  		propertyValue = 0;
>  	properties_.set(properties::Rotation, propertyValue);
>  
> +	/* Frame durations. */
> +	std::map<uint32_t, const ControlInfo *> controlLimits;
> +	std::array<uint32_t, 3> controlIds = {
> +		V4L2_CID_VBLANK,
> +		V4L2_CID_HBLANK,
> +		V4L2_CID_PIXEL_RATE
> +	};

This won't work well with existing kernel drivers, for two reasons. One
of them is that the pixel rate is not guaranteed to be constant, it can
depend on the selected mode, or, for sensor drivers that are not
mode-based, controlled by userspace through the link frequency control.
The second reason is that lost of sensor drivers change the limits of
the h/v blank controls based on the selected mode (or sensor
configuration in general).

The latter is likely something we need to fix in V4L2, I think the
maximum h/v blank values should not be mode-dependent. We don't have to
fix all sensor drivers in one go, but we need to at least update the
spec (and thus first reach an agreement). The former isn't a bug, so it
can't be "fixed".

We will very likely need something quite more elaborate than the code
below. I'm available for a brainstorming session on this topic.

> +	for (uint32_t controlId : controlIds) {
> +		/*
> +		 * Make sure the subdevice reports VBLANK, HBLANK and PIXEL_RATE
> +		 * controls and collect the control information.
> +		 */
> +		auto it = controls.find(controlId);
> +		if (it == controls.end()) {
> +			LOG(CameraSensor, Error)
> +				<< "Camera sensor does not support control "
> +				<< controlId;
> +			return -EINVAL;
> +		}
> +
> +		controlLimits[controlId] = &it->second;
> +	}
> +
> +	/*
> +	 * While the maximum line and frame sizes can be calculated by adding
> +	 * each control's maximum supported value to the current configuration,
> +	 * the minimum sizes shall be calculated by using the minimum available
> +	 * resolution. Get the current and smalles available size then calculate
> +	 * the frame durations.
> +	 */
> +	V4L2SubdeviceFormat fmt{};
> +	int ret = subdev_->getFormat(0, &fmt);
> +	if (ret)
> +		return ret;
> +	const Size &currentSize = fmt.size;
> +	const Size &minSize = *sizes_.begin();
> +
> +	std::vector<int32_t> durations;
> +	int32_t duration;
> +
> +	/*
> +	 * Pixel rate is reported in Hz while frame duration is expressed in
> +	 * nanoseconds. Adjust it and calculate the minimum and maximum
> +	 * durations.
> +	 */
> +	int64_t pixelRate = controlLimits[V4L2_CID_PIXEL_RATE]->max().get<int64_t>();
> +	float rate = static_cast<float>(pixelRate) / 1e9F;
> +
> +	/* Minimum frame duration: higher frame rate. */
> +	duration = (minSize.width +
> +		    controlLimits[V4L2_CID_HBLANK]->min().get<int32_t>())
> +		 * (minSize.height +
> +		    controlLimits[V4L2_CID_VBLANK]->min().get<int32_t>());
> +	duration = static_cast<int32_t>(static_cast<float>(duration) / rate);
> +	durations.push_back(duration);
> +
> +	pixelRate = controlLimits[V4L2_CID_PIXEL_RATE]->min().get<int64_t>();
> +	rate = static_cast<float>(pixelRate) / 1e9F;
> +
> +	/* Maximum frame duration: lower frame rate. */
> +	duration = (currentSize.width +
> +		    controlLimits[V4L2_CID_HBLANK]->max().get<int32_t>())
> +		 * (currentSize.height +
> +		    controlLimits[V4L2_CID_VBLANK]->max().get<int32_t>());
> +	duration = static_cast<int32_t>(static_cast<float>(duration) / rate);
> +	durations.push_back(duration);
> +
> +	properties_.set(properties::FrameDurationLimits, durations);
> +
> +	LOG(CameraSensor, Debug) << "Frame durations interval = ["
> +				 << durations[0] << " - "
> +				 << durations[1] << "]";
> +
> +	return 0;
>  }
>  
>  /**

-- 
Regards,

Laurent Pinchart