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

[Laurent Pinchart]

Hi Marian,

On Wed, Mar 17, 2021 at 02:17:44PM +0100, Marian Cichy wrote:
> On 3/17/21 11:58 AM, Laurent Pinchart wrote:
> > On Wed, Mar 17, 2021 at 10:55:25AM +0100, Marian Cichy wrote:
> >> On 3/16/21 8:28 PM, Laurent Pinchart wrote:
> >>> 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.
> >
> > Looks like there's some work to do on the sensor side then ;-)
> >
> >> 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.
> >
> > Do you have a use case for this, compared to configuring the sensor with
> > a longer vertical blanking ?
> 
> Not yet, but is it always possible to make my vblank arbitrarily long? 
> For example I have a 60 fps camera but I want to reduce it down to 10 
> fps. Maybe some devices won't allow big enough vblank values.

If you want to go for very low frame rates, such as one frame per
minute, then sensors may not be able to achieve that. What's the
theoretical higher frame duration limit of the CSI ?

> Another possibility could be that you don't have a sensor but an HDMI 
> input as your source, where you can't control the frame rate (but maybe 
> this isn't even in the scope of libcamera).

Indeed out of scope for libcamera, at least for now :-)

> >> 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
> 

-- 
Regards,

Laurent Pinchart