[FFmpeg-devel] PATCH grab.c, adding support for hardware based VIDIOSFPS

[Benoit Fouet]

Hi,

Trent Piepho wrote:
> On Mon, 4 Jun 2007, Benoit Fouet wrote:
>   
>> mmh wrote:
>>     
>>>  >
>>>  > this is wrong.
>>>  > from my videodev2.h:
>>>  > #define VIDIOC_S_RDS            _IOWR ('V', BASE_VIDIOC_PRIVATE+11,
>>>  > struct v4l2_radio_rds_set)
>>>  >
>>>  > i can (if i want) change the BASE_VIDIOC_PRIVATE+11 by
>>>  > BASE_VIDIOC_PRIVATE+20
>>>  > this ioctl will be defined, but will not do what you want...
>>>  >
>>>  > moreover, it can just return without any error, and lead you to believe
>>>  > that framerate is accepted by the camera, even though this is not true...
>>>       
>
> Not exactly.  The size of the argument to the ioctl (struct
> v4l2_radio_rds_set) is part of the ioctl number.  You could assign two
> different ioctls to BASE_VIDIOC_PRIVATE+20 and they won't be the same if
> the size of the arugment and the direction (R, W, WR) doesn't match.
>
>   

yes, my explanation was quite incomplete

>>> #ifndef VIDIOSFPS
>>> #define VIDIOSFPS		_IOW('v',BASE_VIDIOCPRIVATE+20, int)			/* Set fps */
>>> #endif
>>>
>>>       
>> yes, i really think this should be removed
>> and, FWIW, syntax in v4l2 is, most of the time, as follows:
>> VIDIOC_S_*
>> i don't know if it's planned to add such a feature in future videodev
>> version, though...
>>     
>
> This isn't a v4l2 ioctl, but a v4l1 ioctl.  V4L2 uses the ioctl range 'V',
> while v4l1 uses 'v'.  Notice the case difference.  Also BASE_VIDIOCPRIVATE
> is the base for v4l1 private ioctls and BASE_VIDIOC_PRIVATE is the base for
> v4l2.  It should still be VIDIOCSFPS to have correct v4l1 naming
> conventions.
>
>   

ok, my bad, i work mainly with v4l2...

> The whole ioctl name clash thing isn't solved by just removing the #define
> from the ffmpeg code.  What happens if someone compiles on a kernel where
> 'v',private+20 is SFPS and then runs on a kernel where it's something else?
>
>   
the ifdef should do it, no ?
and compiling on another machine (or at least configuration) than the
running one is bad :)

> This is why changing ioctls once they make it into the kernel isn't
> allowed.

anyway, thanks for developping ;)
Ben
-- 
Purple Labs S.A.
www.purplelabs.com