Re: [PATCH/RFC v2 3/4] gspca: sonixj: Add V4L2_CID_JPEG_COMPRESSION_QUALITY control support

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

 



On 01/14/2012 09:47 AM, Jean-Francois Moine wrote:
> On Fri,  6 Jan 2012 19:14:41 +0100
> Sylwester Nawrocki<snjw23@xxxxxxxxx>  wrote:
> 
>> The JPEG compression quality value can currently be read using the
>> VIDIOC_G_JPEGCOMP ioctl. As the quality field of struct v4l2_jpgecomp
>> is being deprecated, we add the V4L2_CID_JPEG_COMPRESSION_QUALITY
>> control, so after the deprecation period VIDIOC_G_JPEGCOMP ioctl
>> handler can be removed, leaving the control the only user interface
>> for retrieving the compression quality.
> 	[snip]
> 
> This patch works, but, to follow the general control mechanism in gspca,
> it should be better to remove the variable 'quality' of 'struct sd' and
> to replace all 'sd->quality' by 'sd->ctrls[QUALITY].val'.
> 
> Then, initialization
> 
> 	sd->quality = QUALITY_DEF;
> 
> in sd_config() is no more useful, and there is no need to have a
> getjpegqual() function, the control descriptor for QUALITY having just:
> 
> 	.set_control = setjpegqual

Thank you for reviewing the patches. I wasn't sure I fully understood
the locking, hence I left the 'quality' field in 'struct sd' not removed. 
I've modified both subdrivers according to your suggestions. I would have 
one question before I send the corrected patches though. Unlike zc3xx, 
the configured quality value in sonixj driver changes dynamically, i.e. 
it drifts away in few seconds from whatever value the user sets. This makes
me wonder if .set_control operation should be implemented for the QUALITY
control, and whether to leave V4L2_CTRL_FLAG_READ_ONLY control flag or not.

There seem to be not much value in setting such control for the user,
but maybe it's different for other devices covered by the sonixj driver.

--
Best regards,
Sylwester
--
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