[libcamera-devel] [RFC PATCH 3/6] libcamera: v4l2_subdevice: Add methods to get/set frame interval

[Marian Cichy]

On 3/16/21 8:28 PM, Laurent Pinchart wrote:
> Hi Marian,
>
> Thank you for the patch.
>
> On Tue, Mar 16, 2021 at 04:52:08PM +0100, Marian Cichy wrote:
>> Add API-methods to get and set the frame interval on V4L2-subdevices.
>>
>> Signed-off-by: Marian Cichy <m.cichy at pengutronix.de>
>> ---
>>   include/libcamera/internal/v4l2_subdevice.h |  4 ++
>>   src/libcamera/v4l2_subdevice.cpp            | 43 +++++++++++++++++++++
>>   2 files changed, 47 insertions(+)
>>
>> diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h
>> index d2b9ca55..310746d9 100644
>> --- a/include/libcamera/internal/v4l2_subdevice.h
>> +++ b/include/libcamera/internal/v4l2_subdevice.h
>> @@ -12,6 +12,7 @@
>>   #include <vector>
>>   
>>   #include <libcamera/class.h>
>> +#include <libcamera/fraction.h>
>>   #include <libcamera/geometry.h>
>>   
>>   #include "libcamera/internal/formats.h"
>> @@ -60,6 +61,9 @@ public:
>>   	int setFormat(unsigned int pad, V4L2SubdeviceFormat *format,
>>   		      Whence whence = ActiveFormat);
>>   
>> +	int getFrameInterval(unsigned int pad, Fraction *frameInterval);
>> +	int setFrameInterval(unsigned int pad, Fraction *frameInterval);
>> +
>>   	static std::unique_ptr<V4L2Subdevice>
>>   	fromEntityName(const MediaDevice *media, const std::string &entity);
>>   
>> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
>> index 721ff5a9..eac2d7d3 100644
>> --- a/src/libcamera/v4l2_subdevice.cpp
>> +++ b/src/libcamera/v4l2_subdevice.cpp
>> @@ -17,6 +17,7 @@
>>   #include <linux/media-bus-format.h>
>>   #include <linux/v4l2-subdev.h>
>>   
>> +#include <libcamera/fraction.h>
>>   #include <libcamera/geometry.h>
>>   
>>   #include "libcamera/internal/log.h"
>> @@ -440,6 +441,48 @@ int V4L2Subdevice::setFormat(unsigned int pad, V4L2SubdeviceFormat *format,
>>   	return 0;
>>   }
>>   
>> +int V4L2Subdevice::getFrameInterval(unsigned int pad, Fraction *frameInterval)
>> +{
>> +	struct v4l2_subdev_frame_interval subdevFrameInterval = {};
>> +	subdevFrameInterval.pad = pad;
>> +
>> +	int ret = ioctl(VIDIOC_SUBDEV_G_FRAME_INTERVAL, &subdevFrameInterval);
> Sensor drivers are supposed to control the frame rate through horizontal
> and vertical blanking. Only in very specific cases do we want to use
> these ioctls, when the sensor exposes a frame interval at the register
> level (this will only be applicable to sensors containing an ISP). What
> sensor are you using ?

I am using the ar0237 sensor, it's driver is not upstream yet. We 
implement hblank/vblank only as read-only controls for now.

However, using the i.MX-6Q, it is also possible to configure 
frame-dropping on the CSI-subdev. Afaik, this can only be configured 
with the frame_interval ioctl since the frame interval has to be set on 
source and sink pad.

This is also why we propagate the frame interval from the sensor through 
the media pipeline. If for some reason, the CSI is configured to drop 
frames, we query the frame interval at the sensor, then try to set it on 
all following subdevices to effectively deactivate frame dropping.

Of course, it is also thinkable that we want to configure the CSI to 
drop frames, for example to manage bandwidth.

>
>> +	if (ret) {
>> +		LOG(V4L2, Debug)
>> +			<< "Unable to get frame interval on pad " << pad
>> +			<< ": " << strerror(-ret);
>> +		return ret;
>> +	}
>> +
>> +	frameInterval->numerator = subdevFrameInterval.interval.numerator;
>> +	frameInterval->denominator = subdevFrameInterval.interval.denominator;
>> +
>> +	return 0;
>> +}
>> +
>> +int V4L2Subdevice::setFrameInterval(unsigned int pad, Fraction *frameInterval)
>> +{
>> +	struct v4l2_subdev_frame_interval subdevFrameInterval = {};
>> +	struct v4l2_fract fract = { frameInterval->numerator,
>> +				    frameInterval->denominator };
>> +
>> +	subdevFrameInterval.pad = pad;
>> +	subdevFrameInterval.interval = fract;
>> +
>> +	int ret = ioctl(VIDIOC_SUBDEV_S_FRAME_INTERVAL, &subdevFrameInterval);
>> +	if (ret) {
>> +		LOG(V4L2, Debug)
>> +			<< "Unable to set frame interval on pad " << pad
>> +			<< ": " << strerror(-ret);
>> +		return ret;
>> +	}
>> +
>> +	frameInterval->numerator = subdevFrameInterval.interval.numerator;
>> +	frameInterval->denominator = subdevFrameInterval.interval.denominator;
>> +
>> +	return 0;
>> +}
>> +
>>   /**
>>    * \brief Create a new video subdevice instance from \a entity in media device
>>    * \a media