Re: [PATCH v3 6/7] media: Add controls for JPEG quantization tables
On Wed, Aug 29, 2018 at 11:50 AM Ezequiel Garcia wrote:
>
> On Mon, 2018-08-27 at 09:47 +0200, Paul Kocialkowski wrote:
> > Hi,
> >
> > On Wed, 2018-08-22 at 13:59 -0300, Ezequiel Garcia wrote:
> > > From: Shunqian Zheng
> > >
> > > 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?
> >
>
> I think it makes a lot of sense. I don't see the benefit in having luma
> and chroma separated, and consistency is good.
>
> I guess the more consistent solution would be to expose a compound
> control, similar to the video quantization one.
>
> struct v4l2_ctrl_jpeg_quantization {
>__u8luma_quantization_matrix[64];
>__u8chroma_quantization_matrix[64];
> };
Makes sense indeed. It also lets us avoid the hassle of setting
.min/.max/.dims and other array control stuff, since everything is
already defined by the C struct itself.
Best regards,
Tomasz
Re: [PATCH v3 6/7] media: Add controls for JPEG quantization tables
On Mon, 2018-08-27 at 09:47 +0200, Paul Kocialkowski wrote:
> Hi,
>
> On Wed, 2018-08-22 at 13:59 -0300, Ezequiel Garcia wrote:
> > From: Shunqian Zheng
> >
> > 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?
>
I think it makes a lot of sense. I don't see the benefit in having luma
and chroma separated, and consistency is good.
I guess the more consistent solution would be to expose a compound
control, similar to the video quantization one.
struct v4l2_ctrl_jpeg_quantization {
__u8luma_quantization_matrix[64];
__u8chroma_quantization_matrix[64];
};
Thanks!
Eze
Re: [PATCH v3 6/7] media: Add controls for JPEG quantization tables
Hi,
On Wed, 2018-08-22 at 13:59 -0300, Ezequiel Garcia wrote:
> From: Shunqian Zheng
>
> 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
> Signed-off-by: Ezequiel Garcia
> ---
> 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
signature.asc
Description: This is a digitally signed message part
[PATCH v3 6/7] media: Add controls for JPEG quantization tables
From: Shunqian Zheng
Add V4L2_CID_JPEG_LUMA/CHROMA_QUANTIZATION controls to allow userspace
configure the JPEG quantization tables.
Signed-off-by: Shunqian Zheng
Signed-off-by: Ezequiel Garcia
---
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 {
#defineV4L2_JPEG_ACTIVE_MARKER_DQT (1 << 17)
#defineV4L2_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)
--
2.18.0
