Re: [PATCH v3 6/7] media: Add controls for JPEG quantization tables

[Paul Kocialkowski <paul.kocialkowski-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org>]

[-- Attachment #1.1: Type: text/plain, Size: 4448 bytes --]

Hi,

On Wed, 2018-08-22 at 13:59 -0300, Ezequiel Garcia wrote:
> From: Shunqian Zheng <zhengsq-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> 
> 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 <zhengsq-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> Signed-off-by: Ezequiel Garcia <ezequiel-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
> ---
>  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 #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 200 bytes --]

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip