linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sylwester Nawrocki <sylvester.nawrocki@gmail.com>
To: Jean-Francois Moine <moinejf@free.fr>
Cc: linux-media@vger.kernel.org
Subject: Re: [PATCH/RFC v2 3/4] gspca: sonixj: Add V4L2_CID_JPEG_COMPRESSION_QUALITY control support
Date: Sat, 14 Jan 2012 18:42:20 +0100	[thread overview]
Message-ID: <4F11BE7C.3060601@gmail.com> (raw)
In-Reply-To: <20120114094720.781f89a5@tele>

On 01/14/2012 09:47 AM, Jean-Francois Moine wrote:
> On Fri,  6 Jan 2012 19:14:41 +0100
> Sylwester Nawrocki<snjw23@gmail.com>  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

  reply	other threads:[~2012-01-14 17:42 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-11-12 19:46 [RFC] JPEG encoders control class Sylwester Nawrocki
2011-11-26 18:43 ` Sakari Ailus
2011-11-26 20:59   ` Sylwester Nawrocki
2011-11-28 12:20 ` Hans Verkuil
2011-11-28 12:51   ` Sylwester Nawrocki
2011-11-28 18:48   ` Luca Risolia
2011-11-29 17:53     ` Jean-Francois Moine
2011-12-27 19:43 ` [RFC/PATCHv1 0/4] JPEG codecs " Sylwester Nawrocki
2011-12-27 19:43   ` [PATCH 1/4] V4L: Add JPEG compression " Sylwester Nawrocki
2011-12-30 21:42     ` Sakari Ailus
2011-12-31 11:54       ` Sylwester Nawrocki
2011-12-27 19:43   ` [PATCH 2/4] V4L: Add the JPEG compression control class documentation Sylwester Nawrocki
2011-12-27 19:43   ` [PATCH 3/4] gspca: sonixj: Add V4L2_CID_JPEG_COMPRESSION_QUALITY control support Sylwester Nawrocki
2011-12-27 19:43   ` [PATCH 4/4] gspca: zc3xx: " Sylwester Nawrocki
2012-01-06 18:14 ` [PATCH/RFC v2 0/4] JPEG codecs control class Sylwester Nawrocki
2012-01-06 18:14 ` [PATCH/RFC v2 1/4] V4L: Add JPEG compression " Sylwester Nawrocki
2012-01-06 18:14 ` [PATCH/RFC v2 2/4] V4L: Add JPEG compression control class documentation Sylwester Nawrocki
2012-01-06 18:14 ` [PATCH/RFC v2 3/4] gspca: sonixj: Add V4L2_CID_JPEG_COMPRESSION_QUALITY control support Sylwester Nawrocki
2012-01-14  8:47   ` Jean-Francois Moine
2012-01-14 17:42     ` Sylwester Nawrocki [this message]
2012-01-14 18:24       ` Jean-Francois Moine
2012-01-14 19:35         ` [PATCH/RFC v3 0/3] JPEG codecs control class Sylwester Nawrocki
2012-01-14 19:35           ` [PATCH/RFC v3 1/3] V4L: Add JPEG compression " Sylwester Nawrocki
2012-01-14 19:35           ` [PATCH/RFC v3 2/3] V4L: Add JPEG compression control class documentation Sylwester Nawrocki
2012-01-14 19:35           ` [PATCH/RFC v3 3/3] gspca: zc3xx: Add V4L2_CID_JPEG_COMPRESSION_QUALITY control support Sylwester Nawrocki
2012-01-14 19:53             ` [PATCH/RFC v4 " Sylwester Nawrocki
2012-01-21 14:45               ` Sylwester Nawrocki
2012-01-25 11:49                 ` Jean-Francois Moine
2012-01-14 20:07         ` [PATCH/RFC v2 3/4] gspca: sonixj: " Sylwester Nawrocki
2012-01-06 18:14 ` [PATCH/RFC v2 4/4] gspca: zc3xx: " Sylwester Nawrocki
2012-01-14  8:47   ` Jean-Francois Moine

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4F11BE7C.3060601@gmail.com \
    --to=sylvester.nawrocki@gmail.com \
    --cc=linux-media@vger.kernel.org \
    --cc=moinejf@free.fr \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).