[RFC] JPEG encoders control class

[RFC] JPEG encoders control class

[Sylwester Nawrocki]

Hi all,

This RFC is discussing the current support of JPEG encoders in V4L2 and 
a proposal of new JPEG control class.


Motivation
==========

JPEG encoder control is also required at the sub-device level, but currently 
there are only defined ioctls in regular V4L2 device API. It doesn't seem
to make sense for these current ioctls to be inherited by sub-device nodes, 
since they're not generic enough, incomplete and rather not compliant with
JFIF JPEG standard [2], [3].


Current implementation
======================

Currently two ioctls are available [4]:

#define VIDIOC_G_JPEGCOMP	 _IOR('V', 61, struct v4l2_jpegcompression)
#define VIDIOC_S_JPEGCOMP	 _IOW('V', 62, struct v4l2_jpegcompression)

And the corresponding data structure is defined as:

struct v4l2_jpegcompression {
	int quality;

	int  APPn;              /* Number of APP segment to be written,
				 * must be 0..15 */
	int  APP_len;           /* Length of data in JPEG APPn segment */
	char APP_data[60];      /* Data in the JPEG APPn segment. */

	int  COM_len;           /* Length of data in JPEG COM segment */
	char COM_data[60];      /* Data in JPEG COM segment */

	__u32 jpeg_markers;     /* Which markers should go into the JPEG
				 * output. Unless you exactly know what
				 * you do, leave them untouched.
				 * Inluding less markers will make the
				 * resulting code smaller, but there will
				 * be fewer applications which can read it.
				 * The presence of the APP and COM marker
				 * is influenced by APP_len and COM_len
				 * ONLY, not by this property! */

#define V4L2_JPEG_MARKER_DHT (1<<3)    /* Define Huffman Tables */
#define V4L2_JPEG_MARKER_DQT (1<<4)    /* Define Quantization Tables */
#define V4L2_JPEG_MARKER_DRI (1<<5)    /* Define Restart Interval */
#define V4L2_JPEG_MARKER_COM (1<<6)    /* Comment segment */
#define V4L2_JPEG_MARKER_APP (1<<7)    /* App segment, driver will
					* allways use APP0 */
};


What are the issues with such an implementation ?

These ioctls don't allow to re-program the quantization and Huffman tables 
(DQT, DHT). Additionally, the standard valid segment length for the application
defined APPn and the comment COM segment is 2...65535, while currently this is
limited to 60 bytes.

Therefore APP_data and COM_data, rather than fixed size arrays, should be 
pointers to a variable length buffer.

Only two drivers upstream really use VIDIOC_[S/G]_JPEGCOMP ioctls for anything
more than compression quality query/control. It might make sense to create 
separate control for image quality and to obsolete the v4l2_jpegcompressin::quality 
field.

Below is a brief review of usage of VIDIOC_[S/G]_JPEGCOMP ioctls in current mainline 
drivers. Listed are parts of struct v4l2_jpegcompression used in each case.


cpia2
-----

vidioc_g_jpegcomp, vidioc_s_jpegcomp
- compression quality ignored, returns fixed value (80)
- uses APP_data/len, COM_data/len
- markers (only DHT can be disabled by the applications) 


zoran
-----

vidioc_g_jpegcomp, vidioc_s_jpegcomp
- compression quality, values 5...100, used only to calculate buffer size
- APP_data/len, COM_data/len
- markers field used to control inclusion of selected JPEG markers
  in the output buffer


et61x251, sn9c102, s2255drv.c
-----------------------------

vidioc_g_jpegcomp, vidioc_s_jpegcomp 
- compression quality only, 
  valid values: et61x251, sn9c102use - {0, 1}, s2255drv.c - (0..100)


staging/media/go7007
--------------------

vidioc_g_jpegcomp
- only for reporting JPEG markers (_DHT and _DQT returned),
- always returns fixed value of compression quality (50)

vidioc_s_jpegcomp
 - does nothing, only returns error code when passed parameter
   do not match the device capabilities


drivers/media/video/gspca/conex.c  
drivers/media/video/gspca/jeilinj.c
drivers/media/video/gspca/mars.c
drivers/media/video/gspca/ov519.c
drivers/media/video/gspca/spca500.c
drivers/media/video/gspca/stk014.c
drivers/media/video/gspca/sunplus.c
drivers/media/video/gspca/topro.c
drivers/media/video/gspca/zc3xx.c
------------------------------------

vidioc_s_jpegcomp
- compression quality

vidioc_g_jpegcomp
 - compression quality, marker flags


drivers/media/video/gspca/sonixj.c
------------------------------------

vidioc_g_jpegcomp
 - compression quality, marker flags
 
--------------------------------------

The following is an initial draft of the new control class

o V4L2_CTRL_CLASS_JPEG

As not everything might be covered by the controls (the application data and comment
segments, quantization and Huffman tables, etc.) the control class should probably
just complement VIDIOC_[G/S]_JPEGCOMP ioctls, rather than entirely replacing them.


Proposed controls
=================

1. Chroma sub-sampling
---------------------

The subsampling factors describe how each component of an input image is sampled,
in respect to maximum sample rate in each spatial dimension.
More general description can be found in [2], clause A.1.1., "Dimensions and 
sampling factors".

The chroma subsampling would describe how Cb, Cr components should be downsampled
after coverting an input image from RGB to Y'CbCr color space.

o V4L2_CID_JPEG_CHROMA_SUBSAMPLING

  - V4L2_JPEG_CHROMA_SUBSAMPLING_GRAY - only luminance component is present,
  - V4L2_JPEG_CHROMA_SUBSAMPLING_410  - subsample Cr, Cb signals horizontally by
                                        4 and vertically by 2
  - V4L2_JPEG_CHROMA_SUBSAMPLING_411  - horizontally subsample Cr, Cb signals by
                                        a factor of 4
  - V4L2_JPEG_CHROMA_SUBSAMPLING_420  - subsample Cr, Cb signals horizontally and
                                        vertically by 2
  - V4L2_JPEG_CHROMA_SUBSAMPLING_422  - horizontally subsample Cr, Cb signals by
                                        a factor of 2,
  - V4L2_JPEG_CHROMA_SUBSAMPLING_444  - no chroma subsampling, each pixel has Y, 
                                        Cr and Cb values.

Using no subsampling produces sharper colours, even with higher compression
(in order to achieve similar file size) [7], thus it seems important to provide 
the user with a method of precise subsampling control. 


2. Restart interval (DRI)
-----------------------

o V4L2_CID_JPEG_RESTART_INTERVAL

The restart interval (DRI marker) determines the interval of inserting RSTm
markers. The purpose of RSTm markers is to additionally reinitialize decoder 
process' predictor with initial default value. For lossy compression processes
the restart interval is expressed in MCU (Minimm Coded Unit).
If restart interval value is 0 DRI and RSTm (m = 0..7) markers will not be 
inserted. Consequently this control would make current V4L2_JPEG_MARKER_DRI 
markers flag redundant. This control would be useful for S5P JPEG IP block [6].


3. Image quality
----------------

o V4L2_CID_JPEG_QUALITY	

Image quality is not defined in the standard but it is used as an intermediate 
parameter by many encoders to control set of encoding parameters, which then 
allow to obtain certain image quality and corresponding file size.
IMHO it makes sense to add the quality control to the JPEG class as it's widely
used, not only for webcams. 

As far as the value range is concerned, probably it's better to leave this driver
specific. The applications would then be more aware of what is supported by 
a device (min, max, step) and they could translate driver specific range into 
standardised values (0..100) if needed. Still the drivers could do the translation
themselves if required. The specification would only say the 0..100 range is 
recommended.


4. JPEG markers presence
------------------------

Markers serve as identifiers of various structural parts of compressed data 
formats. All markers are assigned two-byte codes: an 0xFF byte followed by 
a byte which is not equal to 0 or 0xFF. [2] Excluding the reserved ones there
is 39 valid codes.

I'm not really sure how the markers inhibit feature is useful, but since some
drivers use it let's assume it is needed. Likely a 32-bit bitmask control could
be used for activating/deactivating markers, as it doesn't make sense for some 
of markers to be freely discarded from the compressed data.

o V4L2_CID_JPEG_ACTIVE_MARKERS

Following markers might be covered by this control, listed in Table E.1, [2]: 
APP0..15, COM, DHT, DQT, DAC and additionally DNL. 
There is still room for 10 additional markers which might be added if required.


The above list of controls is most likely not exhaustive, it's just an attempt
to cover features available in the mainline drivers and the S5P SoC JPEG codec
IP block [6].

In order to support reconfiguration of quantization and Huffman tables the 
VIDIOC_[G/S]_JPEGCOMP probably need to be re-designed, but it's out of scope
of this RFC. 


References
==========

[1] http://www.mail-archive.com/linux-media@vger.kernel.org/msg01783.html

[2] http://www.w3.org/Graphics/JPEG/itu-t81.pdf

[3] http://www.w3.org/Graphics/JPEG/jfif3.pdf

[4] http://linuxtv.org/downloads/v4l-dvb-apis/vidioc-g-jpegcomp.html

[5] http://www.mail-archive.com/linux-media@vger.kernel.org/msg01784.html

[6] http://patchwork.linuxtv.org/patch/8197

[7] http://www.ampsoft.net/webdesign-l/jpeg-compression.html


--
Thanks,
Sylwester

Re: [RFC] JPEG encoders control class

[Sakari Ailus]

Hi Sylwester,

Thanks for the RFC. I originally missed it; thanks for pointing to it!

On Sat, Nov 12, 2011 at 08:46:25PM +0100, Sylwester Nawrocki wrote:
> Hi all,
> 
> This RFC is discussing the current support of JPEG encoders in V4L2 and 
> a proposal of new JPEG control class.
> 
> 
> Motivation
> ==========
> 
> JPEG encoder control is also required at the sub-device level, but currently 
> there are only defined ioctls in regular V4L2 device API. It doesn't seem
> to make sense for these current ioctls to be inherited by sub-device nodes, 
> since they're not generic enough, incomplete and rather not compliant with
> JFIF JPEG standard [2], [3].
> 
> 
> Current implementation
> ======================
> 
> Currently two ioctls are available [4]:
> 
> #define VIDIOC_G_JPEGCOMP	 _IOR('V', 61, struct v4l2_jpegcompression)
> #define VIDIOC_S_JPEGCOMP	 _IOW('V', 62, struct v4l2_jpegcompression)
> 
> And the corresponding data structure is defined as:
> 
> struct v4l2_jpegcompression {
> 	int quality;
> 
> 	int  APPn;              /* Number of APP segment to be written,
> 				 * must be 0..15 */
> 	int  APP_len;           /* Length of data in JPEG APPn segment */
> 	char APP_data[60];      /* Data in the JPEG APPn segment. */
> 
> 	int  COM_len;           /* Length of data in JPEG COM segment */
> 	char COM_data[60];      /* Data in JPEG COM segment */
> 
> 	__u32 jpeg_markers;     /* Which markers should go into the JPEG
> 				 * output. Unless you exactly know what
> 				 * you do, leave them untouched.
> 				 * Inluding less markers will make the
> 				 * resulting code smaller, but there will
> 				 * be fewer applications which can read it.
> 				 * The presence of the APP and COM marker
> 				 * is influenced by APP_len and COM_len
> 				 * ONLY, not by this property! */
> 
> #define V4L2_JPEG_MARKER_DHT (1<<3)    /* Define Huffman Tables */
> #define V4L2_JPEG_MARKER_DQT (1<<4)    /* Define Quantization Tables */
> #define V4L2_JPEG_MARKER_DRI (1<<5)    /* Define Restart Interval */
> #define V4L2_JPEG_MARKER_COM (1<<6)    /* Comment segment */
> #define V4L2_JPEG_MARKER_APP (1<<7)    /* App segment, driver will
> 					* allways use APP0 */
> };
> 
> 
> What are the issues with such an implementation ?
> 
> These ioctls don't allow to re-program the quantization and Huffman tables 
> (DQT, DHT). Additionally, the standard valid segment length for the application
> defined APPn and the comment COM segment is 2...65535, while currently this is
> limited to 60 bytes.
> 
> Therefore APP_data and COM_data, rather than fixed size arrays, should be 
> pointers to a variable length buffer.
> 
> Only two drivers upstream really use VIDIOC_[S/G]_JPEGCOMP ioctls for anything
> more than compression quality query/control. It might make sense to create 
> separate control for image quality and to obsolete the v4l2_jpegcompressin::quality 
> field.
> 
> Below is a brief review of usage of VIDIOC_[S/G]_JPEGCOMP ioctls in current mainline 
> drivers. Listed are parts of struct v4l2_jpegcompression used in each case.
> 
> 
> cpia2
> -----
> 
> vidioc_g_jpegcomp, vidioc_s_jpegcomp
> - compression quality ignored, returns fixed value (80)
> - uses APP_data/len, COM_data/len
> - markers (only DHT can be disabled by the applications) 
> 
> 
> zoran
> -----
> 
> vidioc_g_jpegcomp, vidioc_s_jpegcomp
> - compression quality, values 5...100, used only to calculate buffer size
> - APP_data/len, COM_data/len
> - markers field used to control inclusion of selected JPEG markers
>   in the output buffer
> 
> 
> et61x251, sn9c102, s2255drv.c
> -----------------------------
> 
> vidioc_g_jpegcomp, vidioc_s_jpegcomp 
> - compression quality only, 
>   valid values: et61x251, sn9c102use - {0, 1}, s2255drv.c - (0..100)
> 
> 
> staging/media/go7007
> --------------------
> 
> vidioc_g_jpegcomp
> - only for reporting JPEG markers (_DHT and _DQT returned),
> - always returns fixed value of compression quality (50)
> 
> vidioc_s_jpegcomp
>  - does nothing, only returns error code when passed parameter
>    do not match the device capabilities
> 
> 
> drivers/media/video/gspca/conex.c  
> drivers/media/video/gspca/jeilinj.c
> drivers/media/video/gspca/mars.c
> drivers/media/video/gspca/ov519.c
> drivers/media/video/gspca/spca500.c
> drivers/media/video/gspca/stk014.c
> drivers/media/video/gspca/sunplus.c
> drivers/media/video/gspca/topro.c
> drivers/media/video/gspca/zc3xx.c
> ------------------------------------
> 
> vidioc_s_jpegcomp
> - compression quality
> 
> vidioc_g_jpegcomp
>  - compression quality, marker flags
> 
> 
> drivers/media/video/gspca/sonixj.c
> ------------------------------------
> 
> vidioc_g_jpegcomp
>  - compression quality, marker flags
>  
> --------------------------------------
> 
> The following is an initial draft of the new control class
> 
> o V4L2_CTRL_CLASS_JPEG
> 
> As not everything might be covered by the controls (the application data and comment
> segments, quantization and Huffman tables, etc.) the control class should probably
> just complement VIDIOC_[G/S]_JPEGCOMP ioctls, rather than entirely replacing them.

I wonder if there would be enough benefits in having blob controls. The
challenge is how to describe the format of the data in a standard and
flexible way and also what those bits actually signify. We will have lots of
table format data when the image processing functionality (much of which is
hardware-dependent) in ISPs get better supported.

FYI: you have over 80 characters per line. It's hard to read without
rewrapping.

> Proposed controls
> =================
> 
> 1. Chroma sub-sampling
> ---------------------
> 
> The subsampling factors describe how each component of an input image is sampled,
> in respect to maximum sample rate in each spatial dimension.
> More general description can be found in [2], clause A.1.1., "Dimensions and 
> sampling factors".
> 
> The chroma subsampling would describe how Cb, Cr components should be downsampled
> after coverting an input image from RGB to Y'CbCr color space.
> 
> o V4L2_CID_JPEG_CHROMA_SUBSAMPLING
> 
>   - V4L2_JPEG_CHROMA_SUBSAMPLING_GRAY - only luminance component is present,
>   - V4L2_JPEG_CHROMA_SUBSAMPLING_410  - subsample Cr, Cb signals horizontally by
>                                         4 and vertically by 2
>   - V4L2_JPEG_CHROMA_SUBSAMPLING_411  - horizontally subsample Cr, Cb signals by
>                                         a factor of 4
>   - V4L2_JPEG_CHROMA_SUBSAMPLING_420  - subsample Cr, Cb signals horizontally and
>                                         vertically by 2
>   - V4L2_JPEG_CHROMA_SUBSAMPLING_422  - horizontally subsample Cr, Cb signals by
>                                         a factor of 2,
>   - V4L2_JPEG_CHROMA_SUBSAMPLING_444  - no chroma subsampling, each pixel has Y, 
>                                         Cr and Cb values.
> 
> Using no subsampling produces sharper colours, even with higher compression
> (in order to achieve similar file size) [7], thus it seems important to provide 
> the user with a method of precise subsampling control. 
> 
> 
> 2. Restart interval (DRI)
> -----------------------
> 
> o V4L2_CID_JPEG_RESTART_INTERVAL
> 
> The restart interval (DRI marker) determines the interval of inserting RSTm
> markers. The purpose of RSTm markers is to additionally reinitialize decoder 
> process' predictor with initial default value. For lossy compression processes
> the restart interval is expressed in MCU (Minimm Coded Unit).
> If restart interval value is 0 DRI and RSTm (m = 0..7) markers will not be 
> inserted. Consequently this control would make current V4L2_JPEG_MARKER_DRI 
> markers flag redundant. This control would be useful for S5P JPEG IP block [6].
> 
> 
> 3. Image quality
> ----------------
> 
> o V4L2_CID_JPEG_QUALITY	
> 
> Image quality is not defined in the standard but it is used as an intermediate 
> parameter by many encoders to control set of encoding parameters, which then 
> allow to obtain certain image quality and corresponding file size.
> IMHO it makes sense to add the quality control to the JPEG class as it's widely
> used, not only for webcams. 
> 
> As far as the value range is concerned, probably it's better to leave this driver
> specific. The applications would then be more aware of what is supported by 
> a device (min, max, step) and they could translate driver specific range into 
> standardised values (0..100) if needed. Still the drivers could do the translation
> themselves if required. The specification would only say the 0..100 range is 
> recommended.

I think it's best to leave this hardware specific. Granularity might matter
sometimes in the user space. As the step is enforced (which is correct), it
may be impossible to support the span 0--100 with desired granularity.

