diff mbox series

[v5,02/10] media: uapi: mpeg2: rework quantisation matrices semantics

Message ID 20210403180756.175881-3-ezequiel@collabora.com (mailing list archive)
State New, archived
Headers show
Series MPEG-2 stateless API cleanup and destaging | expand

Commit Message

Ezequiel Garcia April 3, 2021, 6:07 p.m. UTC
As stated in the MPEG-2 specification, section 6.3.7 "Quant matrix
extension":

  Each quantisation matrix has a default set of values. When a
  sequence_header_code is decoded all matrices shall be reset to
  their default values. User defined matrices may be downloaded
  and this can occur in a sequence_header() or in a
  quant_matrix_extension().

The load_intra_quantiser_matrix syntax elements are transmitted
in the bistream headers, signalling that a quantisation matrix
needs to be loaded and used for pictures transmitted afterwards
(until the matrices are reset).

This "load" semantics are implemented in the V4L2 interface
without the need of any "load" flags: passing the control
is effectively a load.

Therefore, rework the V4L2_CID_MPEG_VIDEO_MPEG2_QUANTISATION
semantics to match the MPEG-2 semantics. Quantisation matrices
values are now initialized by the V4L2 control core to their
reset default value, and applications are expected to reset
their values as specified.

The quantisation control is therefore optional, and used to
load bitstream-defined values in the quantisation matrices.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
Co-developed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
---
 .../media/v4l/ext-ctrls-codec.rst             | 17 ---------
 drivers/media/v4l2-core/v4l2-ctrls.c          | 21 ++++++++++
 .../staging/media/sunxi/cedrus/cedrus_mpeg2.c | 38 +------------------
 include/media/mpeg2-ctrls.h                   |  5 ---
 4 files changed, 23 insertions(+), 58 deletions(-)

Comments

Hans Verkuil April 6, 2021, 10:25 a.m. UTC | #1
On 03/04/2021 20:07, Ezequiel Garcia wrote:
> As stated in the MPEG-2 specification, section 6.3.7 "Quant matrix
> extension":
> 
>   Each quantisation matrix has a default set of values. When a
>   sequence_header_code is decoded all matrices shall be reset to
>   their default values. User defined matrices may be downloaded
>   and this can occur in a sequence_header() or in a
>   quant_matrix_extension().
> 
> The load_intra_quantiser_matrix syntax elements are transmitted
> in the bistream headers, signalling that a quantisation matrix

bistream -> bitstream

> needs to be loaded and used for pictures transmitted afterwards
> (until the matrices are reset).
> 
> This "load" semantics are implemented in the V4L2 interface
> without the need of any "load" flags: passing the control
> is effectively a load.
> 
> Therefore, rework the V4L2_CID_MPEG_VIDEO_MPEG2_QUANTISATION
> semantics to match the MPEG-2 semantics. Quantisation matrices
> values are now initialized by the V4L2 control core to their
> reset default value, and applications are expected to reset
> their values as specified.
> 
> The quantisation control is therefore optional, and used to
> load bitstream-defined values in the quantisation matrices.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> Co-developed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> ---
>  .../media/v4l/ext-ctrls-codec.rst             | 17 ---------
>  drivers/media/v4l2-core/v4l2-ctrls.c          | 21 ++++++++++
>  .../staging/media/sunxi/cedrus/cedrus_mpeg2.c | 38 +------------------
>  include/media/mpeg2-ctrls.h                   |  5 ---
>  4 files changed, 23 insertions(+), 58 deletions(-)
> 
> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> index 151d1c676b6e..d9546f0aa2e8 100644
> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> @@ -1746,23 +1746,6 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
>      :stub-columns: 0
>      :widths:       1 1 2
>  
> -    * - __u8
> -      - ``load_intra_quantiser_matrix``
> -      - One bit to indicate whether to load the ``intra_quantiser_matrix`` data.
> -    * - __u8
> -      - ``load_non_intra_quantiser_matrix``
> -      - One bit to indicate whether to load the ``non_intra_quantiser_matrix``
> -	data.
> -    * - __u8
> -      - ``load_chroma_intra_quantiser_matrix``
> -      - One bit to indicate whether to load the
> -	``chroma_intra_quantiser_matrix`` data, only relevant for non-4:2:0 YUV
> -	formats.
> -    * - __u8
> -      - ``load_chroma_non_intra_quantiser_matrix``
> -      - One bit to indicate whether to load the
> -	``chroma_non_intra_quantiser_matrix`` data, only relevant for non-4:2:0
> -	YUV formats.
>      * - __u8
>        - ``intra_quantiser_matrix[64]``
>        - The quantisation matrix coefficients for intra-coded frames, in zigzag
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> index 74f5ca1a5f6c..5d92a2b33a6e 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -57,6 +57,18 @@ static bool is_new_manual(const struct v4l2_ctrl *master)
>  	return master->is_auto && master->val == master->manual_mode_value;
>  }
>  
> +/* Default MPEG-2 quantisation coefficients, from the specification. */

