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

[mmh]

Benoit Fouet writes:
 > Hi,
 > 
 > mmh wrote:
 > > If the underlying video4linux driver supports frame rate control then
 > > use it instead of user space software support.
 > >
 > >   
 > > ------------------------------------------------------------------------
 > >
 > > Index: grab.c
 > > ===================================================================
 > > --- grab.c	(revision 9203)
 > > +++ grab.c	(working copy)
 > > @@ -28,6 +28,10 @@
 > >  #include <linux/videodev.h>
 > >  #include <time.h>
 > >  
 > > +#ifndef VIDIOSFPS
 > > +#define VIDIOSFPS		_IOW('v',BASE_VIDIOCPRIVATE+20, int)			/* Set fps */
 > > +#endif
 > > +
 > >   
 > 
 > i think this is "dangerous", as this ioctl id can be used for "private"
 > usage
 > 
 > >  typedef struct {
 > >      int fd;
 > >      int frame_format; /* see VIDEO_PALETTE_xxx */
 > > @@ -35,6 +39,7 @@
 > >      int width, height;
 > >      int frame_rate;
 > >      int frame_rate_base;
 > > +    int sw_fps_reduction;
 > >      int64_t time_frame;
 > >      int frame_size;
 > >      struct video_capability video_cap;
 > > @@ -78,6 +83,7 @@
 > >      VideoData *s = s1->priv_data;
 > >      AVStream *st;
 > >      int width, height;
 > > +    int fps;
 > >      int video_fd, frame_size;
 > >      int ret, frame_rate, frame_rate_base;
 > >      int desired_palette, desired_depth;
 > > @@ -183,6 +189,9 @@
 > >              goto fail1;
 > >      }
 > >  
 > > +    fps = s->frame_rate / s->frame_rate_base;
 > > +    s->sw_fps_reduction  = ioctl( video_fd, VIDIOSFPS, &fps ) < 0;
 > > +
 > >   
 > 
 > maybe something like:
 > #ifdef VIDIOSFPS
 >     fps = s->frame_rate / s->frame_rate_base;
 >     s->sw_fps_reduction = ioctl( video_fd, VIDIOSFPS, &fps ) < 0;
 > #else
 >     s->sw_fps_reduction = 1;
 > #endif

I like this thanks for this idea I thought about wrapping the whole
thing but then hesitated your point is well taken.

 > 
 > you may also need to flag fps variable definition...
????

 > 
 > >      ret = ioctl(video_fd,VIDIOCGMBUF,&s->gb_buffers);
 > >      if (ret < 0) {
 > >          /* try to use read based access */
 > > @@ -327,9 +336,11 @@
 > >              }
 > >              break;
 > >          }
 > > -        ts.tv_sec = delay / 1000000;
 > > -        ts.tv_nsec = (delay % 1000000) * 1000;
 > > -        nanosleep(&ts, NULL);
 > > +        if (s->sw_fps_reduction) {
 > > +            ts.tv_sec = delay / 1000000;
 > > +            ts.tv_nsec = (delay % 1000000) * 1000;
 > > +            nanosleep(&ts, NULL);
 > > +	}
 > >   
 > 
 > and the cosmetics should come as a separate patch
 > 

Sorry, I don't agree really because the indentation here is part of
the improvement I don't see it as a cosmetic.

 > >      }
 > >  
 > >      if (av_new_packet(pkt, s->frame_size) < 0)
 > >   


This doesn't have the cosmetic removed because you can't see what the
new functionality is with out it.  I will commit the white space
separately if the group really wants me to.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: grab.fps.p3
Type: text/x-patch
Size: 1620 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070604/f220bf15/attachment.bin>
-------------- next part --------------

Thanks
Marc