> 4. JPEG markers presence
> ------------------------
> 
> Markers serve as identifiers of various structural parts of compressed data 
> formats. All markers are assigned two-byte codes: an 0xFF byte followed by 
> a byte which is not equal to 0 or 0xFF. [2] Excluding the reserved ones there
> is 39 valid codes.
> 
> I'm not really sure how the markers inhibit feature is useful, but since some
> drivers use it let's assume it is needed. Likely a 32-bit bitmask control could
> be used for activating/deactivating markers, as it doesn't make sense for some 
> of markers to be freely discarded from the compressed data.
> 
> o V4L2_CID_JPEG_ACTIVE_MARKERS
> 
> Following markers might be covered by this control, listed in Table E.1, [2]: 
> APP0..15, COM, DHT, DQT, DAC and additionally DNL. 
> There is still room for 10 additional markers which might be added if required.
> 
> 
> The above list of controls is most likely not exhaustive, it's just an attempt
> to cover features available in the mainline drivers and the S5P SoC JPEG codec
> IP block [6].
> 
> In order to support reconfiguration of quantization and Huffman tables the 
> VIDIOC_[G/S]_JPEGCOMP probably need to be re-designed, but it's out of scope
> of this RFC. 

Is this something that a regular user might typically want to do?

Also, if you keep adding more functionality to a single structure like that
it forces user to get all the other parameters (s)he doesn't want to change
before issuing the set operation. I don't particularly like that idea.

> References
> ==========
> 
> [1] http://www.mail-archive.com/linux-media@vger.kernel.org/msg01783.html
> 
> [2] http://www.w3.org/Graphics/JPEG/itu-t81.pdf
> 
> [3] http://www.w3.org/Graphics/JPEG/jfif3.pdf
> 
> [4] http://linuxtv.org/downloads/v4l-dvb-apis/vidioc-g-jpegcomp.html
> 
> [5] http://www.mail-archive.com/linux-media@vger.kernel.org/msg01784.html
> 
> [6] http://patchwork.linuxtv.org/patch/8197
> 
> [7] http://www.ampsoft.net/webdesign-l/jpeg-compression.html

Kind regards,

-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi	jabber/XMPP/Gmail: sailus@retiisi.org.uk

Re: [RFC] JPEG encoders control class

[Sylwester Nawrocki]

Hi Sakari,

On 11/26/2011 07:43 PM, Sakari Ailus wrote:
> Hi Sylwester,
> 
> Thanks for the RFC. I originally missed it; thanks for pointing to it!

Thanks for your comments!

> 
> On Sat, Nov 12, 2011 at 08:46:25PM +0100, Sylwester Nawrocki wrote:
>> Hi all,
>>
>> This RFC is discussing the current support of JPEG encoders in V4L2 and
>> a proposal of new JPEG control class.
>>
...
>>
>> The following is an initial draft of the new control class
>>
>> o V4L2_CTRL_CLASS_JPEG
>>
>> As not everything might be covered by the controls (the application data and comment
>> segments, quantization and Huffman tables, etc.) the control class should probably
>> just complement VIDIOC_[G/S]_JPEGCOMP ioctls, rather than entirely replacing them.
> 
> I wonder if there would be enough benefits in having blob controls. The
> challenge is how to describe the format of the data in a standard and
> flexible way and also what those bits actually signify. We will have lots of
> table format data when the image processing functionality (much of which is
> hardware-dependent) in ISPs get better supported.

If the table data is parametrized and the parameters are not inside the table
then it's indeed not an easy task to provide a generic meta-data format.
 
In case of JPEG the format of tables/markers/segments is well defined and 
AFAICT single binary blob ID and its length might be all what's needed
for the applications and drivers to interpret the data.

> 
> FYI: you have over 80 characters per line. It's hard to read without
> rewrapping.

Ups, sorry. I have just turned off word wrapping in Thunderbird completely 
as it was extremely annoying and I've lost a bit the 80-character sense :/  
Thank you for the notice.

> 
>> Proposed controls
>> =================
>>
>> 1. Chroma sub-sampling
>> ---------------------
>>
>> The subsampling factors describe how each component of an input image is sampled,
>> in respect to maximum sample rate in each spatial dimension.
>> More general description can be found in [2], clause A.1.1., "Dimensions and
>> sampling factors".
>>
>> The chroma subsampling would describe how Cb, Cr components should be downsampled
>> after converting an input image from RGB to Y'CbCr color space.
>>
>> o V4L2_CID_JPEG_CHROMA_SUBSAMPLING
>>
>>    - V4L2_JPEG_CHROMA_SUBSAMPLING_GRAY - only luminance component is present,
>>    - V4L2_JPEG_CHROMA_SUBSAMPLING_410  - subsample Cr, Cb signals horizontally by
>>                                          4 and vertically by 2
>>    - V4L2_JPEG_CHROMA_SUBSAMPLING_411  - horizontally subsample Cr, Cb signals by
>>                                          a factor of 4
>>    - V4L2_JPEG_CHROMA_SUBSAMPLING_420  - subsample Cr, Cb signals horizontally and
>>                                          vertically by 2
>>    - V4L2_JPEG_CHROMA_SUBSAMPLING_422  - horizontally subsample Cr, Cb signals by
>>                                          a factor of 2,
>>    - V4L2_JPEG_CHROMA_SUBSAMPLING_444  - no chroma subsampling, each pixel has Y,
>>                                          Cr and Cb values.
>>
>> Using no subsampling produces sharper colours, even with higher compression
>> (in order to achieve similar file size) [7], thus it seems important to provide
>> the user with a method of precise subsampling control.
>>
>>
>> 2. Restart interval (DRI)
>> -----------------------
>>
>> o V4L2_CID_JPEG_RESTART_INTERVAL
>>
>> The restart interval (DRI marker) determines the interval of inserting RSTm
>> markers. The purpose of RSTm markers is to additionally reinitialize decoder
>> process' predictor with initial default value. For lossy compression processes
>> the restart interval is expressed in MCU (Minimm Coded Unit).
>> If restart interval value is 0 DRI and RSTm (m = 0..7) markers will not be
>> inserted. Consequently this control would make current V4L2_JPEG_MARKER_DRI
>> markers flag redundant. This control would be useful for S5P JPEG IP block [6].
>>
>>
>> 3. Image quality
>> ----------------
>>
>> o V4L2_CID_JPEG_QUALITY	
>>
>> Image quality is not defined in the standard but it is used as an intermediate
>> parameter by many encoders to control set of encoding parameters, which then
>> allow to obtain certain image quality and corresponding file size.
>> IMHO it makes sense to add the quality control to the JPEG class as it's widely
>> used, not only for webcams.
>>
>> As far as the value range is concerned, probably it's better to leave this driver
>> specific. The applications would then be more aware of what is supported by
>> a device (min, max, step) and they could translate driver specific range into
>> standardised values (0..100) if needed. Still the drivers could do the translation
>> themselves if required. The specification would only say the 0..100 range is
>> recommended.
> 
> I think it's best to leave this hardware specific. Granularity might matter
> sometimes in the user space. As the step is enforced (which is correct), it
> may be impossible to support the span 0--100 with desired granularity.

Agreed. So we leave it hardware specific, deferring the translation to 0..100
range to the applications.

> 
>> 4. JPEG markers presence
>> ------------------------
>>
>> Markers serve as identifiers of various structural parts of compressed data
>> formats. All markers are assigned two-byte codes: an 0xFF byte followed by
>> a byte which is not equal to 0 or 0xFF. [2] Excluding the reserved ones there
>> is 39 valid codes.
>>
>> I'm not really sure how the markers inhibit feature is useful, but since some
>> drivers use it let's assume it is needed. Likely a 32-bit bitmask control could
>> be used for activating/deactivating markers, as it doesn't make sense for some
>> of markers to be freely discarded from the compressed data.
>>
>> o V4L2_CID_JPEG_ACTIVE_MARKERS
>>
>> Following markers might be covered by this control, listed in Table E.1, [2]:
>> APP0..15, COM, DHT, DQT, DAC and additionally DNL.
>> There is still room for 10 additional markers which might be added if required.
>>
>>
>> The above list of controls is most likely not exhaustive, it's just an attempt
>> to cover features available in the mainline drivers and the S5P SoC JPEG codec
>> IP block [6].
>>
>> In order to support reconfiguration of quantization and Huffman tables the
>> VIDIOC_[G/S]_JPEGCOMP probably need to be re-designed, but it's out of scope
>> of this RFC.
> 
> Is this something that a regular user might typically want to do?

I don't think so. It would be rather an interface for libraries, since this is 
more or less hardware specific data.

> 
> Also, if you keep adding more functionality to a single structure like that
> it forces user to get all the other parameters (s)he doesn't want to change
> before issuing the set operation. I don't particularly like that idea.

Agreed. I thought about making the ioctl more v4l2 control like, i.e. make
it handle one data segment at time, with an ID field at the data structure 
indicating what standard data segment type is handled. Something like this 
could be possibly covered by a binary blob controls, if such have existed.

--
Thanks,
Sylwester

Re: [RFC] JPEG encoders control class

[Hans Verkuil]

On Saturday 12 November 2011 20:46:25 Sylwester Nawrocki wrote:
> Hi all,
> 
> This RFC is discussing the current support of JPEG encoders in V4L2 and
> a proposal of new JPEG control class.
> 
> 
> Motivation
> ==========
> 
> JPEG encoder control is also required at the sub-device level, but
> currently there are only defined ioctls in regular V4L2 device API. It
> doesn't seem to make sense for these current ioctls to be inherited by
> sub-device nodes, since they're not generic enough, incomplete and rather
> not compliant with JFIF JPEG standard [2], [3].
> 
> 
> Current implementation
> ======================
> 
> Currently two ioctls are available [4]:
> 
> #define VIDIOC_G_JPEGCOMP	 _IOR('V', 61, struct v4l2_jpegcompression)
> #define VIDIOC_S_JPEGCOMP	 _IOW('V', 62, struct v4l2_jpegcompression)
> 
> And the corresponding data structure is defined as:
> 
> struct v4l2_jpegcompression {
> 	int quality;
> 
> 	int  APPn;              /* Number of APP segment to be written,
> 				 * must be 0..15 */
> 	int  APP_len;           /* Length of data in JPEG APPn segment */
> 	char APP_data[60];      /* Data in the JPEG APPn segment. */
> 
> 	int  COM_len;           /* Length of data in JPEG COM segment */
> 	char COM_data[60];      /* Data in JPEG COM segment */
> 
> 	__u32 jpeg_markers;     /* Which markers should go into the JPEG
> 				 * output. Unless you exactly know what
> 				 * you do, leave them untouched.
> 				 * Inluding less markers will make the
> 				 * resulting code smaller, but there will
> 				 * be fewer applications which can read it.
> 				 * The presence of the APP and COM marker
> 				 * is influenced by APP_len and COM_len
> 				 * ONLY, not by this property! */
> 
> #define V4L2_JPEG_MARKER_DHT (1<<3)    /* Define Huffman Tables */
> #define V4L2_JPEG_MARKER_DQT (1<<4)    /* Define Quantization Tables */
> #define V4L2_JPEG_MARKER_DRI (1<<5)    /* Define Restart Interval */
> #define V4L2_JPEG_MARKER_COM (1<<6)    /* Comment segment */
> #define V4L2_JPEG_MARKER_APP (1<<7)    /* App segment, driver will
> 					* allways use APP0 */
> };
> 
> 
> What are the issues with such an implementation ?
> 
> These ioctls don't allow to re-program the quantization and Huffman tables
> (DQT, DHT). Additionally, the standard valid segment length for the
> application defined APPn and the comment COM segment is 2...65535, while
> currently this is limited to 60 bytes.
> 
> Therefore APP_data and COM_data, rather than fixed size arrays, should be
> pointers to a variable length buffer.
> 
> Only two drivers upstream really use VIDIOC_[S/G]_JPEGCOMP ioctls for
> anything more than compression quality query/control. It might make sense
> to create separate control for image quality and to obsolete the
> v4l2_jpegcompressin::quality field.
> 
> Below is a brief review of usage of VIDIOC_[S/G]_JPEGCOMP ioctls in current
> mainline drivers. Listed are parts of struct v4l2_jpegcompression used in
> each case.
> 
> 
> cpia2
> -----
> 
> vidioc_g_jpegcomp, vidioc_s_jpegcomp
> - compression quality ignored, returns fixed value (80)
> - uses APP_data/len, COM_data/len
> - markers (only DHT can be disabled by the applications)
> 
> 
> zoran
> -----
> 
> vidioc_g_jpegcomp, vidioc_s_jpegcomp
> - compression quality, values 5...100, used only to calculate buffer size
> - APP_data/len, COM_data/len
> - markers field used to control inclusion of selected JPEG markers
>   in the output buffer
> 
> 
> et61x251, sn9c102, s2255drv.c
> -----------------------------

Note that et61x251 and sn9c102 are going to be deprecated and removed at some
time in the future (gspca will support these devices).

> 
> vidioc_g_jpegcomp, vidioc_s_jpegcomp
> - compression quality only,
>   valid values: et61x251, sn9c102use - {0, 1}, s2255drv.c - (0..100)
> 
> 
> staging/media/go7007
> --------------------
> 
> vidioc_g_jpegcomp
> - only for reporting JPEG markers (_DHT and _DQT returned),
> - always returns fixed value of compression quality (50)
> 
> vidioc_s_jpegcomp
>  - does nothing, only returns error code when passed parameter
>    do not match the device capabilities
> 
> 
> drivers/media/video/gspca/conex.c
> drivers/media/video/gspca/jeilinj.c
> drivers/media/video/gspca/mars.c
> drivers/media/video/gspca/ov519.c
> drivers/media/video/gspca/spca500.c
> drivers/media/video/gspca/stk014.c
> drivers/media/video/gspca/sunplus.c
> drivers/media/video/gspca/topro.c
> drivers/media/video/gspca/zc3xx.c
> ------------------------------------
> 
> vidioc_s_jpegcomp
> - compression quality
> 
> vidioc_g_jpegcomp
>  - compression quality, marker flags
> 
> 
> drivers/media/video/gspca/sonixj.c
> ------------------------------------
> 
> vidioc_g_jpegcomp
>  - compression quality, marker flags
> 
> --------------------------------------
> 
> The following is an initial draft of the new control class
> 
> o V4L2_CTRL_CLASS_JPEG
> 
> As not everything might be covered by the controls (the application data
> and comment segments, quantization and Huffman tables, etc.) the control
> class should probably just complement VIDIOC_[G/S]_JPEGCOMP ioctls, rather
> than entirely replacing them.
> 
> 
> Proposed controls
> =================
> 
> 1. Chroma sub-sampling
> ---------------------
> 
> The subsampling factors describe how each component of an input image is
> sampled, in respect to maximum sample rate in each spatial dimension.
> More general description can be found in [2], clause A.1.1., "Dimensions
> and sampling factors".
> 
> The chroma subsampling would describe how Cb, Cr components should be
> downsampled after coverting an input image from RGB to Y'CbCr color space.
> 
> o V4L2_CID_JPEG_CHROMA_SUBSAMPLING
> 
>   - V4L2_JPEG_CHROMA_SUBSAMPLING_GRAY - only luminance component is
> present, - V4L2_JPEG_CHROMA_SUBSAMPLING_410  - subsample Cr, Cb signals
> horizontally by 4 and vertically by 2
>   - V4L2_JPEG_CHROMA_SUBSAMPLING_411  - horizontally subsample Cr, Cb
> signals by a factor of 4
>   - V4L2_JPEG_CHROMA_SUBSAMPLING_420  - subsample Cr, Cb signals
> horizontally and vertically by 2
>   - V4L2_JPEG_CHROMA_SUBSAMPLING_422  - horizontally subsample Cr, Cb
> signals by a factor of 2,
>   - V4L2_JPEG_CHROMA_SUBSAMPLING_444  - no chroma subsampling, each pixel
> has Y, Cr and Cb values.
> 
> Using no subsampling produces sharper colours, even with higher compression
> (in order to achieve similar file size) [7], thus it seems important to
> provide the user with a method of precise subsampling control.
> 
> 
> 2. Restart interval (DRI)
> -----------------------
> 
> o V4L2_CID_JPEG_RESTART_INTERVAL
> 
> The restart interval (DRI marker) determines the interval of inserting RSTm
> markers. The purpose of RSTm markers is to additionally reinitialize
> decoder process' predictor with initial default value. For lossy
> compression processes the restart interval is expressed in MCU (Minimm
> Coded Unit).
> If restart interval value is 0 DRI and RSTm (m = 0..7) markers will not be
> inserted. Consequently this control would make current V4L2_JPEG_MARKER_DRI
> markers flag redundant. This control would be useful for S5P JPEG IP block
> [6].
> 
> 
> 3. Image quality
> ----------------
> 
> o V4L2_CID_JPEG_QUALITY
> 
> Image quality is not defined in the standard but it is used as an
> intermediate parameter by many encoders to control set of encoding
> parameters, which then allow to obtain certain image quality and
> corresponding file size. IMHO it makes sense to add the quality control to
> the JPEG class as it's widely used, not only for webcams.
> 
> As far as the value range is concerned, probably it's better to leave this
> driver specific. The applications would then be more aware of what is
> supported by a device (min, max, step) and they could translate driver
> specific range into standardised values (0..100) if needed. Still the
> drivers could do the translation themselves if required. The specification
> would only say the 0..100 range is recommended.

It should also say that higher numbers must represent better quality.

> 
> 
> 4. JPEG markers presence
> ------------------------
> 
> Markers serve as identifiers of various structural parts of compressed data
> formats. All markers are assigned two-byte codes: an 0xFF byte followed by
> a byte which is not equal to 0 or 0xFF. [2] Excluding the reserved ones
> there is 39 valid codes.
> 
> I'm not really sure how the markers inhibit feature is useful, but since
> some drivers use it let's assume it is needed. Likely a 32-bit bitmask
> control could be used for activating/deactivating markers, as it doesn't
> make sense for some of markers to be freely discarded from the compressed
> data.
> 
> o V4L2_CID_JPEG_ACTIVE_MARKERS
> 
> Following markers might be covered by this control, listed in Table E.1,
> [2]: APP0..15, COM, DHT, DQT, DAC and additionally DNL.
> There is still room for 10 additional markers which might be added if
> required.

Are there limitation on the contents of the COM field? I.e., can it contain
'\0' values? If not, then it can perhaps be represented by a string control.

> 
> 
> The above list of controls is most likely not exhaustive, it's just an
> attempt to cover features available in the mainline drivers and the S5P
> SoC JPEG codec IP block [6].
> 
> In order to support reconfiguration of quantization and Huffman tables the
> VIDIOC_[G/S]_JPEGCOMP probably need to be re-designed, but it's out of
> scope of this RFC.

Overall I'm pleased to see this RFC. The JPEGCOMP ioctls are poorly designed
and having a well-designed replacement is long overdue.

Regards,

	Hans

