Em Thu, 4 Jun 2009 14:39:52 +0200 Laurent Pinchart <laurent.pinchart@xxxxxxxxx> escreveu: > Mauro, > > Please pull from http://linuxtv.org/hg/~pinchartl/uvcvideo/ > > for the following 3 changesets: > > uvcvideo: Add generic control blacklist. > uvcvideo: Don't accept to change the format when buffers are allocated. > uvcvideo: Implement VIDIOC_[GS]_JPEGCOMP I have a few comments about the code of the last patch: + jpeg->quality = video->streaming->ctrl.wCompQuality / 100; Wouldn't be better to round it to the closest value instead of just truncating? + memcpy(&probe, &video->streaming->ctrl, sizeof probe); Please use sizeof(probe) instead. ERROR: do not use assignment in if condition #64: FILE: linux/drivers/media/video/uvc/uvc_v4l2.c:376: + if ((ret = uvc_probe_video(video, &probe)) < 0) ERROR: do not use assignment in if condition #80: FILE: linux/drivers/media/video/uvc/uvc_v4l2.c:872: + if ((ret = uvc_acquire_privileges(handle)) < 0) total: 2 errors, 0 warnings, 81 lines checked Please, one statement per line. - Also, I have a few comments, from API POV. It doesn't seem that those ioctls are properly implemented. There are some things there that sounded obfuscated for me, and it you aren't implementing. Probably because it is not clear enough. Also, we used to have a similar set of ioctls for MPEG, that were removed, in favor of the usage of extended controls, with a cleaner interface. That's said, instead of adding more support for this obfuscated API, maybe we could deprecate those in favor of some controls that could make more sense, and let vidio_ioctl2 provide backward compat for it by calling the proper controls, just to preserve binary compatibility with legacy applications. Cheers, Mauro -- 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