Default -> Default intra

> +static const u8 mpeg2_intra_quant_matrix[64] = {
> +	8,  16, 16, 19, 16, 19, 22, 22,
> +	22, 22, 22, 22, 26, 24, 26, 27,
> +	27, 27, 26, 26, 26, 26, 27, 27,
> +	27, 29, 29, 29, 34, 34, 34, 29,
> +	29, 29, 27, 27, 29, 29, 32, 32,
> +	34, 34, 37, 38, 37, 35, 35, 34,
> +	35, 38, 38, 40, 40, 40, 48, 48,
> +	46, 46, 56, 56, 58, 69, 69, 83
> +};
> +
>  /* Returns NULL or a character pointer array containing the menu for
>     the given control ID. The pointer array ends with a NULL pointer.
>     An empty string signifies a menu entry that is invalid. This allows
> @@ -1656,6 +1668,7 @@ static void std_init_compound(const struct v4l2_ctrl *ctrl, u32 idx,
>  			      union v4l2_ctrl_ptr ptr)
>  {
>  	struct v4l2_ctrl_mpeg2_slice_params *p_mpeg2_slice_params;
> +	struct v4l2_ctrl_mpeg2_quantisation *p_mpeg2_quant;
>  	struct v4l2_ctrl_vp8_frame *p_vp8_frame;
>  	struct v4l2_ctrl_fwht_params *p_fwht_params;
>  	void *p = ptr.p + idx * ctrl->elem_size;
> @@ -1680,6 +1693,14 @@ static void std_init_compound(const struct v4l2_ctrl *ctrl, u32 idx,
>  		p_mpeg2_slice_params->picture.picture_coding_type =
>  					V4L2_MPEG2_PICTURE_CODING_TYPE_I;
>  		break;
> +	case V4L2_CTRL_TYPE_MPEG2_QUANTISATION:
> +		p_mpeg2_quant = p;
> +
> +		memcpy(p_mpeg2_quant->intra_quantiser_matrix,
> +		       mpeg2_intra_quant_matrix,
> +		       ARRAY_SIZE(mpeg2_intra_quant_matrix));
> +		memset(p_mpeg2_quant->non_intra_quantiser_matrix, 16, 64);

64 -> sizeof(p_mpeg2_quant->non_intra_quantiser_matrix)

Also add a comment before the memset:

	/*
	 * The default non-intra MPEG-2 quantisation coefficients are all 16,
	 * as per the specification.
	 */