> 
> 
> References
> ==========
> 
> [1] http://www.mail-archive.com/linux-media@vger.kernel.org/msg01783.html
> 
> [2] http://www.w3.org/Graphics/JPEG/itu-t81.pdf
> 
> [3] http://www.w3.org/Graphics/JPEG/jfif3.pdf
> 
> [4] http://linuxtv.org/downloads/v4l-dvb-apis/vidioc-g-jpegcomp.html
> 
> [5] http://www.mail-archive.com/linux-media@vger.kernel.org/msg01784.html
> 
> [6] http://patchwork.linuxtv.org/patch/8197
> 
> [7] http://www.ampsoft.net/webdesign-l/jpeg-compression.html
> 
> 
> --
> Thanks,
> Sylwester
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] JPEG encoders control class

[Sylwester Nawrocki]

On 11/28/2011 01:20 PM, Hans Verkuil wrote:
> On Saturday 12 November 2011 20:46:25 Sylwester Nawrocki wrote:
>> Hi all,
>>
>> This RFC is discussing the current support of JPEG encoders in V4L2 and
>> a proposal of new JPEG control class.
...
>>
>> et61x251, sn9c102, s2255drv.c
>> -----------------------------
> 
> Note that et61x251 and sn9c102 are going to be deprecated and removed at some
> time in the future (gspca will support these devices).

Ok, thanks for the update.

...
>> The following is an initial draft of the new control class
>>
>> o V4L2_CTRL_CLASS_JPEG
>>
>> As not everything might be covered by the controls (the application data
>> and comment segments, quantization and Huffman tables, etc.) the control
>> class should probably just complement VIDIOC_[G/S]_JPEGCOMP ioctls, rather
>> than entirely replacing them.
>>
>>
>> Proposed controls
>> =================
>>
>> 1. Chroma sub-sampling
>> ---------------------
>>
>> The subsampling factors describe how each component of an input image is
>> sampled, in respect to maximum sample rate in each spatial dimension.
>> More general description can be found in [2], clause A.1.1., "Dimensions
>> and sampling factors".
>>
>> The chroma subsampling would describe how Cb, Cr components should be
>> downsampled after coverting an input image from RGB to Y'CbCr color space.
>>
>> o V4L2_CID_JPEG_CHROMA_SUBSAMPLING
>>
>>   - V4L2_JPEG_CHROMA_SUBSAMPLING_GRAY - only luminance component is
>> present, - V4L2_JPEG_CHROMA_SUBSAMPLING_410  - subsample Cr, Cb signals
>> horizontally by 4 and vertically by 2
>>   - V4L2_JPEG_CHROMA_SUBSAMPLING_411  - horizontally subsample Cr, Cb
>> signals by a factor of 4
>>   - V4L2_JPEG_CHROMA_SUBSAMPLING_420  - subsample Cr, Cb signals
>> horizontally and vertically by 2
>>   - V4L2_JPEG_CHROMA_SUBSAMPLING_422  - horizontally subsample Cr, Cb
>> signals by a factor of 2,
>>   - V4L2_JPEG_CHROMA_SUBSAMPLING_444  - no chroma subsampling, each pixel
>> has Y, Cr and Cb values.
>>
>> Using no subsampling produces sharper colours, even with higher compression
>> (in order to achieve similar file size) [7], thus it seems important to
>> provide the user with a method of precise subsampling control.
>>
>>
>> 2. Restart interval (DRI)
>> -----------------------
>>
>> o V4L2_CID_JPEG_RESTART_INTERVAL
>>
>> The restart interval (DRI marker) determines the interval of inserting RSTm
>> markers. The purpose of RSTm markers is to additionally reinitialize
>> decoder process' predictor with initial default value. For lossy
>> compression processes the restart interval is expressed in MCU (Minimm
>> Coded Unit).
>> If restart interval value is 0 DRI and RSTm (m = 0..7) markers will not be
>> inserted. Consequently this control would make current V4L2_JPEG_MARKER_DRI
>> markers flag redundant. This control would be useful for S5P JPEG IP block
>> [6].
>>
>>
>> 3. Image quality
>> ----------------
>>
>> o V4L2_CID_JPEG_QUALITY
>>
>> Image quality is not defined in the standard but it is used as an
>> intermediate parameter by many encoders to control set of encoding
>> parameters, which then allow to obtain certain image quality and
>> corresponding file size. IMHO it makes sense to add the quality control to
>> the JPEG class as it's widely used, not only for webcams.
>>
>> As far as the value range is concerned, probably it's better to leave this
>> driver specific. The applications would then be more aware of what is
>> supported by a device (min, max, step) and they could translate driver
>> specific range into standardised values (0..100) if needed. Still the
>> drivers could do the translation themselves if required. The specification
>> would only say the 0..100 range is recommended.
> 
> It should also say that higher numbers must represent better quality.

ok, certainly it's a good idea to state it explicitly. I seem to have
forgotten to write it down.

> 
>>
>> 4. JPEG markers presence
>> ------------------------
>>
>> Markers serve as identifiers of various structural parts of compressed data
>> formats. All markers are assigned two-byte codes: an 0xFF byte followed by
>> a byte which is not equal to 0 or 0xFF. [2] Excluding the reserved ones
>> there is 39 valid codes.
>>
>> I'm not really sure how the markers inhibit feature is useful, but since
>> some drivers use it let's assume it is needed. Likely a 32-bit bitmask
>> control could be used for activating/deactivating markers, as it doesn't
>> make sense for some of markers to be freely discarded from the compressed
>> data.
>>
>> o V4L2_CID_JPEG_ACTIVE_MARKERS
>>
>> Following markers might be covered by this control, listed in Table E.1,
>> [2]: APP0..15, COM, DHT, DQT, DAC and additionally DNL.
>> There is still room for 10 additional markers which might be added if
>> required.
> 
> Are there limitation on the contents of the COM field? I.e., can it contain
> '\0' values? If not, then it can perhaps be represented by a string control.

There is no limitation, valid values for each comment byte are 0..255, and
the standard (B.2.4.5, p.43, [2]) says "the interpretation is left to the
application".

Thus unfortunately the string control cannot be used here.

> 
>>
>> The above list of controls is most likely not exhaustive, it's just an
>> attempt to cover features available in the mainline drivers and the S5P
>> SoC JPEG codec IP block [6].
>>
>> In order to support reconfiguration of quantization and Huffman tables the
>> VIDIOC_[G/S]_JPEGCOMP probably need to be re-designed, but it's out of
>> scope of this RFC.
> 
> Overall I'm pleased to see this RFC. The JPEGCOMP ioctls are poorly designed
> and having a well-designed replacement is long overdue.

Thanks, at least the first step is already done:)

> 
> Regards,
> 
> 	Hans
> 
>>
>>
>> References
>> ==========
>>
>> [1] http://www.mail-archive.com/linux-media@vger.kernel.org/msg01783.html
>>
>> [2] http://www.w3.org/Graphics/JPEG/itu-t81.pdf
>>
>> [3] http://www.w3.org/Graphics/JPEG/jfif3.pdf
>>
>> [4] http://linuxtv.org/downloads/v4l-dvb-apis/vidioc-g-jpegcomp.html
>>
>> [5] http://www.mail-archive.com/linux-media@vger.kernel.org/msg01784.html
>>
>> [6] http://patchwork.linuxtv.org/patch/8197
>>
>> [7] http://www.ampsoft.net/webdesign-l/jpeg-compression.html

--

Thanks,
Sylwester

Re: [RFC] JPEG encoders control class

[Luca Risolia]

Hans Verkuil ha scritto:
> On Saturday 12 November 2011 20:46:25 Sylwester Nawrocki wrote:
>> Hi all,
>>
>> This RFC is discussing the current support of JPEG encoders in V4L2 and
>> a proposal of new JPEG control class.
>>
>>
>> Motivation
>> ==========
>>
>> JPEG encoder control is also required at the sub-device level, but
>> currently there are only defined ioctls in regular V4L2 device API. It
>> doesn't seem to make sense for these current ioctls to be inherited by
>> sub-device nodes, since they're not generic enough, incomplete and rather
>> not compliant with JFIF JPEG standard [2], [3].
>>
>>
>> Current implementation
>> ======================
>>
>> Currently two ioctls are available [4]:
>>
>> #define VIDIOC_G_JPEGCOMP	 _IOR('V', 61, struct v4l2_jpegcompression)
>> #define VIDIOC_S_JPEGCOMP	 _IOW('V', 62, struct v4l2_jpegcompression)
>>
>> And the corresponding data structure is defined as:
>>
>> struct v4l2_jpegcompression {
>> 	int quality;
>>
>> 	int  APPn;              /* Number of APP segment to be written,
>> 				 * must be 0..15 */
>> 	int  APP_len;           /* Length of data in JPEG APPn segment */
>> 	char APP_data[60];      /* Data in the JPEG APPn segment. */
>>
>> 	int  COM_len;           /* Length of data in JPEG COM segment */
>> 	char COM_data[60];      /* Data in JPEG COM segment */
>>
>> 	__u32 jpeg_markers;     /* Which markers should go into the JPEG
>> 				 * output. Unless you exactly know what
>> 				 * you do, leave them untouched.
>> 				 * Inluding less markers will make the
>> 				 * resulting code smaller, but there will
>> 				 * be fewer applications which can read it.
>> 				 * The presence of the APP and COM marker
>> 				 * is influenced by APP_len and COM_len
>> 				 * ONLY, not by this property! */
>>
>> #define V4L2_JPEG_MARKER_DHT (1<<3)    /* Define Huffman Tables */
>> #define V4L2_JPEG_MARKER_DQT (1<<4)    /* Define Quantization Tables */
>> #define V4L2_JPEG_MARKER_DRI (1<<5)    /* Define Restart Interval */
>> #define V4L2_JPEG_MARKER_COM (1<<6)    /* Comment segment */
>> #define V4L2_JPEG_MARKER_APP (1<<7)    /* App segment, driver will
>> 					* allways use APP0 */
>> };
>>
>>
>> What are the issues with such an implementation ?
>>
>> These ioctls don't allow to re-program the quantization and Huffman tables
>> (DQT, DHT). Additionally, the standard valid segment length for the
>> application defined APPn and the comment COM segment is 2...65535, while
>> currently this is limited to 60 bytes.
>>
>> Therefore APP_data and COM_data, rather than fixed size arrays, should be
>> pointers to a variable length buffer.
>>
>> Only two drivers upstream really use VIDIOC_[S/G]_JPEGCOMP ioctls for
>> anything more than compression quality query/control. It might make sense
>> to create separate control for image quality and to obsolete the
>> v4l2_jpegcompressin::quality field.
>>
>> Below is a brief review of usage of VIDIOC_[S/G]_JPEGCOMP ioctls in current
>> mainline drivers. Listed are parts of struct v4l2_jpegcompression used in
>> each case.
>>
>>
>> cpia2
>> -----
>>
>> vidioc_g_jpegcomp, vidioc_s_jpegcomp
>> - compression quality ignored, returns fixed value (80)
>> - uses APP_data/len, COM_data/len
>> - markers (only DHT can be disabled by the applications)
>>
>>
>> zoran
>> -----
>>
>> vidioc_g_jpegcomp, vidioc_s_jpegcomp
>> - compression quality, values 5...100, used only to calculate buffer size
>> - APP_data/len, COM_data/len
>> - markers field used to control inclusion of selected JPEG markers
>>   in the output buffer
>>
>>
>> et61x251, sn9c102, s2255drv.c
>> -----------------------------
> 
> Note that et61x251 and sn9c102 are going to be deprecated and removed at some
> time in the future (gspca will support these devices).

Has this been discussed yet? Also, last time I tried the gspca driver 
with a number of V4L2-compliant applications, it did not support at all 
or work well for all the sn9c1xx-based devices I have here, which are 
both controllers and sensors the manufacturer sent to me when developing 
the driver with their collaboration. I also don't understand why the 
gspca driver has been accepted in the mainline kernel in the first 
place, despite the fact the sn9c1xx has been present in the kernel since 
long time and already supported many devices at the time the gspca was 
submitted. Maybe it's better to remove the duplicate code form the gspca 
driver and add the different parts (if any) to the sn9c1xx.

Regards
Luca

Re: [RFC] JPEG encoders control class

[Jean-Francois Moine]

On Mon, 28 Nov 2011 19:48:28 +0100
Luca Risolia <luca.risolia@studio.unibo.it> wrote:

> > Note that et61x251 and sn9c102 are going to be deprecated and removed at some
> > time in the future (gspca will support these devices).  
> 
> Has this been discussed yet? Also, last time I tried the gspca driver 
> with a number of V4L2-compliant applications, it did not support at all 
> or work well for all the sn9c1xx-based devices I have here, which are 
> both controllers and sensors the manufacturer sent to me when developing 
> the driver with their collaboration. I also don't understand why the 
> gspca driver has been accepted in the mainline kernel in the first 
> place, despite the fact the sn9c1xx has been present in the kernel since 
> long time and already supported many devices at the time the gspca was 
> submitted. Maybe it's better to remove the duplicate code form the gspca 
> driver and add the different parts (if any) to the sn9c1xx.

Hi Luca,

Removing the sn9c102 is on the way for a long time. I think we were just
waiting for a message from you!

Why is gspca in the kernel? Because of its design: all the system / v4l2
interface are in the main driver, while the subdrivers only deal with
the specific device exchanges. This simplifies development and
maintenance.

At the beginning, the gspca did not handle the sn9c102 webcams (but
some Sonix based webcams were already handled only by gspca). From time
to time, there were users saying that their webcams did not work
correctly with the sn9c102, and that they worked better with gspca. So,
we moved their IDs to gspca.

Now, the sonixjb and sonixj subdrivers handle many more sensors than
the sn9c102 with less problems. Indeed, there are still problems, but,
as I have only 3 webcams (none with Sonix chips), I must wait for user
reports and fix them.

As you have some Sonix based webcams, and as you had contact with the
manufacturers, you should easily find if there are wrong sequences in
the gspca subdrivers and/or what could be done for a better support.
This would help...

Regards.

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

[RFC/PATCHv1 0/4] JPEG codecs control class

[Sylwester Nawrocki]

Hi,

This patchset is a follow up of an RFC which can be found here:
http://www.mail-archive.com/linux-media@vger.kernel.org/msg39012.html

It creates a new JPEG control class with a tiny amount of four controls
in it. It also adds V4L2_CID_JPEG_COMPRESSION_QUALITY control to two gspca
sub-devices: sonixj and zc3xx, as these where drivers I had the hardware
handy for. The gspca patches have been tested with v4l2ucp and v4l2-ctl.

The compression quality control together with other controls on a panel
is quite convenient. It allows to improve image quality instantly, without
a need for using additional application that handles VIDIOC_S/G_JPEGCOMP.


Thanks,
Sylwester


Sylwester Nawrocki (4):
  V4L: Add JPEG compression control class
  V4L: Add the JPEG compression control class documentation
  gspca: sonixj: Add V4L2_CID_JPEG_COMPRESSION_QUALITY control support
  gspca: zc3xx: Add V4L2_CID_JPEG_COMPRESSION_QUALITY control support

 Documentation/DocBook/media/v4l/biblio.xml         |   20 +++
 Documentation/DocBook/media/v4l/controls.xml       |  161 ++++++++++++++++++++
 .../DocBook/media/v4l/vidioc-g-jpegcomp.xml        |   16 ++-
 drivers/media/video/gspca/sonixj.c                 |   23 +++
 drivers/media/video/gspca/zc3xx.c                  |   54 +++++--
 drivers/media/video/v4l2-ctrls.c                   |   24 +++
 include/linux/videodev2.h                          |   26 +++
 7 files changed, 305 insertions(+), 19 deletions(-)

--
1.7.4.1

[PATCH 1/4] V4L: Add JPEG compression control class

[Sylwester Nawrocki]

The V4L2_CID_JPEG_CLASS control class is intended to expose various
adjustable parameters of JPEG encoders and decoders. Following controls
are defined:

 - V4L2_CID_JPEG_CHROMA_SUBSAMPLING,
 - V4L2_CID_JPEG_RESTART_INTERVAL,
 - V4L2_CID_JPEG_COMPRESSION_QUALITY,
 - V4L2_CID_JPEG_ACTIVE_MARKER.

This covers only a part of relevant standard specifications. More
controls should be added in future if required.

The purpose of V4L2_CID_JPEG_CLASS class is also to replace some
functionality covered by VIDIOC_S/G_JPEGCOMP ioctls, i.e. the JPEG
markers presence and compression quality control. The applications
and drivers should switch from the ioctl to control based API, as
described in the Media API DocBook.

Signed-off-by: Sylwester Nawrocki <snjw23@gmail.com>
---

It could be relevant to also define at this stage controls for JPEG
compression process distinction, i.e.:

* the compression process type

enum v4l2_jpeg_compression_type {
       V4L2_JPEG_COMPRESSION_BASELINE_DCT      = 0,
       V4L2_JPEG_COMPRESSION_EXTENDED_DCT      = 1,
       V4L2_JPEG_COMPRESSION_LOSSLESS          = 2,
       V4L2_JPEG_COMPRESSION_HIERARCHICAL      = 3,
};

* data scan mode for entropy coding

enum v4l2_jpeg_coding_type {
       V4L2_JPEG_CODING_SCAN_SEQUENTIAL       = 0,
       V4L2_JPEG_CODING_SCAN_PROGRESSIVE      = 1,
};

* entropy coding method

enum v4l2_jpeg_coding_scan_type {
       V4L2_JPEG_ENTROPY_CODING_HUFFMAN       = 0,
       V4L2_JPEG_ENTROPY_CODING_ARITHMETIC    = 1,
};

However yet there wouldn't be drivers using those controls, or at most
such controls would have only informational purpose (i.e. no more than
one menu entry). For example, arithmetic coding is rarely used due to
patent claims. And the standard enforces baseline sequential DCT-based
process as a minimum, which most of hardware seem to implement. It all
may change when we see first JPEG 2000 hardware codec, however JPEG 2K
might warrant a separate control class.

Comments ?
---
 drivers/media/video/v4l2-ctrls.c |   24 ++++++++++++++++++++++++
 include/linux/videodev2.h        |   26 ++++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/v4l2-ctrls.c b/drivers/media/video/v4l2-ctrls.c
index 0f415da..b801d8c 100644
--- a/drivers/media/video/v4l2-ctrls.c
+++ b/drivers/media/video/v4l2-ctrls.c
@@ -353,6 +353,16 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
 		NULL,
 	};
 
+	static const char * const jpeg_chroma_subsampling[] = {
+		"4:4:4",
+		"4:2:2",
+		"4:2:0",
+		"4:1:1",
+		"4:1:0",
+		"Gray",
+		NULL,
+	};
+
 	switch (id) {
 	case V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ:
 		return mpeg_audio_sampling_freq;
@@ -414,6 +424,9 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
 		return mpeg_mpeg4_level;
 	case V4L2_CID_MPEG_VIDEO_MPEG4_PROFILE:
 		return mpeg4_profile;
+	case V4L2_CID_JPEG_CHROMA_SUBSAMPLING:
+		return jpeg_chroma_subsampling;
+
 	default:
 		return NULL;
 	}
