[libcamera-devel,2/3] pipeline: simple: Add FrameDurations to stream configuration

11639 diff mbox series

Add a instance of CameraSensorInfo to the simple pipeline and populate
it with the sensorInfo from the sensor. With the informations from the
sensor, compute the frame duration by using lineLength, minFrameLength
and pixelRate and save it in the controlList of the stream.

Signed-off-by: Marian Cichy <m.cichy@pengutronix.de>
---
 src/libcamera/pipeline/simple/simple.cpp | 9 +++++++++
 1 file changed, 9 insertions(+)

Hi Marian,

On Mon, Mar 22, 2021 at 11:42:41AM +0100, Marian Cichy wrote:
> Add a instance of CameraSensorInfo to the simple pipeline and populate
> it with the sensorInfo from the sensor. With the informations from the
> sensor, compute the frame duration by using lineLength, minFrameLength
> and pixelRate and save it in the controlList of the stream.

I would more simply phrase it as:

Use the camera sensor frame lenght an pixel rate to calculate the
per-stream FrameDurations in the simple pipeline handler.

>
> Signed-off-by: Marian Cichy <m.cichy@pengutronix.de>
> ---
>  src/libcamera/pipeline/simple/simple.cpp | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 24d940fc..2df83302 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -22,6 +22,8 @@
>  #include <linux/media-bus-format.h>
>
>  #include <libcamera/camera.h>
> +#include <libcamera/controls.h>
> +#include <libcamera/control_ids.h>
>  #include <libcamera/request.h>
>  #include <libcamera/stream.h>
>
> @@ -176,6 +178,7 @@ public:
>
>  	std::vector<Stream> streams_;
>  	std::unique_ptr<CameraSensor> sensor_;
> +	CameraSensorInfo sensorInfo_;
>  	std::list<Entity> entities_;
>  	V4L2VideoDevice *video_;
>
> @@ -339,6 +342,8 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,
>  	ret = sensor_->init();
>  	if (ret)
>  		sensor_.reset();
> +
> +	sensor_->sensorInfo(&sensorInfo_);

If I'm not mistaken the simple pipeline handler is meant to work with
YUV sensors too, for which it is not possible to retrieve the
CameraSensorInfo so you should check the return error and not try to
access it if this call fails.

Also, the content of CameraSensorInfo depends on the current sensor
configuration so you shall fetch it at configure() time otherwise you
would work with a stale representation of the sensor configuration.

>  }
>
>  int SimpleCameraData::init()
> @@ -679,6 +684,10 @@ CameraConfiguration *SimplePipelineHandler::generateConfiguration(Camera *camera
>  		StreamConfiguration cfg{ StreamFormats{ formats } };
>  		cfg.pixelFormat = formats.begin()->first;
>  		cfg.size = formats.begin()->second[0].max;
> +		double frameDuration = (data->sensorInfo_.minFrameLength * data->sensorInfo_.lineLength) /
> +			(data->sensorInfo_.pixelRate / 1'000'000);

Using the sensor information at generateConfiguration() time is
unfortunately a no-go in my opinion. As the information there reported
depend on the configuration applied to the sensor, you would here get
either the ones relative to the default sensor configuration (the
first time you generate a configuration) or even worse the information
relative to the configuration that was applied the last time
Camera::configure() was called. IOW do not use CameraSensorInfo if not
after a setFormat() has been called on the sensor.

If you intend to calculate the absolute limits of the Camera that's
different, and you can read as an example the IPU3 pipeline handler
initControls() how its done (and we had to made assumptions there
too).

As you can see minFrameLength and well as maxFrameLenght represent the
lower and upper frame lenght bounds, not the current frame lenght :/
If you where doing this at configure() time I would have rather used
the frame sizes and get the VBLANK control value to compute the frame
lenght, but that's a problem for you as you want this information to
be reported at generateConfiguration() time not at configure() time
and at this very time no configuration has been applied to the sensor
:/

IOW at 'generateConfiguration()' time you can only have a (probably
misleading) approximation of the current frame duration, as you would
need to apply a configuration to the sensor to have that information
computed properly. And as you can only here estimate the absolute
limits by using the min/max frame lenghtes, I would rather compute
them at camera creation time as we do in the other pipeline handlers
:/

I'm sorry but I don't see an easy way out here as we really need to
kick the driver with a SUBDEV_S_FMT to have at least VBLANK and pixel
rate updated. Maybe Laurent has some ideas ?

Thanks
   j

> +		cfg.controls.set(controls::FrameDurations, { static_cast<int64_t>(frameDuration),
> +							     static_cast<int64_t>(frameDuration) });
>
>  		config->addConfiguration(cfg);
>  	}
> --
> 2.29.2
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel