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