@@ -606,6 +619,14 @@ const char *v4l2_ctrl_get_name(u32 id)
 	case V4L2_CID_FLASH_CHARGE:		return "Charge";
 	case V4L2_CID_FLASH_READY:		return "Ready to strobe";
 
+	/* JPEG encoder controls */
+	/* Keep the order of the 'case's the same as in videodev2.h! */
+	case V4L2_CID_JPEG_CLASS:		return "JPEG Compression Controls";
+	case V4L2_CID_JPEG_CHROMA_SUBSAMPLING:	return "Chroma Subsampling";
+	case V4L2_CID_JPEG_RESTART_INTERVAL:	return "Restart Interval";
+	case V4L2_CID_JPEG_COMPRESSION_QUALITY:	return "Compression Quality";
+	case V4L2_CID_JPEG_ACTIVE_MARKERS:	return "Active Markers";
+
 	default:
 		return NULL;
 	}
@@ -692,6 +713,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_MPEG_VIDEO_H264_VUI_SAR_IDC:
 	case V4L2_CID_MPEG_VIDEO_MPEG4_LEVEL:
 	case V4L2_CID_MPEG_VIDEO_MPEG4_PROFILE:
+	case V4L2_CID_JPEG_CHROMA_SUBSAMPLING:
 		*type = V4L2_CTRL_TYPE_MENU;
 		break;
 	case V4L2_CID_RDS_TX_PS_NAME:
@@ -703,6 +725,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_MPEG_CLASS:
 	case V4L2_CID_FM_TX_CLASS:
 	case V4L2_CID_FLASH_CLASS:
+	case V4L2_CID_JPEG_CLASS:
 		*type = V4L2_CTRL_TYPE_CTRL_CLASS;
 		/* You can neither read not write these */
 		*flags |= V4L2_CTRL_FLAG_READ_ONLY | V4L2_CTRL_FLAG_WRITE_ONLY;
@@ -716,6 +739,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 		*max = 0xFFFFFF;
 		break;
 	case V4L2_CID_FLASH_FAULT:
