Re: [PATCH v3] ov10635: Add OmniVision ov10635 SoC camera driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Guennadi,

> > +{
> > +   struct i2c_client *client = v4l2_get_subdevdata(sd);
> > +   struct ov10635_priv *priv = to_ov10635(client);
> > +   struct v4l2_captureparm *cp = &parms->parm.capture;
> > +   enum v4l2_mbus_pixelcode code;
> > +   int ret;
> > +
> > +   if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> > +      return -EINVAL;
> > +   if (cp->extendedmode != 0)
> > +      return -EINVAL;
> > +
> > +   /* FIXME Check we can handle the requested framerate */
> > +   priv->fps_denominator = cp->timeperframe.numerator;
> > +   priv->fps_numerator = cp->timeperframe.denominator;
> 
> Yes, fixing this could be a good idea :) Just add one parameter to your 
> set_params() and use NULL elsewhere.

There is one issue with setting the camera to achieve different framerate. 
The camera can work at up to 60fps with lower resolutions, i.e. when 
vertical sub-sampling is used. However, the API uses separate functions 
for changing resolution and framerate. So, userspace could use a low 
resolution, high framerate setting, then attempt to use a high resolution, 
low framerate setting. Clearly, it's possible for userspace to call s_fmt 
and s_parm in a way that attempts to set high resolution with the old 
(high) framerate. In this case, a check for valid settings will fail.

Is this a generally known issue and userspace works round it?

Thanks
Phil
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux