Hi,

On Wed, 2018-08-22 at 13:59 -0300, Ezequiel Garcia wrote:
> From: Shunqian Zheng <zhen...@rock-chips.com>
> 
> Add V4L2_CID_JPEG_LUMA/CHROMA_QUANTIZATION controls to allow userspace
> configure the JPEG quantization tables.

How about having a single control for quantization?

In MPEG-2/H.264/H.265, we have a single control exposed as a structure,
which contains the tables for both luma and chroma. In the case of JPEG,
it's not that big a deal, but for advanced video formats, it would be
too much hassle to have one control per table.

In order to keep the interface consistent, I think it'd be best to merge
both matrices into a single control.

What do you think?

Paul

> Signed-off-by: Shunqian Zheng <zhen...@rock-chips.com>
> Signed-off-by: Ezequiel Garcia <ezequ...@collabora.com>
> ---
>  Documentation/media/uapi/v4l/extended-controls.rst | 13 +++++++++++++
>  drivers/media/v4l2-core/v4l2-ctrls.c               | 13 +++++++++++++
>  include/uapi/linux/v4l2-controls.h                 |  3 +++
>  3 files changed, 29 insertions(+)
> 
> diff --git a/Documentation/media/uapi/v4l/extended-controls.rst 
> b/Documentation/media/uapi/v4l/extended-controls.rst
> index 9f7312bf3365..1189750a022c 100644
> --- a/Documentation/media/uapi/v4l/extended-controls.rst
> +++ b/Documentation/media/uapi/v4l/extended-controls.rst
> @@ -3354,6 +3354,19 @@ JPEG Control IDs
>      Specify which JPEG markers are included in compressed stream. This
>      control is valid only for encoders.
>  
> +.. _jpeg-quant-tables-control:
> +
> +``V4L2_CID_JPEG_LUMA_QUANTIZATION (__u8 matrix)``
> +    Sets the luma quantization table to be used for encoding
> +    or decoding a V4L2_PIX_FMT_JPEG_RAW format buffer. This table is
> +    a 8x8-byte matrix, and it's expected to be in JPEG zigzag order,
> +    as per the JPEG specification.
> +
> +``V4L2_CID_JPEG_CHROMA_QUANTIZATION (__u8 matrix)``
> +    Sets the chroma quantization table to be used for encoding
> +    or decoding a V4L2_PIX_FMT_JPEG_RAW format buffer. This table is
> +    a 8x8-byte matrix, and it's expected to be in JPEG zigzag order,
> +    as per the JPEG specification.
>  
>  
>  .. flat-table::
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
> b/drivers/media/v4l2-core/v4l2-ctrls.c
> index 6ab15f4a0fea..677af8069bfc 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -999,6 +999,8 @@ const char *v4l2_ctrl_get_name(u32 id)
>       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";
> +     case V4L2_CID_JPEG_LUMA_QUANTIZATION:   return "Luminance Quantization 
> Matrix";
> +     case V4L2_CID_JPEG_CHROMA_QUANTIZATION: return "Chrominance 
> Quantization Matrix";
>  
>       /* Image source controls */
>       /* Keep the order of the 'case's the same as in v4l2-controls.h! */
> @@ -1286,6 +1288,17 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum 
> v4l2_ctrl_type *type,
>       case V4L2_CID_DETECT_MD_REGION_GRID:
>               *type = V4L2_CTRL_TYPE_U8;
>               break;
> +     case V4L2_CID_JPEG_LUMA_QUANTIZATION:
> +     case V4L2_CID_JPEG_CHROMA_QUANTIZATION:
> +             *min = *def = 0;
> +             *max = 0xff;
> +             *step = 1;
> +             *type = V4L2_CTRL_TYPE_U8;
> +             if (dims) {
> +                     memset(dims, 0, V4L2_CTRL_MAX_DIMS * sizeof(dims[0]));
> +                     dims[0] = dims[1] = 8;
> +             }
> +             break;
>       case V4L2_CID_DETECT_MD_THRESHOLD_GRID:
>               *type = V4L2_CTRL_TYPE_U16;
>               break;
> diff --git a/include/uapi/linux/v4l2-controls.h 
> b/include/uapi/linux/v4l2-controls.h
> index e4ee10ee917d..a466acf40625 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -987,6 +987,9 @@ enum v4l2_jpeg_chroma_subsampling {
>  #define      V4L2_JPEG_ACTIVE_MARKER_DQT             (1 << 17)
>  #define      V4L2_JPEG_ACTIVE_MARKER_DHT             (1 << 18)
>  
> +#define V4L2_CID_JPEG_LUMA_QUANTIZATION              
> (V4L2_CID_JPEG_CLASS_BASE + 5)
> +#define V4L2_CID_JPEG_CHROMA_QUANTIZATION    (V4L2_CID_JPEG_CLASS_BASE + 6)
> +
>  
>  /* Image source controls */
>  #define V4L2_CID_IMAGE_SOURCE_CLASS_BASE     (V4L2_CTRL_CLASS_IMAGE_SOURCE | 
> 0x900)
-- 
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com

Attachment: signature.asc
Description: This is a digitally signed message part

Reply via email to