[libcamera-devel,RFC,3/6] libcamera: v4l2_subdevice: Add methods to get/set frame interval
diff mbox series

Message ID 20210316155211.6679-4-m.cichy@pengutronix.de
State Superseded
Headers show
Series
  • Add propagation of sensor frame interval
Related show

Commit Message

Marian Cichy March 16, 2021, 3:52 p.m. UTC
Add API-methods to get and set the frame interval on V4L2-subdevices.

Signed-off-by: Marian Cichy <m.cichy@pengutronix.de>
---
 include/libcamera/internal/v4l2_subdevice.h |  4 ++
 src/libcamera/v4l2_subdevice.cpp            | 43 +++++++++++++++++++++
 2 files changed, 47 insertions(+)

Comments

Laurent Pinchart March 16, 2021, 7:28 p.m. UTC | #1
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@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 ?

> +	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
Marian Cichy March 17, 2021, 9:55 a.m. UTC | #2
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@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
Laurent Pinchart March 17, 2021, 10:58 a.m. UTC | #3
Hi Marian,

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

> 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
Marian Cichy March 17, 2021, 1:17 p.m. UTC | #4
On 3/17/21 11:58 AM, Laurent Pinchart wrote:
> Hi Marian,
>
> 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@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.

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

>
>> 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
Laurent Pinchart March 17, 2021, 2:06 p.m. UTC | #5
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@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
>

Patch
diff mbox series

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);
+	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