[FFmpeg-devel] [PATCH] Misc patches for libavdevice/v4l.c

[Stefano Sabatini]

On date Friday 2008-12-26 14:09:20 +0100, Michael Niedermayer encoded:
> On Fri, Dec 26, 2008 at 11:36:36AM +0100, Stefano Sabatini wrote:
> > On date Friday 2008-12-26 00:52:12 +0100, Michael Niedermayer encoded:
> > > On Wed, Dec 24, 2008 at 03:37:05PM +0100, Stefano Sabatini wrote:
> [...]
> > > [...]
> > > 
> > > 
> > > > Index: ffmpeg/libavdevice/v4l.c
> > > > ===================================================================
> > > > --- ffmpeg.orig/libavdevice/v4l.c	2008-12-21 23:45:13.000000000 +0100
> > > > +++ ffmpeg/libavdevice/v4l.c	2008-12-24 13:34:57.000000000 +0100
> > > > @@ -84,11 +84,6 @@
> > > >      }
> > > >      s->time_base = ap->time_base;
> > > >  
> > > > -    if((unsigned)ap->width > 32767 || (unsigned)ap->height > 32767) {
> > > > -        av_log(s1, AV_LOG_ERROR, "Capture size is out of range: %dx%d\n",
> > > > -            ap->width, ap->height);
> > > > -        return -1;
> > > > -    }
> > > >      s->video_win.width = ap->width;
> > > >      s->video_win.height = ap->height;
> > > >  
> > > ?
> > 
> > The idea is that this check is useless, since either the VIDIOCSWIN
> > either the VIDIOCMCAPTURE iotctl will perform a check on the size (but
> > I don't know where the 32767 value comes from).
> 
> I think the check is insufficient and more not less checking is needed
> 
>  frame_size = s->video_win.width * s->video_win.height * video_formats[j].depth / 8;
> 
> will not work with 32767*32767*...

OK, 32767 = 2^15 -1.

We may then check for 16383 = 2^14 -1 (check the patch below), or
maybe some function like these ones may help:

/* check this for an introduction to overflow */
/* http://www.fefe.de/intof.html */

/**
 * Safe-multiply \p a for \p b and put the result in \p res.
 *
 * @return a negative number in case of overflow, 0 otherwise.
 */
int safe_mul32(int32_t *res, int32_t a, int32_t b)
{
    int64_t res1 = (int64_t)a * b;
    if (res1 > 0x7fffffff)      /* INT32_MAX */
        return -1;
    *res = res1;
    return 0;
}

/**
 * Safe-multiply a list of int32_t number and put the result in \p
 * res.
 *
 * @param num the number of int32_t to be multiplied, it has to match the
 * number of parameters provided
 * @return a negative number in case of overflow, 0 otherwise.
 */
int vsafe_mul32(int32_t *res, int num, ...)
{
  va_list args;
  int i;

  *res = 1;

  va_start(args, num);

  for (i = 0; i < num; i++)
      if (safe_mul32(res, *res, va_arg(args, int32_t)) < 0)
          return -1;

  va_end(args);

  return 0;
}

I don't know if we have already something like that in libavutil.

> > Patches attached:
> > 
> > * v4l-add-videocswin-check.patch: add a missing check
> 
> ok

Applied.
  
> [...]
> > * v4l-vidiocmcapture-errmsg.patch: more consistent with the other
> >   ioctls error messages, no strong opinion on this though.
> 
> ok

Applied.

Regards.
-- 
FFmpeg = Faboulous and Fast Martial Picky Egregious Game
-------------- next part --------------
A non-text attachment was scrubbed...
Name: v4l-safer-size-check.patch
Type: text/x-diff
Size: 571 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20081227/8e5055eb/attachment.patch>