> +		break;
>  	case V4L2_CTRL_TYPE_VP8_FRAME:
>  		p_vp8_frame = p;
>  		p_vp8_frame->num_dct_parts = 1;
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
> index 459f71679a4f..e3154f631858 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
> @@ -13,30 +13,6 @@
>  #include "cedrus_hw.h"
>  #include "cedrus_regs.h"
>  
> -/* Default MPEG-2 quantisation coefficients, from the specification. */
> -
> -static const u8 intra_quantisation_matrix_default[64] = {
> -	8,  16, 16, 19, 16, 19, 22, 22,
> -	22, 22, 22, 22, 26, 24, 26, 27,
> -	27, 27, 26, 26, 26, 26, 27, 27,
> -	27, 29, 29, 29, 34, 34, 34, 29,
> -	29, 29, 27, 27, 29, 29, 32, 32,
> -	34, 34, 37, 38, 37, 35, 35, 34,
> -	35, 38, 38, 40, 40, 40, 48, 48,
> -	46, 46, 56, 56, 58, 69, 69, 83
> -};
> -
> -static const u8 non_intra_quantisation_matrix_default[64] = {
> -	16, 16, 16, 16, 16, 16, 16, 16,
> -	16, 16, 16, 16, 16, 16, 16, 16,
> -	16, 16, 16, 16, 16, 16, 16, 16,
> -	16, 16, 16, 16, 16, 16, 16, 16,
> -	16, 16, 16, 16, 16, 16, 16, 16,
> -	16, 16, 16, 16, 16, 16, 16, 16,
> -	16, 16, 16, 16, 16, 16, 16, 16,
> -	16, 16, 16, 16, 16, 16, 16, 16
> -};
> -
>  static enum cedrus_irq_status cedrus_mpeg2_irq_status(struct cedrus_ctx *ctx)
>  {
>  	struct cedrus_dev *dev = ctx->dev;
> @@ -99,12 +75,7 @@ static void cedrus_mpeg2_setup(struct cedrus_ctx *ctx, struct cedrus_run *run)
>  	cedrus_engine_enable(ctx, CEDRUS_CODEC_MPEG2);
>  
>  	/* Set intra quantisation matrix. */
> -
> -	if (quantisation && quantisation->load_intra_quantiser_matrix)
> -		matrix = quantisation->intra_quantiser_matrix;
> -	else
> -		matrix = intra_quantisation_matrix_default;
> -
> +	matrix = quantisation->intra_quantiser_matrix;
>  	for (i = 0; i < 64; i++) {
>  		reg = VE_DEC_MPEG_IQMINPUT_WEIGHT(i, matrix[i]);
>  		reg |= VE_DEC_MPEG_IQMINPUT_FLAG_INTRA;
> @@ -113,12 +84,7 @@ static void cedrus_mpeg2_setup(struct cedrus_ctx *ctx, struct cedrus_run *run)
>  	}
>  
>  	/* Set non-intra quantisation matrix. */
> -
> -	if (quantisation && quantisation->load_non_intra_quantiser_matrix)
> -		matrix = quantisation->non_intra_quantiser_matrix;
> -	else
> -		matrix = non_intra_quantisation_matrix_default;
> -
> +	matrix = quantisation->non_intra_quantiser_matrix;
>  	for (i = 0; i < 64; i++) {
>  		reg = VE_DEC_MPEG_IQMINPUT_WEIGHT(i, matrix[i]);
>  		reg |= VE_DEC_MPEG_IQMINPUT_FLAG_NON_INTRA;
> diff --git a/include/media/mpeg2-ctrls.h b/include/media/mpeg2-ctrls.h
> index b8adf3ac2c1d..8ea2c7f3a172 100644
> --- a/include/media/mpeg2-ctrls.h
> +++ b/include/media/mpeg2-ctrls.h
> @@ -68,11 +68,6 @@ struct v4l2_ctrl_mpeg2_slice_params {
>  
>  struct v4l2_ctrl_mpeg2_quantisation {
>  	/* ISO/IEC 13818-2, ITU-T Rec. H.262: Quant matrix extension */
> -	__u8	load_intra_quantiser_matrix;
> -	__u8	load_non_intra_quantiser_matrix;
> -	__u8	load_chroma_intra_quantiser_matrix;
> -	__u8	load_chroma_non_intra_quantiser_matrix;
> -
>  	__u8	intra_quantiser_matrix[64];
>  	__u8	non_intra_quantiser_matrix[64];
>  	__u8	chroma_intra_quantiser_matrix[64];
> 

Regards,

	Hans
diff mbox series

Patch

diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
index 151d1c676b6e..d9546f0aa2e8 100644
--- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
+++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
@@ -1746,23 +1746,6 @@  enum v4l2_mpeg_video_h264_hierarchical_coding_type -
     :stub-columns: 0
     :widths:       1 1 2
 
-    * - __u8
-      - ``load_intra_quantiser_matrix``
-      - One bit to indicate whether to load the ``intra_quantiser_matrix`` data.
-    * - __u8
-      - ``load_non_intra_quantiser_matrix``
-      - One bit to indicate whether to load the ``non_intra_quantiser_matrix``
-	data.
-    * - __u8
-      - ``load_chroma_intra_quantiser_matrix``
-      - One bit to indicate whether to load the
-	``chroma_intra_quantiser_matrix`` data, only relevant for non-4:2:0 YUV
-	formats.
-    * - __u8
-      - ``load_chroma_non_intra_quantiser_matrix``
-      - One bit to indicate whether to load the
-	``chroma_non_intra_quantiser_matrix`` data, only relevant for non-4:2:0
-	YUV formats.
     * - __u8
       - ``intra_quantiser_matrix[64]``
       - The quantisation matrix coefficients for intra-coded frames, in zigzag
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index 74f5ca1a5f6c..5d92a2b33a6e 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -57,6 +57,18 @@  static bool is_new_manual(const struct v4l2_ctrl *master)
 	return master->is_auto && master->val == master->manual_mode_value;
 }
 
+/* Default MPEG-2 quantisation coefficients, from the specification. */
+static const u8 mpeg2_intra_quant_matrix[64] = {
+	8,  16, 16, 19, 16, 19, 22, 22,
+	22, 22, 22, 22, 26, 24, 26, 27,
+	27, 27, 26, 26, 26, 26, 27, 27,
+	27, 29, 29, 29, 34, 34, 34, 29,
+	29, 29, 27, 27, 29, 29, 32, 32,
+	34, 34, 37, 38, 37, 35, 35, 34,
+	35, 38, 38, 40, 40, 40, 48, 48,
+	46, 46, 56, 56, 58, 69, 69, 83
+};
+
 /* Returns NULL or a character pointer array containing the menu for
    the given control ID. The pointer array ends with a NULL pointer.
    An empty string signifies a menu entry that is invalid. This allows
@@ -1656,6 +1668,7 @@  static void std_init_compound(const struct v4l2_ctrl *ctrl, u32 idx,
 			      union v4l2_ctrl_ptr ptr)
 {
 	struct v4l2_ctrl_mpeg2_slice_params *p_mpeg2_slice_params;
+	struct v4l2_ctrl_mpeg2_quantisation *p_mpeg2_quant;
 	struct v4l2_ctrl_vp8_frame *p_vp8_frame;
 	struct v4l2_ctrl_fwht_params *p_fwht_params;
 	void *p = ptr.p + idx * ctrl->elem_size;
@@ -1680,6 +1693,14 @@  static void std_init_compound(const struct v4l2_ctrl *ctrl, u32 idx,
 		p_mpeg2_slice_params->picture.picture_coding_type =
 					V4L2_MPEG2_PICTURE_CODING_TYPE_I;
 		break;
+	case V4L2_CTRL_TYPE_MPEG2_QUANTISATION:
+		p_mpeg2_quant = p;
+
+		memcpy(p_mpeg2_quant->intra_quantiser_matrix,
+		       mpeg2_intra_quant_matrix,
+		       ARRAY_SIZE(mpeg2_intra_quant_matrix));
+		memset(p_mpeg2_quant->non_intra_quantiser_matrix, 16, 64);
+		break;
 	case V4L2_CTRL_TYPE_VP8_FRAME:
 		p_vp8_frame = p;
 		p_vp8_frame->num_dct_parts = 1;
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
index 459f71679a4f..e3154f631858 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
@@ -13,30 +13,6 @@ 
 #include "cedrus_hw.h"
 #include "cedrus_regs.h"
 
-/* Default MPEG-2 quantisation coefficients, from the specification. */
-
-static const u8 intra_quantisation_matrix_default[64] = {
-	8,  16, 16, 19, 16, 19, 22, 22,
-	22, 22, 22, 22, 26, 24, 26, 27,
-	27, 27, 26, 26, 26, 26, 27, 27,
-	27, 29, 29, 29, 34, 34, 34, 29,
-	29, 29, 27, 27, 29, 29, 32, 32,
-	34, 34, 37, 38, 37, 35, 35, 34,
-	35, 38, 38, 40, 40, 40, 48, 48,
-	46, 46, 56, 56, 58, 69, 69, 83
-};
-
-static const u8 non_intra_quantisation_matrix_default[64] = {
-	16, 16, 16, 16, 16, 16, 16, 16,
-	16, 16, 16, 16, 16, 16, 16, 16,
-	16, 16, 16, 16, 16, 16, 16, 16,
-	16, 16, 16, 16, 16, 16, 16, 16,
-	16, 16, 16, 16, 16, 16, 16, 16,
-	16, 16, 16, 16, 16, 16, 16, 16,
-	16, 16, 16, 16, 16, 16, 16, 16,
-	16, 16, 16, 16, 16, 16, 16, 16
-};
-
 static enum cedrus_irq_status cedrus_mpeg2_irq_status(struct cedrus_ctx *ctx)
 {
 	struct cedrus_dev *dev = ctx->dev;
@@ -99,12 +75,7 @@  static void cedrus_mpeg2_setup(struct cedrus_ctx *ctx, struct cedrus_run *run)
 	cedrus_engine_enable(ctx, CEDRUS_CODEC_MPEG2);
 
 	/* Set intra quantisation matrix. */
-
-	if (quantisation && quantisation->load_intra_quantiser_matrix)
-		matrix = quantisation->intra_quantiser_matrix;
-	else
-		matrix = intra_quantisation_matrix_default;
-
+	matrix = quantisation->intra_quantiser_matrix;
 	for (i = 0; i < 64; i++) {
 		reg = VE_DEC_MPEG_IQMINPUT_WEIGHT(i, matrix[i]);
 		reg |= VE_DEC_MPEG_IQMINPUT_FLAG_INTRA;
@@ -113,12 +84,7 @@  static void cedrus_mpeg2_setup(struct cedrus_ctx *ctx, struct cedrus_run *run)
 	}
 
 	/* Set non-intra quantisation matrix. */
-
-	if (quantisation && quantisation->load_non_intra_quantiser_matrix)
-		matrix = quantisation->non_intra_quantiser_matrix;
-	else
-		matrix = non_intra_quantisation_matrix_default;
-
+	matrix = quantisation->non_intra_quantiser_matrix;
 	for (i = 0; i < 64; i++) {
 		reg = VE_DEC_MPEG_IQMINPUT_WEIGHT(i, matrix[i]);
 		reg |= VE_DEC_MPEG_IQMINPUT_FLAG_NON_INTRA;
diff --git a/include/media/mpeg2-ctrls.h b/include/media/mpeg2-ctrls.h
index b8adf3ac2c1d..8ea2c7f3a172 100644
--- a/include/media/mpeg2-ctrls.h
+++ b/include/media/mpeg2-ctrls.h
@@ -68,11 +68,6 @@  struct v4l2_ctrl_mpeg2_slice_params {
 
 struct v4l2_ctrl_mpeg2_quantisation {
 	/* ISO/IEC 13818-2, ITU-T Rec. H.262: Quant matrix extension */
-	__u8	load_intra_quantiser_matrix;
-	__u8	load_non_intra_quantiser_matrix;
-	__u8	load_chroma_intra_quantiser_matrix;
-	__u8	load_chroma_non_intra_quantiser_matrix;
-
 	__u8	intra_quantiser_matrix[64];
 	__u8	non_intra_quantiser_matrix[64];
 	__u8	chroma_intra_quantiser_matrix[64];