+	case V4L2_CID_JPEG_ACTIVE_MARKERS:
 		*type = V4L2_CTRL_TYPE_BITMASK;
 		break;
 	case V4L2_CID_MIN_BUFFERS_FOR_CAPTURE:
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 3d62631..64f02a5 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -1080,6 +1080,7 @@ struct v4l2_ext_controls {
 #define V4L2_CTRL_CLASS_CAMERA 0x009a0000	/* Camera class controls */
 #define V4L2_CTRL_CLASS_FM_TX 0x009b0000	/* FM Modulator control class */
 #define V4L2_CTRL_CLASS_FLASH 0x009c0000	/* Camera flash controls */
+#define V4L2_CTRL_CLASS_JPEG 0x009d0000		/* JPEG-compression controls */
 
 #define V4L2_CTRL_ID_MASK      	  (0x0fffffff)
 #define V4L2_CTRL_ID2CLASS(id)    ((id) & 0x0fff0000UL)
@@ -1688,6 +1689,31 @@ enum v4l2_flash_strobe_source {
 #define V4L2_CID_FLASH_CHARGE			(V4L2_CID_FLASH_CLASS_BASE + 11)
 #define V4L2_CID_FLASH_READY			(V4L2_CID_FLASH_CLASS_BASE + 12)
 
+/*  JPEG-class control IDs defined by V4L2 */
+#define V4L2_CID_JPEG_CLASS_BASE		(V4L2_CTRL_CLASS_JPEG | 0x900)
+#define V4L2_CID_JPEG_CLASS			(V4L2_CTRL_CLASS_JPEG | 1)
+
+#define	V4L2_CID_JPEG_CHROMA_SUBSAMPLING	(V4L2_CID_JPEG_CLASS_BASE + 1)
+enum v4l2_jpeg_chroma_subsampling {
+	V4L2_JPEG_CHROMA_SUBSAMPLING_444	= 0,
+	V4L2_JPEG_CHROMA_SUBSAMPLING_422	= 1,
+	V4L2_JPEG_CHROMA_SUBSAMPLING_420	= 2,
+	V4L2_JPEG_CHROMA_SUBSAMPLING_411	= 3,
+	V4L2_JPEG_CHROMA_SUBSAMPLING_410	= 4,
+	V4L2_JPEG_CHROMA_SUBSAMPLING_GRAY	= 5,
+};
+#define	V4L2_CID_JPEG_RESTART_INTERVAL		(V4L2_CID_JPEG_CLASS_BASE + 2)
+#define	V4L2_CID_JPEG_COMPRESSION_QUALITY	(V4L2_CID_JPEG_CLASS_BASE + 3)
+
+#define	V4L2_CID_JPEG_ACTIVE_MARKERS		(V4L2_CID_JPEG_CLASS_BASE + 4)
+#define	V4L2_JPEG_ACTIVE_MARKER_APP0		(1 << 0)
+#define	V4L2_JPEG_ACTIVE_MARKER_APP1		(1 << 1)
+#define	V4L2_JPEG_ACTIVE_MARKER_COM		(1 << 16)
+#define	V4L2_JPEG_ACTIVE_MARKER_DQT		(1 << 17)
+#define	V4L2_JPEG_ACTIVE_MARKER_DHT		(1 << 18)
+#define	V4L2_JPEG_ACTIVE_MARKER_DAC		(1 << 19)
+#define	V4L2_JPEG_ACTIVE_MARKER_DNL		(1 << 20)
+
 /*
  *	T U N I N G
  */
-- 
1.7.4.1

[PATCH 2/4] V4L: Add the JPEG compression control class documentation

[Sylwester Nawrocki]

Add DocBook entries for the JPEG control class.

Signed-off-by: Sylwester Nawrocki <snjw23@gmail.com>
---
 Documentation/DocBook/media/v4l/biblio.xml         |   20 +++
 Documentation/DocBook/media/v4l/controls.xml       |  161 ++++++++++++++++++++
 .../DocBook/media/v4l/vidioc-g-jpegcomp.xml        |   16 ++-
 3 files changed, 195 insertions(+), 2 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/biblio.xml b/Documentation/DocBook/media/v4l/biblio.xml
index cea6fd3..7dc65c5 100644
--- a/Documentation/DocBook/media/v4l/biblio.xml
+++ b/Documentation/DocBook/media/v4l/biblio.xml
@@ -128,6 +128,26 @@ url="http://www.ijg.org">http://www.ijg.org</ulink>)</corpauthor>
       <subtitle>Version 1.02</subtitle>
     </biblioentry>
 
+    <biblioentry id="itu-t81">
+      <abbrev>ITU-T.81</abbrev>
+      <authorgroup>
+	<corpauthor>International Telecommunication Union
+(<ulink url="http://www.itu.int">http://www.itu.int</ulink>)</corpauthor>
+      </authorgroup>
+      <title>ITU-T Recommendation T.81
+"Information Technology &mdash; Digital Compression and Coding of Continous-Tone
+Still Images &mdash; Requirements and Guidelines"</title>
+    </biblioentry>
+
+    <biblioentry id="w3c-jpeg-jfif">
+      <abbrev>W3C JPEG JFIF</abbrev>
+      <authorgroup>
+	<corpauthor>The World Wide Web Consortium (<ulink
+url="http://www.w3.org/Graphics/JPEG">http://www.w3.org</ulink>)</corpauthor>
+      </authorgroup>
+      <title>JPEG JFIF</title>
+    </biblioentry>
+
     <biblioentry id="smpte12m">
       <abbrev>SMPTE&nbsp;12M</abbrev>
       <authorgroup>
diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml
index c0422c6..ab9e56b 100644
--- a/Documentation/DocBook/media/v4l/controls.xml
+++ b/Documentation/DocBook/media/v4l/controls.xml
@@ -3364,6 +3364,167 @@ interface and may change in the future.</para>
 	</tbody>
       </tgroup>
       </table>
+    </section>
+
+    <section id="jpeg-controls">
+      <title>JPEG Control Reference</title>
+      <para>The JPEG class includes controls for common features of JPEG
+      encoders and decoders. Currently it includes features for codecs
+      implementing progressive baseline DCT compression process with
+      Huffman entrophy coding.</para>
+      <table pgwide="1" frame="none" id="jpeg-control-id">
+      <title>JPEG Control IDs</title>
 
+      <tgroup cols="4">
+	<colspec colname="c1" colwidth="1*" />
+	<colspec colname="c2" colwidth="6*" />
+	<colspec colname="c3" colwidth="2*" />
+	<colspec colname="c4" colwidth="6*" />
+	<spanspec namest="c1" nameend="c2" spanname="id" />
+	<spanspec namest="c2" nameend="c4" spanname="descr" />
+	<thead>
+	  <row>
+	    <entry spanname="id" align="left">ID</entry>
+	    <entry align="left">Type</entry>
+	  </row><row rowsep="1"><entry spanname="descr" align="left">Description</entry>
+	  </row>
+	</thead>
+	<tbody valign="top">
+	  <row><entry></entry></row>
+	  <row>
+	    <entry spanname="id"><constant>V4L2_CID_JPEG_CLASS</constant>&nbsp;</entry>
+	    <entry>class</entry>
+	  </row><row><entry spanname="descr">The JPEG class descriptor. Calling
+	  &VIDIOC-QUERYCTRL; for this control will return a description of this
+	  control class.
+
+	</entry>
+	  </row>
+	  <row>
+	    <entry spanname="id"><constant>V4L2_CID_JPEG_CHROMA_SUBSAMPLING</constant></entry>
+	    <entry>menu</entry>
+	  </row>
+	  <row id="jpeg-chroma-subsampling-control">
+	    <entry spanname="descr">The chroma subsampling factors describe how
+	    each component of an input image is sampled, in respect to maximum
+	    sample rate in each spatial dimension. See <xref linkend="itu-t81"/>,
+	    clause A.1.1. for more details. The <constant>
+	    V4L2_CID_JPEG_CHROMA_SUBSAMPLING</constant> control determines how
+	    Cb and Cr components are downsampled after coverting an input image
+	    from RGB to Y'CbCr color space.
+	    </entry>
+	  </row>
+	  <row>
+	    <entrytbl spanname="descr" cols="2">
+	      <tbody valign="top">
+		<row>
+		  <entry><constant>V4L2_JPEG_CHROMA_SUBSAMPLING_444</constant>
+		  </entry><entry>No chroma subsampling, each pixel has
+		  Y, Cr and Cb values.</entry>
+		</row>
+		<row>
+		  <entry><constant>V4L2_JPEG_CHROMA_SUBSAMPLING_422</constant>
+		  </entry><entry>Horizontally subsample Cr, Cb components
+		  by a factor of 2.</entry>
+		</row>
+		<row>
+		  <entry><constant>V4L2_JPEG_CHROMA_SUBSAMPLING_420</constant>
+		  </entry><entry>Subsample Cr, Cb components horizontally
+		  and vertically by 2.</entry>
+		</row>
+		<row>
+		  <entry><constant>V4L2_JPEG_CHROMA_SUBSAMPLING_411</constant>
+		  </entry><entry>Horizontally subsample Cr, Cb components
+		  by a factor of 4.</entry>
+		</row>
+		<row>
+		  <entry><constant>V4L2_JPEG_CHROMA_SUBSAMPLING_410</constant>
+		  </entry><entry>Subsample Cr, Cb components horizontally
+		  by 4 and vertically by 2.</entry>
+		</row>
+		<row>
+		  <entry><constant>V4L2_JPEG_CHROMA_SUBSAMPLING_GRAY</constant>
+		  </entry><entry>Use only luminance component.</entry>
+		</row>
+	      </tbody>
+	    </entrytbl>
+	  </row>
+	  <row>
+	    <entry spanname="id"><constant>V4L2_CID_JPEG_RESTART_INTERVAL</constant>
+	    </entry><entry>integer</entry>
+	  </row>
+	  <row><entry spanname="descr">
+	      The restart interval determines an interval of inserting RSTm
+	      markers (m = 0..7). The purpose of these markers is to additionally
+	      reinitialize the encoder process, in order to process blocks of
+	      an image independently.
+	      For the lossy compression processes the restart interval unit is
+	      MCU (Minimum Coded Unit) and its value is contained in DRI
+	      (Define Restart Interval) marker. If <constant>
+	      V4L2_CID_JPEG_RESTART_INTERVAL</constant> control is set to 0,
+	      DRI and RSTm markers will not be inserted.
+	    </entry>
+	  </row>
+	  <row id="jpeg-quality-control">
+	    <entry spanname="id"><constant>V4L2_CID_JPEG_COMPRESION_QUALITY</constant></entry>
+	    <entry>integer</entry>
+	  </row>
+	  <row>
+	    <entry spanname="descr">
+	      <constant>V4L2_CID_JPEG_COMPRESION_QUALITY</constant> control
+	      determines trade-off between image quality and size.
+	      It provides simpler method for applications to control image quality,
+	      without a need for direct reconfiguration of luminance and chrominance
+	      quantization tables.
+
+	      In cases where a driver uses quantization tables configured directly
+	      by an application, using interfaces defined elsewhere, <constant>
+	      V4L2_CID_JPEG_COMPRESION_QUALITY</constant> control should be set
+	      by driver to 0.
+
+	      <para>The value range of this control is driver-specific. Only
+	      positive, non-zero values are meaningful. The recommended range
+	      is 1 - 100, where larger values correspond to better image quality.
+	      </para>
+	    </entry>
+	    </row>
+	  <row id="jpeg-active-marker-control">
+	    <entry spanname="id"><constant>V4L2_CID_JPEG_ACTIVE_MARKER</constant></entry>
+	    <entry>bitmask</entry>
+	  </row>
+	  <row>
+	    <entry spanname="descr">Specify which JPEG markers are included
+	    in compressed stream. This control is valid only for encoders.
+	    </entry>
+	  </row>
+	  <row>
+	    <entrytbl spanname="descr" cols="2">
+	      <tbody valign="top">
+		<row>
+		  <entry><constant>V4L2_JPEG_ACTIVE_MARKER_APP0</constant></entry>
+		  <entry>Application data segment APP<subscript>0</subscript>.</entry>
+		</row><row>
+		  <entry><constant>V4L2_JPEG_ACTIVE_MARKER_APP1</constant></entry>
+		  <entry>Application data segment APP<subscript>1</subscript>.</entry>
+		</row><row>
+		  <entry><constant>V4L2_JPEG_ACTIVE_MARKER_COM</constant></entry>
+		  <entry>Comment segment.</entry>
+		</row><row>
+		  <entry><constant>V4L2_JPEG_ACTIVE_MARKER_DQT</constant></entry>
+		  <entry>Quantization tables segment.</entry>
+		</row><row>
+		  <entry><constant>V4L2_JPEG_ACTIVE_MARKER_DHT</constant></entry>
+		  <entry>Huffman tables segment.</entry>
+		</row>
+	      </tbody>
+	    </entrytbl>
+	  </row>
+	  <row><entry></entry></row>
+	</tbody>
+      </tgroup>
+      </table>
+      <para>For more details about JPEG specification, refer
+      to <xref linkend="itu-t81"/>, <xref linkend="jfif"/>,
+      <xref linkend="w3c-jpeg-jfif"/>.</para>
     </section>
 </section>
diff --git a/Documentation/DocBook/media/v4l/vidioc-g-jpegcomp.xml b/Documentation/DocBook/media/v4l/vidioc-g-jpegcomp.xml
index 01ea24b..4874849 100644
--- a/Documentation/DocBook/media/v4l/vidioc-g-jpegcomp.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-g-jpegcomp.xml
@@ -57,6 +57,11 @@
   <refsect1>
     <title>Description</title>
 
+    <para>These ioctls are <emphasis role="bold">deprecated</emphasis>.
+    New drivers and applications should use <link linkend="jpeg-controls">
+    JPEG class controls</link> for image quality and JPEG markers control.
+    </para>
+
     <para>[to do]</para>
 
     <para>Ronald Bultje elaborates:</para>
@@ -86,7 +91,10 @@ to add them.</para>
 	  <row>
 	    <entry>int</entry>
 	    <entry><structfield>quality</structfield></entry>
-	    <entry></entry>
+	    <entry>Deprecated. If <link linkend="jpeg-quality-control"><constant>
+	    V4L2_CID_JPEG_IMAGE_QUALITY</constant></link> control is exposed by
+	    a driver applications should use it instead and ignore this field.
+	    </entry>
 	  </row>
 	  <row>
 	    <entry>int</entry>
@@ -116,7 +124,11 @@ to add them.</para>
 	  <row>
 	    <entry>__u32</entry>
 	    <entry><structfield>jpeg_markers</structfield></entry>
-	    <entry>See <xref linkend="jpeg-markers" />.</entry>
+	    <entry>See <xref linkend="jpeg-markers"/>. Deprecated.
+	    If <link linkend="jpeg-active-marker-control"><constant>
+	    V4L2_CID_JPEG_ACTIVE_MARKER</constant></link> control
+	    is exposed by a driver applications should use it instead
+	    and ignore this field.</entry>
 	  </row>
 	</tbody>
       </tgroup>
-- 
1.7.4.1

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

[Sylwester Nawrocki]

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.

For completeness the V4L2_CID_JPEG_ACTIVE_MARKER control should be also
added.

Cc: Jean-Francois Moine <moinejf@free.fr>
Signed-off-by: Sylwester Nawrocki <snjw23@gmail.com>
---
 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.4.1

[PATCH 4/4] gspca: zc3xx: Add V4L2_CID_JPEG_COMPRESSION_QUALITY control support

[Sylwester Nawrocki]

The JPEG compression quality control is currently done by means of the
VIDIOC_S/G_JPEGCOMP ioctls. 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_S/G_JPEGCOMP ioctl handlers can be
removed, leaving the control the only user interface for compression
quality configuration.

For completeness, the V4L2_CID_JPEG_ACTIVE_MARKER control should be also
added.

Cc: Jean-Francois Moine <moinejf@free.fr>
Signed-off-by: Sylwester Nawrocki <snjw23@gmail.com>
---
 drivers/media/video/gspca/zc3xx.c |   54 +++++++++++++++++++++++++-----------
 1 files changed, 37 insertions(+), 17 deletions(-)

diff --git a/drivers/media/video/gspca/zc3xx.c b/drivers/media/video/gspca/zc3xx.c
index f22e02f..019a93b 100644
--- a/drivers/media/video/gspca/zc3xx.c
+++ b/drivers/media/video/gspca/zc3xx.c
@@ -46,6 +46,7 @@ enum e_ctrl {
 	AUTOGAIN,
 	LIGHTFREQ,
 	SHARPNESS,
+	QUALITY,
 	NCTRLS		/* number of controls */
 };
 
@@ -57,11 +58,6 @@ struct sd {
 
 	struct gspca_ctrl ctrls[NCTRLS];
 
-	u8 quality;			/* image quality */
-#define QUALITY_MIN 50
-#define QUALITY_MAX 80
-#define QUALITY_DEF 70
-
 	u8 bridge;
 	u8 sensor;		/* Type of image sensor chip */
 	u16 chip_revision;
@@ -101,6 +97,12 @@ static void setexposure(struct gspca_dev *gspca_dev);
 static int sd_setautogain(struct gspca_dev *gspca_dev, __s32 val);
 static void setlightfreq(struct gspca_dev *gspca_dev);
 static void setsharpness(struct gspca_dev *gspca_dev);
+static int sd_setquality(struct gspca_dev *gspca_dev, __s32 val);
+
+/* JPEG image quality */
+#define QUALITY_MIN 50
+#define QUALITY_MAX 80
+#define QUALITY_DEF 70
 
 static const struct ctrl sd_ctrls[NCTRLS] = {
 [BRIGHTNESS] = {
@@ -188,6 +190,18 @@ static const struct ctrl sd_ctrls[NCTRLS] = {
 	    },
 	    .set_control = setsharpness
 	},
+[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,
+	    },
+	    .set = sd_setquality
+	},
 };
 
 static const struct v4l2_pix_format vga_mode[] = {
@@ -6411,7 +6425,7 @@ static int sd_config(struct gspca_dev *gspca_dev,
 	sd->sensor = id->driver_info;
 
 	gspca_dev->cam.ctrls = sd->ctrls;
-	sd->quality = QUALITY_DEF;
+	sd->ctrls[QUALITY].val = QUALITY_DEF;
 
 	return 0;
 }
@@ -6685,7 +6699,7 @@ static int sd_start(struct gspca_dev *gspca_dev)
 	/* create the JPEG header */
 	jpeg_define(sd->jpeg_hdr, gspca_dev->height, gspca_dev->width,
 			0x21);		/* JPEG 422 */
-	jpeg_set_qual(sd->jpeg_hdr, sd->quality);
+	jpeg_set_qual(sd->jpeg_hdr, sd->ctrls[QUALITY].val);
 
 	mode = gspca_dev->cam.cam_mode[gspca_dev->curr_mode].priv;
 	switch (sd->sensor) {
@@ -6893,29 +6907,35 @@ static int sd_querymenu(struct gspca_dev *gspca_dev,
 	return -EINVAL;
 }
 
-static int sd_set_jcomp(struct gspca_dev *gspca_dev,
-			struct v4l2_jpegcompression *jcomp)
+static int sd_setquality(struct gspca_dev *gspca_dev, __s32 val)
 {
 	struct sd *sd = (struct sd *) gspca_dev;
 
-	if (jcomp->quality < QUALITY_MIN)
-		sd->quality = QUALITY_MIN;
-	else if (jcomp->quality > QUALITY_MAX)
-		sd->quality = QUALITY_MAX;
-	else
-		sd->quality = jcomp->quality;
+	sd->ctrls[QUALITY].val = val;
+
 	if (gspca_dev->streaming)
-		jpeg_set_qual(sd->jpeg_hdr, sd->quality);
+		jpeg_set_qual(sd->jpeg_hdr, val);
+
 	return gspca_dev->usb_err;
 }
 
+static int sd_set_jcomp(struct gspca_dev *gspca_dev,
+			struct v4l2_jpegcompression *jcomp)
+{
+	struct sd *sd = (struct sd *) gspca_dev;
+
+	sd->ctrls[QUALITY].val = clamp_t(u8, jcomp->quality,
+					QUALITY_MIN, QUALITY_MAX);
+	return sd_setquality(gspca_dev, sd->ctrls[QUALITY].val);
+}
+
 static int sd_get_jcomp(struct gspca_dev *gspca_dev,
 			struct v4l2_jpegcompression *jcomp)
 {
 	struct sd *sd = (struct sd *) gspca_dev;
 
 	memset(jcomp, 0, sizeof *jcomp);
-	jcomp->quality = sd->quality;
+	jcomp->quality = sd->ctrls[QUALITY].val;
 	jcomp->jpeg_markers = V4L2_JPEG_MARKER_DHT
 			| V4L2_JPEG_MARKER_DQT;
 	return 0;
-- 
1.7.4.1

Re: [PATCH 1/4] V4L: Add JPEG compression control class

[Sakari Ailus]

Hi Sylwester,

On Tue, Dec 27, 2011 at 08:43:28PM +0100, Sylwester Nawrocki wrote:
...
> +#define	V4L2_CID_JPEG_ACTIVE_MARKERS		(V4L2_CID_JPEG_CLASS_BASE + 4)

Just a few comments. I like the approach, and I'd just remove the 'S' from
the CID name.

Cheers,

-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi	jabber/XMPP/Gmail: sailus@retiisi.org.uk

Re: [PATCH 1/4] V4L: Add JPEG compression control class

[Sylwester Nawrocki]

Hi Sakari,

On 12/30/2011 10:42 PM, Sakari Ailus wrote:
> Hi Sylwester,
> 
> On Tue, Dec 27, 2011 at 08:43:28PM +0100, Sylwester Nawrocki wrote:
> ...
>> +#define	V4L2_CID_JPEG_ACTIVE_MARKERS		(V4L2_CID_JPEG_CLASS_BASE + 4)
> 
> Just a few comments. I like the approach, and I'd just remove the 'S' from
> the CID name.

Thanks for your review. Ok, I'll make it singular form. I was going back and forth
with that, and in last minute even forgot to make it consistent in the DocBook
patches.

I think we need to also redesign VIDIOC_S/G_JPEGCOMP ioctls, to support
quantization
and Huffman tables setup. My idea was to firstly add the controls support in
drivers,
let some time for application to start using them (1 year or so), then remove
VIDIOC_S/G_JPEGCOMP support from drivers in question. Finally having very few
drivers
and applications using VIDIOC_S/G_JPEGCOMP we could deprecate those ioctls and add
new ones.
As of now only two drivers are using S/G_JPEGCOMP for anything else than image
quality
and the markers, with gspca being main user.

-- 

Thanks,
Sylwester

[PATCH/RFC v2 0/4] JPEG codecs control class

[Sylwester Nawrocki]

Hello,

This patch set adds new control class - V4L2_CID_JPEG_CLASS with initially
four controls in it. It also adds V4L2_CID_JPEG_COMPRESSION_QUALITY control
to two gspca sub-devices: sonixj and zc3xx, as these where drivers I had
the hardware handy for. The gspca patches have been tested with v4l2ucp
and v4l2-ctl.

I could provide patches for some other drivers that currently use
VIDIOC_S/G_JPEGCOMP ioctls for image quality setting only.

The initial RFC can be found at
http://www.mail-archive.com/linux-media@vger.kernel.org/msg39012.html

Changes since v1 [1]:
 - renamed trailing 'S' from V4L2_CID_JPEG_ACTIVE_MARKERS;
 - removed V4L2_JPEG_ACTIVE_MARKER_DAC and V4L2_JPEG_ACTIVE_MARKER_DNL
   definitions as these are normally controlled by higher level compression
   attributes and shouldn't be allowed to be discarded independently;

These patches are intended for v3.4. Comments are welcome.


Sylwester Nawrocki (4):
  V4L: Add JPEG compression control class
  V4L: Add JPEG compression control class documentation
  gspca: sonixj: Add V4L2_CID_JPEG_COMPRESSION_QUALITY control support
  gspca: zc3xx: Add V4L2_CID_JPEG_COMPRESSION_QUALITY control support

 Documentation/DocBook/media/v4l/biblio.xml         |   20 +++
 Documentation/DocBook/media/v4l/compat.xml         |   10 ++
 Documentation/DocBook/media/v4l/controls.xml       |  161 ++++++++++++++++++++
 Documentation/DocBook/media/v4l/v4l2.xml           |    9 +
 .../DocBook/media/v4l/vidioc-g-jpegcomp.xml        |   16 ++-
 drivers/media/video/gspca/sonixj.c                 |   23 +++
 drivers/media/video/gspca/zc3xx.c                  |   54 +++++--
 drivers/media/video/v4l2-ctrls.c                   |   24 +++
 include/linux/videodev2.h                          |   24 +++
 9 files changed, 322 insertions(+), 19 deletions(-)

--
Thanks,
Sylwester

[1] http://www.mail-archive.com/linux-media@vger.kernel.org/msg41070.html

[PATCH/RFC v2 1/4] V4L: Add JPEG compression control class

[Sylwester Nawrocki]

The V4L2_CID_JPEG_CLASS control class is intended to expose various
adjustable parameters of JPEG encoders and decoders. Following controls
are defined:

 - V4L2_CID_JPEG_CHROMA_SUBSAMPLING,
 - V4L2_CID_JPEG_RESTART_INTERVAL,
 - V4L2_CID_JPEG_COMPRESSION_QUALITY,
 - V4L2_CID_JPEG_ACTIVE_MARKER.

This covers only a part of relevant standard specifications. More
controls should be added in future if required.

The purpose of V4L2_CID_JPEG_CLASS class is also to replace some
functionality covered by VIDIOC_S/G_JPEGCOMP ioctls, i.e. the JPEG
markers presence and compression quality control. The applications
and drivers should switch from the ioctl to control based API, as
described in the subsequent patches for the Media API DocBook.

Signed-off-by: Sylwester Nawrocki <snjw23@gmail.com>
---
 drivers/media/video/v4l2-ctrls.c |   24 ++++++++++++++++++++++++
 include/linux/videodev2.h        |   24 ++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/v4l2-ctrls.c b/drivers/media/video/v4l2-ctrls.c
index da1f4c2..1fd6435 100644
--- a/drivers/media/video/v4l2-ctrls.c
+++ b/drivers/media/video/v4l2-ctrls.c
@@ -353,6 +353,16 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
 		NULL,
 	};
 
+	static const char * const jpeg_chroma_subsampling[] = {
+		"4:4:4",
+		"4:2:2",
+		"4:2:0",
+		"4:1:1",
+		"4:1:0",
+		"Gray",
+		NULL,
+	};
+
 	switch (id) {
 	case V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ:
 		return mpeg_audio_sampling_freq;
@@ -414,6 +424,9 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
 		return mpeg_mpeg4_level;
 	case V4L2_CID_MPEG_VIDEO_MPEG4_PROFILE:
 		return mpeg4_profile;
+	case V4L2_CID_JPEG_CHROMA_SUBSAMPLING:
+		return jpeg_chroma_subsampling;
+
 	default:
 		return NULL;
 	}
@@ -607,6 +620,14 @@ const char *v4l2_ctrl_get_name(u32 id)
 	case V4L2_CID_FLASH_CHARGE:		return "Charge";
 	case V4L2_CID_FLASH_READY:		return "Ready to strobe";
 
+	/* JPEG encoder controls */
+	/* Keep the order of the 'case's the same as in videodev2.h! */
+	case V4L2_CID_JPEG_CLASS:		return "JPEG Compression Controls";
+	case V4L2_CID_JPEG_CHROMA_SUBSAMPLING:	return "Chroma Subsampling";
+	case V4L2_CID_JPEG_RESTART_INTERVAL:	return "Restart Interval";
+	case V4L2_CID_JPEG_COMPRESSION_QUALITY:	return "Compression Quality";
+	case V4L2_CID_JPEG_ACTIVE_MARKER:	return "Active Markers";
+
 	default:
 		return NULL;
 	}
@@ -693,6 +714,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_MPEG_VIDEO_H264_VUI_SAR_IDC:
 	case V4L2_CID_MPEG_VIDEO_MPEG4_LEVEL:
 	case V4L2_CID_MPEG_VIDEO_MPEG4_PROFILE:
+	case V4L2_CID_JPEG_CHROMA_SUBSAMPLING:
 		*type = V4L2_CTRL_TYPE_MENU;
 		break;
 	case V4L2_CID_RDS_TX_PS_NAME:
@@ -704,6 +726,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_MPEG_CLASS:
 	case V4L2_CID_FM_TX_CLASS:
 	case V4L2_CID_FLASH_CLASS:
+	case V4L2_CID_JPEG_CLASS:
 		*type = V4L2_CTRL_TYPE_CTRL_CLASS;
 		/* You can neither read not write these */
 		*flags |= V4L2_CTRL_FLAG_READ_ONLY | V4L2_CTRL_FLAG_WRITE_ONLY;
@@ -717,6 +740,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 		*max = 0xFFFFFF;
 		break;
 	case V4L2_CID_FLASH_FAULT:
+	case V4L2_CID_JPEG_ACTIVE_MARKER:
 		*type = V4L2_CTRL_TYPE_BITMASK;
 		break;
 	case V4L2_CID_MIN_BUFFERS_FOR_CAPTURE:
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 2965906..a13fd34 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -1080,6 +1080,7 @@ struct v4l2_ext_controls {
 #define V4L2_CTRL_CLASS_CAMERA 0x009a0000	/* Camera class controls */
 #define V4L2_CTRL_CLASS_FM_TX 0x009b0000	/* FM Modulator control class */
 #define V4L2_CTRL_CLASS_FLASH 0x009c0000	/* Camera flash controls */
+#define V4L2_CTRL_CLASS_JPEG 0x009d0000		/* JPEG-compression controls */
 
 #define V4L2_CTRL_ID_MASK      	  (0x0fffffff)
 #define V4L2_CTRL_ID2CLASS(id)    ((id) & 0x0fff0000UL)
@@ -1688,6 +1689,29 @@ enum v4l2_flash_strobe_source {
 #define V4L2_CID_FLASH_CHARGE			(V4L2_CID_FLASH_CLASS_BASE + 11)
 #define V4L2_CID_FLASH_READY			(V4L2_CID_FLASH_CLASS_BASE + 12)
 
+/*  JPEG-class control IDs defined by V4L2 */
+#define V4L2_CID_JPEG_CLASS_BASE		(V4L2_CTRL_CLASS_JPEG | 0x900)
+#define V4L2_CID_JPEG_CLASS			(V4L2_CTRL_CLASS_JPEG | 1)
+
+#define	V4L2_CID_JPEG_CHROMA_SUBSAMPLING	(V4L2_CID_JPEG_CLASS_BASE + 1)
+enum v4l2_jpeg_chroma_subsampling {
+	V4L2_JPEG_CHROMA_SUBSAMPLING_444	= 0,
+	V4L2_JPEG_CHROMA_SUBSAMPLING_422	= 1,
+	V4L2_JPEG_CHROMA_SUBSAMPLING_420	= 2,
+	V4L2_JPEG_CHROMA_SUBSAMPLING_411	= 3,
+	V4L2_JPEG_CHROMA_SUBSAMPLING_410	= 4,
+	V4L2_JPEG_CHROMA_SUBSAMPLING_GRAY	= 5,
+};
+#define	V4L2_CID_JPEG_RESTART_INTERVAL		(V4L2_CID_JPEG_CLASS_BASE + 2)
+#define	V4L2_CID_JPEG_COMPRESSION_QUALITY	(V4L2_CID_JPEG_CLASS_BASE + 3)
+
+#define	V4L2_CID_JPEG_ACTIVE_MARKER		(V4L2_CID_JPEG_CLASS_BASE + 4)
+#define	V4L2_JPEG_ACTIVE_MARKER_APP0		(1 << 0)
+#define	V4L2_JPEG_ACTIVE_MARKER_APP1		(1 << 1)
+#define	V4L2_JPEG_ACTIVE_MARKER_COM		(1 << 16)
+#define	V4L2_JPEG_ACTIVE_MARKER_DQT		(1 << 17)
+#define	V4L2_JPEG_ACTIVE_MARKER_DHT		(1 << 18)
+
 /*
  *	T U N I N G
  */
-- 
1.7.1

[PATCH/RFC v2 2/4] V4L: Add JPEG compression control class documentation

[Sylwester Nawrocki]

Add DocBook entries for the JPEG control class.

Signed-off-by: Sylwester Nawrocki <snjw23@gmail.com>
---
 Documentation/DocBook/media/v4l/biblio.xml         |   20 +++
 Documentation/DocBook/media/v4l/compat.xml         |   10 ++
 Documentation/DocBook/media/v4l/controls.xml       |  161 ++++++++++++++++++++
 Documentation/DocBook/media/v4l/v4l2.xml           |    9 +
 .../DocBook/media/v4l/vidioc-g-jpegcomp.xml        |   16 ++-
 5 files changed, 214 insertions(+), 2 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/biblio.xml b/Documentation/DocBook/media/v4l/biblio.xml
index cea6fd3..7dc65c5 100644
--- a/Documentation/DocBook/media/v4l/biblio.xml
+++ b/Documentation/DocBook/media/v4l/biblio.xml
@@ -128,6 +128,26 @@ url="http://www.ijg.org">http://www.ijg.org</ulink>)</corpauthor>
       <subtitle>Version 1.02</subtitle>
     </biblioentry>
 
+    <biblioentry id="itu-t81">
+      <abbrev>ITU-T.81</abbrev>
+      <authorgroup>
+	<corpauthor>International Telecommunication Union
+(<ulink url="http://www.itu.int">http://www.itu.int</ulink>)</corpauthor>
+      </authorgroup>
+      <title>ITU-T Recommendation T.81
+"Information Technology &mdash; Digital Compression and Coding of Continous-Tone
+Still Images &mdash; Requirements and Guidelines"</title>
+    </biblioentry>
+
+    <biblioentry id="w3c-jpeg-jfif">
+      <abbrev>W3C JPEG JFIF</abbrev>
+      <authorgroup>
+	<corpauthor>The World Wide Web Consortium (<ulink
+url="http://www.w3.org/Graphics/JPEG">http://www.w3.org</ulink>)</corpauthor>
+      </authorgroup>
+      <title>JPEG JFIF</title>
+    </biblioentry>
+
     <biblioentry id="smpte12m">
       <abbrev>SMPTE&nbsp;12M</abbrev>
       <authorgroup>
diff --git a/Documentation/DocBook/media/v4l/compat.xml b/Documentation/DocBook/media/v4l/compat.xml
index 12ba262..1ac96db 100644
--- a/Documentation/DocBook/media/v4l/compat.xml
+++ b/Documentation/DocBook/media/v4l/compat.xml
@@ -2390,6 +2390,16 @@ that used it. It was originally scheduled for removal in 2.6.35.
       </orderedlist>
     </section>
 
+    <section>
+      <title>V4L2 in Linux 3.4</title>
+      <orderedlist>
+        <listitem>
+	  <para>Added <link linkend="jpeg-controls">JPEG compression control
+	  class</link>.</para>
+        </listitem>
+      </orderedlist>
+    </section>
+
     <section id="other">
       <title>Relation of V4L2 to other Linux multimedia APIs</title>
 
diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml
index a1be378..b99f58b 100644
--- a/Documentation/DocBook/media/v4l/controls.xml
+++ b/Documentation/DocBook/media/v4l/controls.xml
@@ -3377,6 +3377,167 @@ interface and may change in the future.</para>
 	</tbody>
       </tgroup>
       </table>
+    </section>
+
+    <section id="jpeg-controls">
+      <title>JPEG Control Reference</title>
+      <para>The JPEG class includes controls for common features of JPEG
+      encoders and decoders. Currently it includes features for codecs
+      implementing progressive baseline DCT compression process with
+      Huffman entrophy coding.</para>
+      <table pgwide="1" frame="none" id="jpeg-control-id">
+      <title>JPEG Control IDs</title>
+
+      <tgroup cols="4">
+	<colspec colname="c1" colwidth="1*" />
+	<colspec colname="c2" colwidth="6*" />
+	<colspec colname="c3" colwidth="2*" />
+	<colspec colname="c4" colwidth="6*" />
+	<spanspec namest="c1" nameend="c2" spanname="id" />
+	<spanspec namest="c2" nameend="c4" spanname="descr" />
+	<thead>
+	  <row>
+	    <entry spanname="id" align="left">ID</entry>
+	    <entry align="left">Type</entry>
+	  </row><row rowsep="1"><entry spanname="descr" align="left">Description</entry>
+	  </row>
+	</thead>
+	<tbody valign="top">
+	  <row><entry></entry></row>
+	  <row>
+	    <entry spanname="id"><constant>V4L2_CID_JPEG_CLASS</constant>&nbsp;</entry>
+	    <entry>class</entry>
+	  </row><row><entry spanname="descr">The JPEG class descriptor. Calling
+	  &VIDIOC-QUERYCTRL; for this control will return a description of this
+	  control class.
+
+	</entry>
+	  </row>
+	  <row>
+	    <entry spanname="id"><constant>V4L2_CID_JPEG_CHROMA_SUBSAMPLING</constant></entry>
+	    <entry>menu</entry>
+	  </row>
+	  <row id="jpeg-chroma-subsampling-control">
+	    <entry spanname="descr">The chroma subsampling factors describe how
+	    each component of an input image is sampled, in respect to maximum
+	    sample rate in each spatial dimension. See <xref linkend="itu-t81"/>,
+	    clause A.1.1. for more details. The <constant>
+	    V4L2_CID_JPEG_CHROMA_SUBSAMPLING</constant> control determines how
+	    Cb and Cr components are downsampled after coverting an input image
+	    from RGB to Y'CbCr color space.
+	    </entry>
+	  </row>
+	  <row>
+	    <entrytbl spanname="descr" cols="2">
+	      <tbody valign="top">
+		<row>
+		  <entry><constant>V4L2_JPEG_CHROMA_SUBSAMPLING_444</constant>
+		  </entry><entry>No chroma subsampling, each pixel has
+		  Y, Cr and Cb values.</entry>
+		</row>
+		<row>
+		  <entry><constant>V4L2_JPEG_CHROMA_SUBSAMPLING_422</constant>
+		  </entry><entry>Horizontally subsample Cr, Cb components
+		  by a factor of 2.</entry>
+		</row>
+		<row>
+		  <entry><constant>V4L2_JPEG_CHROMA_SUBSAMPLING_420</constant>
+		  </entry><entry>Subsample Cr, Cb components horizontally
+		  and vertically by 2.</entry>
+		</row>
+		<row>
+		  <entry><constant>V4L2_JPEG_CHROMA_SUBSAMPLING_411</constant>
+		  </entry><entry>Horizontally subsample Cr, Cb components
+		  by a factor of 4.</entry>
+		</row>
+		<row>
+		  <entry><constant>V4L2_JPEG_CHROMA_SUBSAMPLING_410</constant>
+		  </entry><entry>Subsample Cr, Cb components horizontally
+		  by 4 and vertically by 2.</entry>
+		</row>
+		<row>
+		  <entry><constant>V4L2_JPEG_CHROMA_SUBSAMPLING_GRAY</constant>
+		  </entry><entry>Use only luminance component.</entry>
+		</row>
+	      </tbody>
+	    </entrytbl>
+	  </row>
+	  <row>
+	    <entry spanname="id"><constant>V4L2_CID_JPEG_RESTART_INTERVAL</constant>
+	    </entry><entry>integer</entry>
+	  </row>
+	  <row><entry spanname="descr">
+	      The restart interval determines an interval of inserting RSTm
+	      markers (m = 0..7). The purpose of these markers is to additionally
+	      reinitialize the encoder process, in order to process blocks of
+	      an image independently.
+	      For the lossy compression processes the restart interval unit is
+	      MCU (Minimum Coded Unit) and its value is contained in DRI
+	      (Define Restart Interval) marker. If <constant>
+	      V4L2_CID_JPEG_RESTART_INTERVAL</constant> control is set to 0,
+	      DRI and RSTm markers will not be inserted.
+	    </entry>
+	  </row>
+	  <row id="jpeg-quality-control">
+	    <entry spanname="id"><constant>V4L2_CID_JPEG_COMPRESION_QUALITY</constant></entry>
+	    <entry>integer</entry>
+	  </row>
+	  <row>
+	    <entry spanname="descr">
+	      <constant>V4L2_CID_JPEG_COMPRESION_QUALITY</constant> control
+	      determines trade-off between image quality and size.
+	      It provides simpler method for applications to control image quality,
+	      without a need for direct reconfiguration of luminance and chrominance
+	      quantization tables.
+
+	      In cases where a driver uses quantization tables configured directly
+	      by an application, using interfaces defined elsewhere, <constant>
+	      V4L2_CID_JPEG_COMPRESION_QUALITY</constant> control should be set
+	      by driver to 0.
 
+	      <para>The value range of this control is driver-specific. Only
+	      positive, non-zero values are meaningful. The recommended range
+	      is 1 - 100, where larger values correspond to better image quality.
+	      </para>
+	    </entry>
+	    </row>
+	  <row id="jpeg-active-marker-control">
+	    <entry spanname="id"><constant>V4L2_CID_JPEG_ACTIVE_MARKER</constant></entry>
+	    <entry>bitmask</entry>
+	  </row>
+	  <row>
+	    <entry spanname="descr">Specify which JPEG markers are included
+	    in compressed stream. This control is valid only for encoders.
+	    </entry>
+	  </row>
+	  <row>
+	    <entrytbl spanname="descr" cols="2">
+	      <tbody valign="top">
+		<row>
+		  <entry><constant>V4L2_JPEG_ACTIVE_MARKER_APP0</constant></entry>
+		  <entry>Application data segment APP<subscript>0</subscript>.</entry>
+		</row><row>
+		  <entry><constant>V4L2_JPEG_ACTIVE_MARKER_APP1</constant></entry>
+		  <entry>Application data segment APP<subscript>1</subscript>.</entry>
+		</row><row>
+		  <entry><constant>V4L2_JPEG_ACTIVE_MARKER_COM</constant></entry>
+		  <entry>Comment segment.</entry>
+		</row><row>
+		  <entry><constant>V4L2_JPEG_ACTIVE_MARKER_DQT</constant></entry>
+		  <entry>Quantization tables segment.</entry>
+		</row><row>
+		  <entry><constant>V4L2_JPEG_ACTIVE_MARKER_DHT</constant></entry>
+		  <entry>Huffman tables segment.</entry>
+		</row>
+	      </tbody>
+	    </entrytbl>
+	  </row>
+	  <row><entry></entry></row>
+	</tbody>
+      </tgroup>
+      </table>
+      <para>For more details about JPEG specification, refer
+      to <xref linkend="itu-t81"/>, <xref linkend="jfif"/>,
+      <xref linkend="w3c-jpeg-jfif"/>.</para>
     </section>
 </section>
diff --git a/Documentation/DocBook/media/v4l/v4l2.xml b/Documentation/DocBook/media/v4l/v4l2.xml
index 2ab365c..27f113f 100644
--- a/Documentation/DocBook/media/v4l/v4l2.xml
+++ b/Documentation/DocBook/media/v4l/v4l2.xml
@@ -128,6 +128,15 @@ structs, ioctls) must be noted in more detail in the history chapter
 applications. -->
 
       <revision>
+	<revnumber>3.4</revnumber>
+	<date>2012-01-06</date>
+	<authorinitials>sn</authorinitials>
+	<revremark>Added <link linkend="jpeg-controls">JPEG compression
+	    control class.</link>
+	</revremark>
+      </revision>
+
+      <revision>
 	<revnumber>3.2</revnumber>
 	<date>2011-08-26</date>
 	<authorinitials>hv</authorinitials>
diff --git a/Documentation/DocBook/media/v4l/vidioc-g-jpegcomp.xml b/Documentation/DocBook/media/v4l/vidioc-g-jpegcomp.xml
index 01ea24b..4874849 100644
--- a/Documentation/DocBook/media/v4l/vidioc-g-jpegcomp.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-g-jpegcomp.xml
@@ -57,6 +57,11 @@
   <refsect1>
     <title>Description</title>
 
+    <para>These ioctls are <emphasis role="bold">deprecated</emphasis>.
+    New drivers and applications should use <link linkend="jpeg-controls">
+    JPEG class controls</link> for image quality and JPEG markers control.
+    </para>
+
     <para>[to do]</para>
 
     <para>Ronald Bultje elaborates:</para>
@@ -86,7 +91,10 @@ to add them.</para>
 	  <row>
 	    <entry>int</entry>
 	    <entry><structfield>quality</structfield></entry>
-	    <entry></entry>
+	    <entry>Deprecated. If <link linkend="jpeg-quality-control"><constant>
+	    V4L2_CID_JPEG_IMAGE_QUALITY</constant></link> control is exposed by
+	    a driver applications should use it instead and ignore this field.
+	    </entry>
 	  </row>
 	  <row>
 	    <entry>int</entry>
@@ -116,7 +124,11 @@ to add them.</para>
 	  <row>
 	    <entry>__u32</entry>
 	    <entry><structfield>jpeg_markers</structfield></entry>
-	    <entry>See <xref linkend="jpeg-markers" />.</entry>
+	    <entry>See <xref linkend="jpeg-markers"/>. Deprecated.
+	    If <link linkend="jpeg-active-marker-control"><constant>
+	    V4L2_CID_JPEG_ACTIVE_MARKER</constant></link> control
+	    is exposed by a driver applications should use it instead
+	    and ignore this field.</entry>
 	  </row>
 	</tbody>
       </tgroup>
-- 
1.7.1

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

[Sylwester Nawrocki]

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 <moinejf@free.fr>
Signed-off-by: Sylwester Nawrocki <snjw23@gmail.com>
---
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

[PATCH/RFC v2 4/4] gspca: zc3xx: Add V4L2_CID_JPEG_COMPRESSION_QUALITY control support

[Sylwester Nawrocki]

The JPEG compression quality control is currently done by means of the
VIDIOC_S/G_JPEGCOMP ioctls. 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_S/G_JPEGCOMP ioctl handlers can be
removed, leaving the control the only user interface for compression
quality configuration.

Cc: Jean-Francois Moine <moinejf@free.fr>
Signed-off-by: Sylwester Nawrocki <snjw23@gmail.com>
---
For completeness V4L2_CID_JPEG_ACTIVE_MARKER control might be also added.
---
 drivers/media/video/gspca/zc3xx.c |   54 +++++++++++++++++++++++++-----------
 1 files changed, 37 insertions(+), 17 deletions(-)

diff --git a/drivers/media/video/gspca/zc3xx.c b/drivers/media/video/gspca/zc3xx.c
index f22e02f..019a93b 100644
--- a/drivers/media/video/gspca/zc3xx.c
+++ b/drivers/media/video/gspca/zc3xx.c
@@ -46,6 +46,7 @@ enum e_ctrl {
 	AUTOGAIN,
 	LIGHTFREQ,
 	SHARPNESS,
+	QUALITY,
 	NCTRLS		/* number of controls */
 };
 
@@ -57,11 +58,6 @@ struct sd {
 
 	struct gspca_ctrl ctrls[NCTRLS];
 
-	u8 quality;			/* image quality */
-#define QUALITY_MIN 50
-#define QUALITY_MAX 80
-#define QUALITY_DEF 70
-
 	u8 bridge;
 	u8 sensor;		/* Type of image sensor chip */
 	u16 chip_revision;
@@ -101,6 +97,12 @@ static void setexposure(struct gspca_dev *gspca_dev);
 static int sd_setautogain(struct gspca_dev *gspca_dev, __s32 val);
 static void setlightfreq(struct gspca_dev *gspca_dev);
 static void setsharpness(struct gspca_dev *gspca_dev);
+static int sd_setquality(struct gspca_dev *gspca_dev, __s32 val);
+
+/* JPEG image quality */
+#define QUALITY_MIN 50
+#define QUALITY_MAX 80
+#define QUALITY_DEF 70
 
 static const struct ctrl sd_ctrls[NCTRLS] = {
 [BRIGHTNESS] = {
@@ -188,6 +190,18 @@ static const struct ctrl sd_ctrls[NCTRLS] = {
 	    },
 	    .set_control = setsharpness
 	},
+[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,
+	    },
+	    .set = sd_setquality
+	},
 };
 
 static const struct v4l2_pix_format vga_mode[] = {
@@ -6411,7 +6425,7 @@ static int sd_config(struct gspca_dev *gspca_dev,
 	sd->sensor = id->driver_info;
 
 	gspca_dev->cam.ctrls = sd->ctrls;
-	sd->quality = QUALITY_DEF;
+	sd->ctrls[QUALITY].val = QUALITY_DEF;
 
 	return 0;
 }
@@ -6685,7 +6699,7 @@ static int sd_start(struct gspca_dev *gspca_dev)
 	/* create the JPEG header */
 	jpeg_define(sd->jpeg_hdr, gspca_dev->height, gspca_dev->width,
 			0x21);		/* JPEG 422 */
-	jpeg_set_qual(sd->jpeg_hdr, sd->quality);
+	jpeg_set_qual(sd->jpeg_hdr, sd->ctrls[QUALITY].val);
 
 	mode = gspca_dev->cam.cam_mode[gspca_dev->curr_mode].priv;
 	switch (sd->sensor) {
@@ -6893,29 +6907,35 @@ static int sd_querymenu(struct gspca_dev *gspca_dev,
 	return -EINVAL;
 }
 
-static int sd_set_jcomp(struct gspca_dev *gspca_dev,
-			struct v4l2_jpegcompression *jcomp)
+static int sd_setquality(struct gspca_dev *gspca_dev, __s32 val)
 {
 	struct sd *sd = (struct sd *) gspca_dev;
 
-	if (jcomp->quality < QUALITY_MIN)
-		sd->quality = QUALITY_MIN;
-	else if (jcomp->quality > QUALITY_MAX)
-		sd->quality = QUALITY_MAX;
-	else
-		sd->quality = jcomp->quality;
+	sd->ctrls[QUALITY].val = val;
+
 	if (gspca_dev->streaming)
-		jpeg_set_qual(sd->jpeg_hdr, sd->quality);
+		jpeg_set_qual(sd->jpeg_hdr, val);
+
 	return gspca_dev->usb_err;
 }
 
+static int sd_set_jcomp(struct gspca_dev *gspca_dev,
+			struct v4l2_jpegcompression *jcomp)
+{
+	struct sd *sd = (struct sd *) gspca_dev;
+
+	sd->ctrls[QUALITY].val = clamp_t(u8, jcomp->quality,
+					QUALITY_MIN, QUALITY_MAX);
+	return sd_setquality(gspca_dev, sd->ctrls[QUALITY].val);
+}
+
 static int sd_get_jcomp(struct gspca_dev *gspca_dev,
 			struct v4l2_jpegcompression *jcomp)
 {
 	struct sd *sd = (struct sd *) gspca_dev;
 
 	memset(jcomp, 0, sizeof *jcomp);
-	jcomp->quality = sd->quality;
+	jcomp->quality = sd->ctrls[QUALITY].val;
 	jcomp->jpeg_markers = V4L2_JPEG_MARKER_DHT
 			| V4L2_JPEG_MARKER_DQT;
 	return 0;
-- 
1.7.1

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

[Jean-Francois Moine]

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

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

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

[Jean-Francois Moine]

On Fri,  6 Jan 2012 19:14:42 +0100
Sylwester Nawrocki <snjw23@gmail.com> wrote:

> The JPEG compression quality control is currently done by means of the
> VIDIOC_S/G_JPEGCOMP ioctls. 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_S/G_JPEGCOMP ioctl handlers can be
> removed, leaving the control the only user interface for compression
> quality configuration.

This patch works, but it may be simplified.

Instead of a '.set' pointer, the control descriptor for QUALITY may contain a '.set_control' pointing to a function which just does

	jpeg_set_qual(sd->jpeg_hdr, sd->ctrls[QUALITY].val);

this function being also be called from the obsoleted function
sd_setquality().

Also, in sd_config, there is no need to initialize the variable
sd->ctrls[QUALITY].val.

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

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

[Sylwester Nawrocki]

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

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

[Jean-Francois Moine]

On Sat, 14 Jan 2012 18:42:20 +0100
Sylwester Nawrocki <sylvester.nawrocki@gmail.com> 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/

[PATCH/RFC v3 0/3] JPEG codecs control class

[Sylwester Nawrocki]

Hello,

this patch set adds new control class - V4L2_CID_JPEG_CLASS initially
containing four controls. It also adds V4L2_CID_JPEG_COMPRESSION_QUALITY
control to zc3xx gspca sub-device. The gspca patch has been tested with
v4l2ucp and v4l2-ctl.

The initial RFC can be found at
http://www.mail-archive.com/linux-media@vger.kernel.org/msg39012.html

Changes since v2:
 - improved gspca/z3cxx patch according Jef's comments;
 - removed patch adding jpeg quality control to gspca/sonixj;
 - patches 1/3, 2/3 are unchanged;

Changes since v1:
 - removed trailing 'S' from V4L2_CID_JPEG_ACTIVE_MARKERS;
 - removed V4L2_JPEG_ACTIVE_MARKER_DAC and V4L2_JPEG_ACTIVE_MARKER_DNL
   definitions as these are normally controlled by higher level compression
   attributes and shouldn't be allowed to be discarded independently;


Thanks,
Sylwester

Sylwester Nawrocki (3):
  V4L: Add JPEG compression control class
  V4L: Add JPEG compression control class documentation
  gspca: zc3xx: Add V4L2_CID_JPEG_COMPRESSION_QUALITY control support

 Documentation/DocBook/media/v4l/biblio.xml         |   20 +++
 Documentation/DocBook/media/v4l/compat.xml         |   10 ++
 Documentation/DocBook/media/v4l/controls.xml       |  161 ++++++++++++++++++++
 Documentation/DocBook/media/v4l/v4l2.xml           |    9 +
 .../DocBook/media/v4l/vidioc-g-jpegcomp.xml        |   16 ++-
 drivers/media/video/gspca/zc3xx.c                  |   45 ++++--
 drivers/media/video/v4l2-ctrls.c                   |   24 +++
 include/linux/videodev2.h                          |   24 +++
 8 files changed, 292 insertions(+), 17 deletions(-)

--
1.7.4.1

[PATCH/RFC v3 1/3] V4L: Add JPEG compression control class

[Sylwester Nawrocki]

The V4L2_CID_JPEG_CLASS control class is intended to expose various
adjustable parameters of JPEG encoders and decoders. Following controls
are defined:

 - V4L2_CID_JPEG_CHROMA_SUBSAMPLING,
 - V4L2_CID_JPEG_RESTART_INTERVAL,
 - V4L2_CID_JPEG_COMPRESSION_QUALITY,
 - V4L2_CID_JPEG_ACTIVE_MARKER.

This covers only a part of relevant standard specifications. More
controls should be added in future if required.

The purpose of V4L2_CID_JPEG_CLASS class is also to replace some
functionality covered by VIDIOC_S/G_JPEGCOMP ioctls, i.e. the JPEG
markers presence and compression quality control. The applications
and drivers should switch from the ioctl to control based API, as
described in the subsequent patches for the Media API DocBook.

Signed-off-by: Sylwester Nawrocki <sylvester.nawrocki@gmail.com>
---
 drivers/media/video/v4l2-ctrls.c |   24 ++++++++++++++++++++++++
 include/linux/videodev2.h        |   24 ++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/v4l2-ctrls.c b/drivers/media/video/v4l2-ctrls.c
index da1f4c2..1fd6435 100644
--- a/drivers/media/video/v4l2-ctrls.c
+++ b/drivers/media/video/v4l2-ctrls.c
@@ -353,6 +353,16 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
 		NULL,
 	};

+	static const char * const jpeg_chroma_subsampling[] = {
+		"4:4:4",
+		"4:2:2",
+		"4:2:0",
+		"4:1:1",
+		"4:1:0",
+		"Gray",
+		NULL,
+	};
+
 	switch (id) {
 	case V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ:
 		return mpeg_audio_sampling_freq;
@@ -414,6 +424,9 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
 		return mpeg_mpeg4_level;
 	case V4L2_CID_MPEG_VIDEO_MPEG4_PROFILE:
 		return mpeg4_profile;
+	case V4L2_CID_JPEG_CHROMA_SUBSAMPLING:
+		return jpeg_chroma_subsampling;
+
 	default:
 		return NULL;
 	}
@@ -607,6 +620,14 @@ const char *v4l2_ctrl_get_name(u32 id)
 	case V4L2_CID_FLASH_CHARGE:		return "Charge";
 	case V4L2_CID_FLASH_READY:		return "Ready to strobe";

+	/* JPEG encoder controls */
+	/* Keep the order of the 'case's the same as in videodev2.h! */
+	case V4L2_CID_JPEG_CLASS:		return "JPEG Compression Controls";
+	case V4L2_CID_JPEG_CHROMA_SUBSAMPLING:	return "Chroma Subsampling";
+	case V4L2_CID_JPEG_RESTART_INTERVAL:	return "Restart Interval";
+	case V4L2_CID_JPEG_COMPRESSION_QUALITY:	return "Compression Quality";
+	case V4L2_CID_JPEG_ACTIVE_MARKER:	return "Active Markers";
+
 	default:
 		return NULL;
 	}
@@ -693,6 +714,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_MPEG_VIDEO_H264_VUI_SAR_IDC:
 	case V4L2_CID_MPEG_VIDEO_MPEG4_LEVEL:
 	case V4L2_CID_MPEG_VIDEO_MPEG4_PROFILE:
+	case V4L2_CID_JPEG_CHROMA_SUBSAMPLING:
 		*type = V4L2_CTRL_TYPE_MENU;
 		break;
 	case V4L2_CID_RDS_TX_PS_NAME:
@@ -704,6 +726,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_MPEG_CLASS:
 	case V4L2_CID_FM_TX_CLASS:
 	case V4L2_CID_FLASH_CLASS:
+	case V4L2_CID_JPEG_CLASS:
 		*type = V4L2_CTRL_TYPE_CTRL_CLASS;
 		/* You can neither read not write these */
 		*flags |= V4L2_CTRL_FLAG_READ_ONLY | V4L2_CTRL_FLAG_WRITE_ONLY;
@@ -717,6 +740,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 		*max = 0xFFFFFF;
 		break;
 	case V4L2_CID_FLASH_FAULT:
+	case V4L2_CID_JPEG_ACTIVE_MARKER:
 		*type = V4L2_CTRL_TYPE_BITMASK;
 		break;
 	case V4L2_CID_MIN_BUFFERS_FOR_CAPTURE:
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 012a296..5bd3725 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -1123,6 +1123,7 @@ struct v4l2_ext_controls {
 #define V4L2_CTRL_CLASS_CAMERA 0x009a0000	/* Camera class controls */
 #define V4L2_CTRL_CLASS_FM_TX 0x009b0000	/* FM Modulator control class */
 #define V4L2_CTRL_CLASS_FLASH 0x009c0000	/* Camera flash controls */
+#define V4L2_CTRL_CLASS_JPEG 0x009d0000		/* JPEG-compression controls */

 #define V4L2_CTRL_ID_MASK      	  (0x0fffffff)
 #define V4L2_CTRL_ID2CLASS(id)    ((id) & 0x0fff0000UL)
@@ -1732,6 +1733,29 @@ enum v4l2_flash_strobe_source {
 #define V4L2_CID_FLASH_CHARGE			(V4L2_CID_FLASH_CLASS_BASE + 11)
 #define V4L2_CID_FLASH_READY			(V4L2_CID_FLASH_CLASS_BASE + 12)

+/*  JPEG-class control IDs defined by V4L2 */
+#define V4L2_CID_JPEG_CLASS_BASE		(V4L2_CTRL_CLASS_JPEG | 0x900)
+#define V4L2_CID_JPEG_CLASS			(V4L2_CTRL_CLASS_JPEG | 1)
+
+#define	V4L2_CID_JPEG_CHROMA_SUBSAMPLING	(V4L2_CID_JPEG_CLASS_BASE + 1)
+enum v4l2_jpeg_chroma_subsampling {
+	V4L2_JPEG_CHROMA_SUBSAMPLING_444	= 0,
+	V4L2_JPEG_CHROMA_SUBSAMPLING_422	= 1,
+	V4L2_JPEG_CHROMA_SUBSAMPLING_420	= 2,
+	V4L2_JPEG_CHROMA_SUBSAMPLING_411	= 3,
+	V4L2_JPEG_CHROMA_SUBSAMPLING_410	= 4,
+	V4L2_JPEG_CHROMA_SUBSAMPLING_GRAY	= 5,
+};
+#define	V4L2_CID_JPEG_RESTART_INTERVAL		(V4L2_CID_JPEG_CLASS_BASE + 2)
+#define	V4L2_CID_JPEG_COMPRESSION_QUALITY	(V4L2_CID_JPEG_CLASS_BASE + 3)
+
+#define	V4L2_CID_JPEG_ACTIVE_MARKER		(V4L2_CID_JPEG_CLASS_BASE + 4)
+#define	V4L2_JPEG_ACTIVE_MARKER_APP0		(1 << 0)
+#define	V4L2_JPEG_ACTIVE_MARKER_APP1		(1 << 1)
+#define	V4L2_JPEG_ACTIVE_MARKER_COM		(1 << 16)
+#define	V4L2_JPEG_ACTIVE_MARKER_DQT		(1 << 17)
+#define	V4L2_JPEG_ACTIVE_MARKER_DHT		(1 << 18)
+
 /*
  *	T U N I N G
  */
--
1.7.4.1

[PATCH/RFC v3 2/3] V4L: Add JPEG compression control class documentation

[Sylwester Nawrocki]

Add DocBook entries for the JPEG control class.

Signed-off-by: Sylwester Nawrocki <sylvester.nawrocki@gmail.com>
---
 Documentation/DocBook/media/v4l/biblio.xml         |   20 +++
 Documentation/DocBook/media/v4l/compat.xml         |   10 ++
 Documentation/DocBook/media/v4l/controls.xml       |  161 ++++++++++++++++++++
 Documentation/DocBook/media/v4l/v4l2.xml           |    9 +
 .../DocBook/media/v4l/vidioc-g-jpegcomp.xml        |   16 ++-
 5 files changed, 214 insertions(+), 2 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/biblio.xml b/Documentation/DocBook/media/v4l/biblio.xml
index cea6fd3..7dc65c5 100644
--- a/Documentation/DocBook/media/v4l/biblio.xml
+++ b/Documentation/DocBook/media/v4l/biblio.xml
@@ -128,6 +128,26 @@ url="http://www.ijg.org">http://www.ijg.org</ulink>)</corpauthor>
       <subtitle>Version 1.02</subtitle>
     </biblioentry>

+    <biblioentry id="itu-t81">
+      <abbrev>ITU-T.81</abbrev>
+      <authorgroup>
+	<corpauthor>International Telecommunication Union
+(<ulink url="http://www.itu.int">http://www.itu.int</ulink>)</corpauthor>
+      </authorgroup>
+      <title>ITU-T Recommendation T.81
+"Information Technology &mdash; Digital Compression and Coding of Continous-Tone
+Still Images &mdash; Requirements and Guidelines"</title>
+    </biblioentry>
+
+    <biblioentry id="w3c-jpeg-jfif">
+      <abbrev>W3C JPEG JFIF</abbrev>
+      <authorgroup>
+	<corpauthor>The World Wide Web Consortium (<ulink
+url="http://www.w3.org/Graphics/JPEG">http://www.w3.org</ulink>)</corpauthor>
+      </authorgroup>
+      <title>JPEG JFIF</title>
+    </biblioentry>
+
     <biblioentry id="smpte12m">
       <abbrev>SMPTE&nbsp;12M</abbrev>
       <authorgroup>
diff --git a/Documentation/DocBook/media/v4l/compat.xml b/Documentation/DocBook/media/v4l/compat.xml
index c736380..9136038 100644
--- a/Documentation/DocBook/media/v4l/compat.xml
+++ b/Documentation/DocBook/media/v4l/compat.xml
@@ -2396,6 +2396,16 @@ details.</para>
       </orderedlist>
     </section>

+    <section>
+      <title>V4L2 in Linux 3.4</title>
+      <orderedlist>
+        <listitem>
+	  <para>Added <link linkend="jpeg-controls">JPEG compression control
+	  class</link>.</para>
+        </listitem>
+      </orderedlist>
+    </section>
+
     <section id="other">
       <title>Relation of V4L2 to other Linux multimedia APIs</title>

diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml
index a1be378..b99f58b 100644
--- a/Documentation/DocBook/media/v4l/controls.xml
+++ b/Documentation/DocBook/media/v4l/controls.xml
@@ -3377,6 +3377,167 @@ interface and may change in the future.</para>
 	</tbody>
       </tgroup>
       </table>
+    </section>
+
+    <section id="jpeg-controls">
+      <title>JPEG Control Reference</title>
+      <para>The JPEG class includes controls for common features of JPEG
+      encoders and decoders. Currently it includes features for codecs
+      implementing progressive baseline DCT compression process with
+      Huffman entrophy coding.</para>
+      <table pgwide="1" frame="none" id="jpeg-control-id">
+      <title>JPEG Control IDs</title>
+
+      <tgroup cols="4">
+	<colspec colname="c1" colwidth="1*" />
+	<colspec colname="c2" colwidth="6*" />
+	<colspec colname="c3" colwidth="2*" />
+	<colspec colname="c4" colwidth="6*" />
+	<spanspec namest="c1" nameend="c2" spanname="id" />
+	<spanspec namest="c2" nameend="c4" spanname="descr" />
+	<thead>
+	  <row>
+	    <entry spanname="id" align="left">ID</entry>
+	    <entry align="left">Type</entry>
+	  </row><row rowsep="1"><entry spanname="descr" align="left">Description</entry>
+	  </row>
+	</thead>
+	<tbody valign="top">
+	  <row><entry></entry></row>
+	  <row>
+	    <entry spanname="id"><constant>V4L2_CID_JPEG_CLASS</constant>&nbsp;</entry>
+	    <entry>class</entry>
+	  </row><row><entry spanname="descr">The JPEG class descriptor. Calling
+	  &VIDIOC-QUERYCTRL; for this control will return a description of this
+	  control class.
+
+	</entry>
+	  </row>
+	  <row>
+	    <entry spanname="id"><constant>V4L2_CID_JPEG_CHROMA_SUBSAMPLING</constant></entry>
+	    <entry>menu</entry>
+	  </row>
+	  <row id="jpeg-chroma-subsampling-control">
+	    <entry spanname="descr">The chroma subsampling factors describe how
+	    each component of an input image is sampled, in respect to maximum
+	    sample rate in each spatial dimension. See <xref linkend="itu-t81"/>,
+	    clause A.1.1. for more details. The <constant>
+	    V4L2_CID_JPEG_CHROMA_SUBSAMPLING</constant> control determines how
+	    Cb and Cr components are downsampled after coverting an input image
+	    from RGB to Y'CbCr color space.
+	    </entry>
+	  </row>
+	  <row>
+	    <entrytbl spanname="descr" cols="2">
+	      <tbody valign="top">
+		<row>
+		  <entry><constant>V4L2_JPEG_CHROMA_SUBSAMPLING_444</constant>
+		  </entry><entry>No chroma subsampling, each pixel has
+		  Y, Cr and Cb values.</entry>
+		</row>
+		<row>
+		  <entry><constant>V4L2_JPEG_CHROMA_SUBSAMPLING_422</constant>
+		  </entry><entry>Horizontally subsample Cr, Cb components
+		  by a factor of 2.</entry>
+		</row>
+		<row>
+		  <entry><constant>V4L2_JPEG_CHROMA_SUBSAMPLING_420</constant>
+		  </entry><entry>Subsample Cr, Cb components horizontally
+		  and vertically by 2.</entry>
+		</row>
+		<row>
+		  <entry><constant>V4L2_JPEG_CHROMA_SUBSAMPLING_411</constant>
+		  </entry><entry>Horizontally subsample Cr, Cb components
+		  by a factor of 4.</entry>
+		</row>
+		<row>
+		  <entry><constant>V4L2_JPEG_CHROMA_SUBSAMPLING_410</constant>
+		  </entry><entry>Subsample Cr, Cb components horizontally
+		  by 4 and vertically by 2.</entry>
+		</row>
+		<row>
+		  <entry><constant>V4L2_JPEG_CHROMA_SUBSAMPLING_GRAY</constant>
+		  </entry><entry>Use only luminance component.</entry>
+		</row>
+	      </tbody>
+	    </entrytbl>
+	  </row>
+	  <row>
+	    <entry spanname="id"><constant>V4L2_CID_JPEG_RESTART_INTERVAL</constant>
+	    </entry><entry>integer</entry>
+	  </row>
+	  <row><entry spanname="descr">
+	      The restart interval determines an interval of inserting RSTm
+	      markers (m = 0..7). The purpose of these markers is to additionally
+	      reinitialize the encoder process, in order to process blocks of
+	      an image independently.
+	      For the lossy compression processes the restart interval unit is
+	      MCU (Minimum Coded Unit) and its value is contained in DRI
+	      (Define Restart Interval) marker. If <constant>
+	      V4L2_CID_JPEG_RESTART_INTERVAL</constant> control is set to 0,
+	      DRI and RSTm markers will not be inserted.
+	    </entry>
+	  </row>
+	  <row id="jpeg-quality-control">
+	    <entry spanname="id"><constant>V4L2_CID_JPEG_COMPRESION_QUALITY</constant></entry>
+	    <entry>integer</entry>
+	  </row>
+	  <row>
+	    <entry spanname="descr">
+	      <constant>V4L2_CID_JPEG_COMPRESION_QUALITY</constant> control
+	      determines trade-off between image quality and size.
+	      It provides simpler method for applications to control image quality,
+	      without a need for direct reconfiguration of luminance and chrominance
+	      quantization tables.
+
+	      In cases where a driver uses quantization tables configured directly
+	      by an application, using interfaces defined elsewhere, <constant>
+	      V4L2_CID_JPEG_COMPRESION_QUALITY</constant> control should be set
+	      by driver to 0.

+	      <para>The value range of this control is driver-specific. Only
+	      positive, non-zero values are meaningful. The recommended range
+	      is 1 - 100, where larger values correspond to better image quality.
+	      </para>
+	    </entry>
+	    </row>
+	  <row id="jpeg-active-marker-control">
+	    <entry spanname="id"><constant>V4L2_CID_JPEG_ACTIVE_MARKER</constant></entry>
+	    <entry>bitmask</entry>
+	  </row>
+	  <row>
+	    <entry spanname="descr">Specify which JPEG markers are included
+	    in compressed stream. This control is valid only for encoders.
+	    </entry>
+	  </row>
+	  <row>
+	    <entrytbl spanname="descr" cols="2">
+	      <tbody valign="top">
+		<row>
+		  <entry><constant>V4L2_JPEG_ACTIVE_MARKER_APP0</constant></entry>
+		  <entry>Application data segment APP<subscript>0</subscript>.</entry>
+		</row><row>
+		  <entry><constant>V4L2_JPEG_ACTIVE_MARKER_APP1</constant></entry>
+		  <entry>Application data segment APP<subscript>1</subscript>.</entry>
+		</row><row>
+		  <entry><constant>V4L2_JPEG_ACTIVE_MARKER_COM</constant></entry>
+		  <entry>Comment segment.</entry>
+		</row><row>
+		  <entry><constant>V4L2_JPEG_ACTIVE_MARKER_DQT</constant></entry>
+		  <entry>Quantization tables segment.</entry>
+		</row><row>
+		  <entry><constant>V4L2_JPEG_ACTIVE_MARKER_DHT</constant></entry>
+		  <entry>Huffman tables segment.</entry>
+		</row>
+	      </tbody>
+	    </entrytbl>
+	  </row>
+	  <row><entry></entry></row>
+	</tbody>
+      </tgroup>
+      </table>
+      <para>For more details about JPEG specification, refer
+      to <xref linkend="itu-t81"/>, <xref linkend="jfif"/>,
+      <xref linkend="w3c-jpeg-jfif"/>.</para>
     </section>
 </section>
diff --git a/Documentation/DocBook/media/v4l/v4l2.xml b/Documentation/DocBook/media/v4l/v4l2.xml
index e97c512..44f3820 100644
--- a/Documentation/DocBook/media/v4l/v4l2.xml
+++ b/Documentation/DocBook/media/v4l/v4l2.xml
@@ -128,6 +128,15 @@ structs, ioctls) must be noted in more detail in the history chapter
 applications. -->

       <revision>
+	<revnumber>3.4</revnumber>
+	<date>2012-01-06</date>
+	<authorinitials>sn</authorinitials>
+	<revremark>Added <link linkend="jpeg-controls">JPEG compression
+	    control class.</link>
+	</revremark>
+      </revision>
+
+      <revision>
 	<revnumber>3.2</revnumber>
 	<date>2011-08-26</date>
 	<authorinitials>hv</authorinitials>
diff --git a/Documentation/DocBook/media/v4l/vidioc-g-jpegcomp.xml b/Documentation/DocBook/media/v4l/vidioc-g-jpegcomp.xml
index 01ea24b..4874849 100644
--- a/Documentation/DocBook/media/v4l/vidioc-g-jpegcomp.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-g-jpegcomp.xml
@@ -57,6 +57,11 @@
   <refsect1>
     <title>Description</title>

+    <para>These ioctls are <emphasis role="bold">deprecated</emphasis>.
+    New drivers and applications should use <link linkend="jpeg-controls">
+    JPEG class controls</link> for image quality and JPEG markers control.
+    </para>
+
     <para>[to do]</para>

     <para>Ronald Bultje elaborates:</para>
@@ -86,7 +91,10 @@ to add them.</para>
 	  <row>
 	    <entry>int</entry>
 	    <entry><structfield>quality</structfield></entry>
-	    <entry></entry>
+	    <entry>Deprecated. If <link linkend="jpeg-quality-control"><constant>
+	    V4L2_CID_JPEG_IMAGE_QUALITY</constant></link> control is exposed by
+	    a driver applications should use it instead and ignore this field.
+	    </entry>
 	  </row>
 	  <row>
 	    <entry>int</entry>
@@ -116,7 +124,11 @@ to add them.</para>
 	  <row>
 	    <entry>__u32</entry>
 	    <entry><structfield>jpeg_markers</structfield></entry>
-	    <entry>See <xref linkend="jpeg-markers" />.</entry>
+	    <entry>See <xref linkend="jpeg-markers"/>. Deprecated.
+	    If <link linkend="jpeg-active-marker-control"><constant>
+	    V4L2_CID_JPEG_ACTIVE_MARKER</constant></link> control
+	    is exposed by a driver applications should use it instead
+	    and ignore this field.</entry>
 	  </row>
 	</tbody>
       </tgroup>
--
1.7.4.1

[PATCH/RFC v3 3/3] gspca: zc3xx: Add V4L2_CID_JPEG_COMPRESSION_QUALITY control support

[Sylwester Nawrocki]

The JPEG compression quality control is currently done by means of the
VIDIOC_S/G_JPEGCOMP ioctls. 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_S/G_JPEGCOMP ioctl handlers can be
removed, leaving the control the only user interface for compression
quality configuration.

Cc: Jean-Francois Moine <moinejf@free.fr>
Signed-off-by: Sylwester Nawrocki <sylvester.nawrocki@gmail.com>
---
 drivers/media/video/gspca/zc3xx.c |   45 ++++++++++++++++++++++++------------
 1 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/drivers/media/video/gspca/zc3xx.c b/drivers/media/video/gspca/zc3xx.c
index f22e02f..b6a18c8 100644
--- a/drivers/media/video/gspca/zc3xx.c
+++ b/drivers/media/video/gspca/zc3xx.c
@@ -46,6 +46,7 @@ enum e_ctrl {
 	AUTOGAIN,
 	LIGHTFREQ,
 	SHARPNESS,
+	QUALITY,
 	NCTRLS		/* number of controls */
 };

@@ -57,11 +58,6 @@ struct sd {

 	struct gspca_ctrl ctrls[NCTRLS];

-	u8 quality;			/* image quality */
-#define QUALITY_MIN 50
-#define QUALITY_MAX 80
-#define QUALITY_DEF 70
-
 	u8 bridge;
 	u8 sensor;		/* Type of image sensor chip */
 	u16 chip_revision;
@@ -101,6 +97,12 @@ static void setexposure(struct gspca_dev *gspca_dev);
 static int sd_setautogain(struct gspca_dev *gspca_dev, __s32 val);
 static void setlightfreq(struct gspca_dev *gspca_dev);
 static void setsharpness(struct gspca_dev *gspca_dev);
+static void set_quality(struct gspca_dev *gspca_dev);
+
+/* JPEG image quality */
+#define QUALITY_MIN 50
+#define QUALITY_MAX 80
+#define QUALITY_DEF 70

 static const struct ctrl sd_ctrls[NCTRLS] = {
 [BRIGHTNESS] = {
@@ -188,6 +190,18 @@ static const struct ctrl sd_ctrls[NCTRLS] = {
 	    },
 	    .set_control = setsharpness
 	},
+[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,
+	    },
+	    .set_control = set_quality
+	},
 };

 static const struct v4l2_pix_format vga_mode[] = {
@@ -6411,7 +6425,6 @@ static int sd_config(struct gspca_dev *gspca_dev,
 	sd->sensor = id->driver_info;

 	gspca_dev->cam.ctrls = sd->ctrls;
-	sd->quality = QUALITY_DEF;

 	return 0;
 }
@@ -6685,7 +6698,7 @@ static int sd_start(struct gspca_dev *gspca_dev)
 	/* create the JPEG header */
 	jpeg_define(sd->jpeg_hdr, gspca_dev->height, gspca_dev->width,
 			0x21);		/* JPEG 422 */
-	jpeg_set_qual(sd->jpeg_hdr, sd->quality);
+	jpeg_set_qual(sd->jpeg_hdr, sd->ctrls[QUALITY].val);

 	mode = gspca_dev->cam.cam_mode[gspca_dev->curr_mode].priv;
 	switch (sd->sensor) {
@@ -6893,19 +6906,21 @@ static int sd_querymenu(struct gspca_dev *gspca_dev,
 	return -EINVAL;
 }

+static void set_quality(struct gspca_dev *gspca_dev)
+{
+	struct sd *sd = (struct sd *) gspca_dev;
+	jpeg_set_qual(sd->jpeg_hdr, sd->ctrls[QUALITY].val);
+}
+
 static int sd_set_jcomp(struct gspca_dev *gspca_dev,
 			struct v4l2_jpegcompression *jcomp)
 {
 	struct sd *sd = (struct sd *) gspca_dev;

-	if (jcomp->quality < QUALITY_MIN)
-		sd->quality = QUALITY_MIN;
-	else if (jcomp->quality > QUALITY_MAX)
-		sd->quality = QUALITY_MAX;
-	else
-		sd->quality = jcomp->quality;
+	sd->ctrls[QUALITY].val = clamp_t(u8, jcomp->quality,
+					 QUALITY_MIN, QUALITY_MAX);
 	if (gspca_dev->streaming)
-		jpeg_set_qual(sd->jpeg_hdr, sd->quality);
+		set_quality(gspca_dev);
 	return gspca_dev->usb_err;
 }

@@ -6915,7 +6930,7 @@ static int sd_get_jcomp(struct gspca_dev *gspca_dev,
 	struct sd *sd = (struct sd *) gspca_dev;

 	memset(jcomp, 0, sizeof *jcomp);
-	jcomp->quality = sd->quality;
+	jcomp->quality = sd->ctrls[QUALITY].val;
 	jcomp->jpeg_markers = V4L2_JPEG_MARKER_DHT
 			| V4L2_JPEG_MARKER_DQT;
 	return 0;
--
1.7.4.1

[PATCH/RFC v4 3/3] gspca: zc3xx: Add V4L2_CID_JPEG_COMPRESSION_QUALITY control support

[Sylwester Nawrocki]

The JPEG compression quality control is currently done by means of the
VIDIOC_S/G_JPEGCOMP ioctls. 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_S/G_JPEGCOMP ioctl handlers can be
removed, leaving the control the only user interface for compression
quality configuration.

Cc: Jean-Francois Moine <moinejf@free.fr>
Signed-off-by: Sylwester Nawrocki <sylvester.nawrocki@gmail.com>
---
Removed unneeded substitution of jpeg_set_qual() with set_quality().

 drivers/media/video/gspca/zc3xx.c |   43 +++++++++++++++++++++++++------------
 1 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/drivers/media/video/gspca/zc3xx.c b/drivers/media/video/gspca/zc3xx.c
index f22e02f..1b78598 100644
--- a/drivers/media/video/gspca/zc3xx.c
+++ b/drivers/media/video/gspca/zc3xx.c
@@ -46,6 +46,7 @@ enum e_ctrl {
 	AUTOGAIN,
 	LIGHTFREQ,
 	SHARPNESS,
+	QUALITY,
 	NCTRLS		/* number of controls */
 };
 
@@ -57,11 +58,6 @@ struct sd {
 
 	struct gspca_ctrl ctrls[NCTRLS];
 
-	u8 quality;			/* image quality */
-#define QUALITY_MIN 50
-#define QUALITY_MAX 80
-#define QUALITY_DEF 70
-
 	u8 bridge;
 	u8 sensor;		/* Type of image sensor chip */
 	u16 chip_revision;
@@ -101,6 +97,12 @@ static void setexposure(struct gspca_dev *gspca_dev);
 static int sd_setautogain(struct gspca_dev *gspca_dev, __s32 val);
 static void setlightfreq(struct gspca_dev *gspca_dev);
 static void setsharpness(struct gspca_dev *gspca_dev);
+static void set_quality(struct gspca_dev *gspca_dev);
+
+/* JPEG image quality */
+#define QUALITY_MIN 50
+#define QUALITY_MAX 80
+#define QUALITY_DEF 70
 
 static const struct ctrl sd_ctrls[NCTRLS] = {
 [BRIGHTNESS] = {
@@ -188,6 +190,18 @@ static const struct ctrl sd_ctrls[NCTRLS] = {
 	    },
 	    .set_control = setsharpness
 	},
+[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,
+	    },
+	    .set_control = set_quality
+	},
 };
 
 static const struct v4l2_pix_format vga_mode[] = {
@@ -6411,7 +6425,6 @@ static int sd_config(struct gspca_dev *gspca_dev,
 	sd->sensor = id->driver_info;
 
 	gspca_dev->cam.ctrls = sd->ctrls;
-	sd->quality = QUALITY_DEF;
 
 	return 0;
 }
@@ -6685,7 +6698,7 @@ static int sd_start(struct gspca_dev *gspca_dev)
 	/* create the JPEG header */
 	jpeg_define(sd->jpeg_hdr, gspca_dev->height, gspca_dev->width,
 			0x21);		/* JPEG 422 */
-	jpeg_set_qual(sd->jpeg_hdr, sd->quality);
+	jpeg_set_qual(sd->jpeg_hdr, sd->ctrls[QUALITY].val);
 
 	mode = gspca_dev->cam.cam_mode[gspca_dev->curr_mode].priv;
 	switch (sd->sensor) {
@@ -6893,17 +6906,19 @@ static int sd_querymenu(struct gspca_dev *gspca_dev,
 	return -EINVAL;
 }
 
+static void set_quality(struct gspca_dev *gspca_dev)
+{
+	struct sd *sd = (struct sd *) gspca_dev;
+	jpeg_set_qual(sd->jpeg_hdr, sd->ctrls[QUALITY].val);
+}
+
 static int sd_set_jcomp(struct gspca_dev *gspca_dev,
 			struct v4l2_jpegcompression *jcomp)
 {
 	struct sd *sd = (struct sd *) gspca_dev;
 
-	if (jcomp->quality < QUALITY_MIN)
-		sd->quality = QUALITY_MIN;
-	else if (jcomp->quality > QUALITY_MAX)
-		sd->quality = QUALITY_MAX;
-	else
-		sd->quality = jcomp->quality;
+	sd->ctrls[QUALITY].val = clamp_t(u8, jcomp->quality,
+					 QUALITY_MIN, QUALITY_MAX);
 	if (gspca_dev->streaming)
 		jpeg_set_qual(sd->jpeg_hdr, sd->quality);
 	return gspca_dev->usb_err;
@@ -6915,7 +6930,7 @@ static int sd_get_jcomp(struct gspca_dev *gspca_dev,
 	struct sd *sd = (struct sd *) gspca_dev;
 
 	memset(jcomp, 0, sizeof *jcomp);
-	jcomp->quality = sd->quality;
+	jcomp->quality = sd->ctrls[QUALITY].val;
 	jcomp->jpeg_markers = V4L2_JPEG_MARKER_DHT
 			| V4L2_JPEG_MARKER_DQT;
 	return 0;
-- 
1.7.4.1

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

[Sylwester Nawrocki]

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

Re: [PATCH/RFC v4 3/3] gspca: zc3xx: Add V4L2_CID_JPEG_COMPRESSION_QUALITY control support

[Sylwester Nawrocki]

On 01/14/2012 08:53 PM, Sylwester Nawrocki wrote:
> The JPEG compression quality control is currently done by means of the
> VIDIOC_S/G_JPEGCOMP ioctls. 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_S/G_JPEGCOMP ioctl handlers can be
> removed, leaving the control the only user interface for compression
> quality configuration.
> 
> Cc: Jean-Francois Moine<moinejf@free.fr>
> Signed-off-by: Sylwester Nawrocki<sylvester.nawrocki@gmail.com>

Dear Jean-Francois,

is this patch looking OK or you, or would you have any further remarks ?
I'd like to add it to a pull request in coming week, together with remaining
patches in this series plus a patch for some SoC JPEG codec driver.
Please let me know if there is anything that could be changed/improved.

--
Best regards,
Sylwester

> ---
> Removed unneeded substitution of jpeg_set_qual() with set_quality().
> 
>   drivers/media/video/gspca/zc3xx.c |   43 +++++++++++++++++++++++++------------
>   1 files changed, 29 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/media/video/gspca/zc3xx.c b/drivers/media/video/gspca/zc3xx.c
> index f22e02f..1b78598 100644
> --- a/drivers/media/video/gspca/zc3xx.c
> +++ b/drivers/media/video/gspca/zc3xx.c
> @@ -46,6 +46,7 @@ enum e_ctrl {
>   	AUTOGAIN,
>   	LIGHTFREQ,
>   	SHARPNESS,
> +	QUALITY,
>   	NCTRLS		/* number of controls */
>   };
> 
> @@ -57,11 +58,6 @@ struct sd {
> 
>   	struct gspca_ctrl ctrls[NCTRLS];
> 
> -	u8 quality;			/* image quality */
> -#define QUALITY_MIN 50
> -#define QUALITY_MAX 80
> -#define QUALITY_DEF 70
> -
>   	u8 bridge;
>   	u8 sensor;		/* Type of image sensor chip */
>   	u16 chip_revision;
> @@ -101,6 +97,12 @@ static void setexposure(struct gspca_dev *gspca_dev);
>   static int sd_setautogain(struct gspca_dev *gspca_dev, __s32 val);
>   static void setlightfreq(struct gspca_dev *gspca_dev);
>   static void setsharpness(struct gspca_dev *gspca_dev);
> +static void set_quality(struct gspca_dev *gspca_dev);
> +
> +/* JPEG image quality */
> +#define QUALITY_MIN 50
> +#define QUALITY_MAX 80
> +#define QUALITY_DEF 70
> 
>   static const struct ctrl sd_ctrls[NCTRLS] = {
>   [BRIGHTNESS] = {
> @@ -188,6 +190,18 @@ static const struct ctrl sd_ctrls[NCTRLS] = {
>   	    },
>   	    .set_control = setsharpness
>   	},
> +[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,
> +	    },
> +	    .set_control = set_quality
> +	},
>   };
> 
>   static const struct v4l2_pix_format vga_mode[] = {
> @@ -6411,7 +6425,6 @@ static int sd_config(struct gspca_dev *gspca_dev,
>   	sd->sensor = id->driver_info;
> 
>   	gspca_dev->cam.ctrls = sd->ctrls;
> -	sd->quality = QUALITY_DEF;
> 
>   	return 0;
>   }
> @@ -6685,7 +6698,7 @@ static int sd_start(struct gspca_dev *gspca_dev)
>   	/* create the JPEG header */
>   	jpeg_define(sd->jpeg_hdr, gspca_dev->height, gspca_dev->width,
>   			0x21);		/* JPEG 422 */
> -	jpeg_set_qual(sd->jpeg_hdr, sd->quality);
> +	jpeg_set_qual(sd->jpeg_hdr, sd->ctrls[QUALITY].val);
> 
>   	mode = gspca_dev->cam.cam_mode[gspca_dev->curr_mode].priv;
>   	switch (sd->sensor) {
> @@ -6893,17 +6906,19 @@ static int sd_querymenu(struct gspca_dev *gspca_dev,
>   	return -EINVAL;
>   }
> 
> +static void set_quality(struct gspca_dev *gspca_dev)
> +{
> +	struct sd *sd = (struct sd *) gspca_dev;
> +	jpeg_set_qual(sd->jpeg_hdr, sd->ctrls[QUALITY].val);
> +}
> +
>   static int sd_set_jcomp(struct gspca_dev *gspca_dev,
>   			struct v4l2_jpegcompression *jcomp)
>   {
>   	struct sd *sd = (struct sd *) gspca_dev;
> 
> -	if (jcomp->quality<  QUALITY_MIN)
> -		sd->quality = QUALITY_MIN;
> -	else if (jcomp->quality>  QUALITY_MAX)
> -		sd->quality = QUALITY_MAX;
> -	else
> -		sd->quality = jcomp->quality;
> +	sd->ctrls[QUALITY].val = clamp_t(u8, jcomp->quality,
> +					 QUALITY_MIN, QUALITY_MAX);
>   	if (gspca_dev->streaming)
>   		jpeg_set_qual(sd->jpeg_hdr, sd->quality);
>   	return gspca_dev->usb_err;
> @@ -6915,7 +6930,7 @@ static int sd_get_jcomp(struct gspca_dev *gspca_dev,
>   	struct sd *sd = (struct sd *) gspca_dev;
> 
>   	memset(jcomp, 0, sizeof *jcomp);
> -	jcomp->quality = sd->quality;
> +	jcomp->quality = sd->ctrls[QUALITY].val;
>   	jcomp->jpeg_markers = V4L2_JPEG_MARKER_DHT
>   			| V4L2_JPEG_MARKER_DQT;
>   	return 0;

Re: [PATCH/RFC v4 3/3] gspca: zc3xx: Add V4L2_CID_JPEG_COMPRESSION_QUALITY control support

[Jean-Francois Moine]

On Sat, 21 Jan 2012 15:45:42 +0100
Sylwester Nawrocki <sylvester.nawrocki@gmail.com> wrote:

> is this patch looking OK or you, or would you have any further remarks ?
> I'd like to add it to a pull request in coming week, together with remaining
> patches in this series plus a patch for some SoC JPEG codec driver.
> Please let me know if there is anything that could be changed/improved.

Hi Sylwester,

The patch is OK for me.

Acked-by: Jean-Francois Moine <moinejf@free.fr>

Thanks.

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/