Re: [PATCH/RFC v2 3/4] gspca: sonixj: Add V4L2_CID_JPEG_COMPRESSION_QUALITY control support
On 01/14/2012 07:24 PM, Jean-Francois Moine wrote: > > Setting the JPEG quality in sonixj has been removed when automatic > quality adjustment has been added (git commit b96cfc33e7). At this > time, I let the JPEG get function, but it could have been removed as > well: I don't think the users are interested by this quality, and the > applications may find it looking at the quantization tables of the > images. > > Otherwise, letting the users/applications to set this quality is > dangerous: if the quality is too high, the images cannot be fully > transmitted because their size is too big for the USB 1.1 channel. > > So, IMO, you should let the sonixj as it is, and I will remove the > get_jepgcomp. I see, indeed the quantization tables provide much more precise information. I've dropped the sonixj patch from the series then. -- Thanks, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC v2 3/4] gspca: sonixj: Add V4L2_CID_JPEG_COMPRESSION_QUALITY control support
On Sat, 14 Jan 2012 18:42:20 +0100 Sylwester Nawrocki wrote: > 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. Setting the JPEG quality in sonixj has been removed when automatic quality adjustment has been added (git commit b96cfc33e7). At this time, I let the JPEG get function, but it could have been removed as well: I don't think the users are interested by this quality, and the applications may find it looking at the quantization tables of the images. Otherwise, letting the users/applications to set this quality is dangerous: if the quality is too high, the images cannot be fully transmitted because their size is too big for the USB 1.1 channel. So, IMO, you should let the sonixj as it is, and I will remove the get_jepgcomp. -- Ken ar c'hentañ | ** Breizh ha Linux atav! ** Jef | http://moinejf.free.fr/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC v2 3/4] gspca: sonixj: Add V4L2_CID_JPEG_COMPRESSION_QUALITY control support
On 01/14/2012 09:47 AM, Jean-Francois Moine wrote: > On Fri, 6 Jan 2012 19:14:41 +0100 > Sylwester Nawrocki 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC v2 3/4] gspca: sonixj: Add V4L2_CID_JPEG_COMPRESSION_QUALITY control support
On Fri, 6 Jan 2012 19:14:41 +0100 Sylwester Nawrocki 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 -- Ken ar c'hentañ | ** Breizh ha Linux atav! ** Jef | http://moinejf.free.fr/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC v2 3/4] gspca: sonixj: Add V4L2_CID_JPEG_COMPRESSION_QUALITY control support
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.
Cc: Jean-Francois Moine
Signed-off-by: Sylwester Nawrocki
---
For completeness V4L2_CID_JPEG_ACTIVE_MARKER control might be
also added.
---
drivers/media/video/gspca/sonixj.c | 23 +++
1 files changed, 23 insertions(+), 0 deletions(-)
diff --git a/drivers/media/video/gspca/sonixj.c
b/drivers/media/video/gspca/sonixj.c
index c55d667..c148081 100644
--- a/drivers/media/video/gspca/sonixj.c
+++ b/drivers/media/video/gspca/sonixj.c
@@ -45,6 +45,7 @@ enum e_ctrl {
SHARPNESS,
ILLUM,
FREQ,
+ QUALITY,
NCTRLS /* number of controls */
};
@@ -126,6 +127,7 @@ static void qual_upd(struct work_struct *work);
#define DEF_EN 0x80/* defect pixel by 0: soft, 1: hard */
/* V4L2 controls supported by the driver */
+static int getjpegqual(struct gspca_dev *gspca_dev, s32 *val);
static void setbrightness(struct gspca_dev *gspca_dev);
static void setcontrast(struct gspca_dev *gspca_dev);
static void setcolors(struct gspca_dev *gspca_dev);
@@ -286,6 +288,19 @@ static const struct ctrl sd_ctrls[NCTRLS] = {
},
.set_control = setfreq
},
+[QUALITY] = {
+ {
+ .id = V4L2_CID_JPEG_COMPRESSION_QUALITY,
+ .type= V4L2_CTRL_TYPE_INTEGER,
+ .name= "Compression Quality",
+ .minimum = QUALITY_MIN,
+ .maximum = QUALITY_MAX,
+ .step= 1,
+ .default_value = QUALITY_DEF,
+ .flags = V4L2_CTRL_FLAG_READ_ONLY,
+ },
+ .get = getjpegqual
+ },
};
/* table of the disabled controls */
@@ -2960,6 +2975,14 @@ static int sd_get_jcomp(struct gspca_dev *gspca_dev,
return 0;
}
+static int getjpegqual(struct gspca_dev *gspca_dev, s32 *val)
+{
+ struct sd *sd = (struct sd *) gspca_dev;
+
+ *val = sd->quality;
+ return 0;
+}
+
static int sd_querymenu(struct gspca_dev *gspca_dev,
struct v4l2_querymenu *menu)
{
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
