diff mbox series

[v2,7/8] media: hevc: Add scaling matrix control

Message ID 20210610154442.806107-8-benjamin.gaignard@collabora.com
State Superseded
Headers show
Series Additional features for Hantro HEVC | expand

Commit Message

Benjamin Gaignard June 10, 2021, 3:44 p.m. UTC
HEVC scaling lists are used for the scaling process for transform
coefficients.
V4L2_HEVC_SPS_FLAG_SCALING_LIST_ENABLED has to set when they are
encoded in the bitstream.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
---
version 2:
 - Fix structure name in ext-ctrls-codec.rst

 .../media/v4l/ext-ctrls-codec.rst             | 45 +++++++++++++++++++
 .../media/v4l/vidioc-queryctrl.rst            |  6 +++
 drivers/media/v4l2-core/v4l2-ctrls-core.c     |  6 +++
 drivers/media/v4l2-core/v4l2-ctrls-defs.c     |  4 ++
 include/media/hevc-ctrls.h                    | 11 +++++
 5 files changed, 72 insertions(+)

Comments

Hans Verkuil June 14, 2021, 7:27 a.m. UTC | #1
On 10/06/2021 17:44, Benjamin Gaignard wrote:
> HEVC scaling lists are used for the scaling process for transform

> coefficients.

> V4L2_HEVC_SPS_FLAG_SCALING_LIST_ENABLED has to set when they are

> encoded in the bitstream.


Comparing H264 with HEVC I noticed that the corresponding flag for H264 is
called V4L2_H264_PPS_FLAG_SCALING_MATRIX_PRESENT.

Should those names be aligned? Also, it is part of PPS for H264 and SPS in HEVC,
is that difference correct?

Regards,

	Hans

> 

> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>

> ---

> version 2:

>  - Fix structure name in ext-ctrls-codec.rst

> 

>  .../media/v4l/ext-ctrls-codec.rst             | 45 +++++++++++++++++++

>  .../media/v4l/vidioc-queryctrl.rst            |  6 +++

>  drivers/media/v4l2-core/v4l2-ctrls-core.c     |  6 +++

>  drivers/media/v4l2-core/v4l2-ctrls-defs.c     |  4 ++

>  include/media/hevc-ctrls.h                    | 11 +++++

>  5 files changed, 72 insertions(+)

> 

> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst

> index 8c6e2a11ed95..d4f40bb85263 100644

> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst

> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst

> @@ -3068,6 +3068,51 @@ enum v4l2_mpeg_video_hevc_size_of_length_field -

>  

>      \normalsize

>  

> +``V4L2_CID_MPEG_VIDEO_HEVC_SCALING_MATRIX (struct)``

> +    Specifies the HEVC scaling matrix parameters used for the scaling process

> +    for transform coefficients.

> +    These matrix and parameters are defined according to :ref:`hevc`.

> +    They are described in section 7.4.5 "Scaling list data semantics" of

> +    the specification.

> +

> +.. c:type:: v4l2_ctrl_hevc_scaling_matrix

> +

> +.. raw:: latex

> +

> +    \scriptsize

> +

> +.. tabularcolumns:: |p{5.4cm}|p{6.8cm}|p{5.1cm}|

> +

> +.. cssclass:: longtable

> +

> +.. flat-table:: struct v4l2_ctrl_hevc_scaling_matrix

> +    :header-rows:  0

> +    :stub-columns: 0

> +    :widths:       1 1 2

> +

> +    * - __u8

> +      - ``scaling_list_4x4[6][16]``

> +      -

> +    * - __u8

> +      - ``scaling_list_8x8[6][64]``

> +      -

> +    * - __u8

> +      - ``scaling_list_16x16[6][64]``

> +      -

> +    * - __u8

> +      - ``scaling_list_32x32[2][64]``

> +      -

> +    * - __u8

> +      - ``scaling_list_dc_coef_16x16[6]``

> +      -

> +    * - __u8

> +      - ``scaling_list_dc_coef_32x32[2]``

> +      -

> +

> +.. raw:: latex

> +

> +    \normalsize

> +

>  .. c:type:: v4l2_hevc_dpb_entry

>  

>  .. raw:: latex

> diff --git a/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst b/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst

> index f9ecf6276129..2f491c17dd5d 100644

> --- a/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst

> +++ b/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst

> @@ -495,6 +495,12 @@ See also the examples in :ref:`control`.

>        - n/a

>        - A struct :c:type:`v4l2_ctrl_hevc_slice_params`, containing HEVC

>  	slice parameters for stateless video decoders.

> +    * - ``V4L2_CTRL_TYPE_HEVC_SCALING_MATRIX``

> +      - n/a

> +      - n/a

> +      - n/a

> +      - A struct :c:type:`v4l2_ctrl_hevc_scaling_matrix`, containing HEVC

> +	scaling matrix for stateless video decoders.

>      * - ``V4L2_CTRL_TYPE_VP8_FRAME``

>        - n/a

>        - n/a

> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-core.c b/drivers/media/v4l2-core/v4l2-ctrls-core.c

> index c4b5082849b6..70adfc1b9c81 100644

> --- a/drivers/media/v4l2-core/v4l2-ctrls-core.c

> +++ b/drivers/media/v4l2-core/v4l2-ctrls-core.c

> @@ -687,6 +687,9 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,

>  

>  		break;

>  

> +	case V4L2_CTRL_TYPE_HEVC_SCALING_MATRIX:

> +		break;

> +

>  	case V4L2_CTRL_TYPE_AREA:

>  		area = p;

>  		if (!area->width || !area->height)

> @@ -1240,6 +1243,9 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,

>  	case V4L2_CTRL_TYPE_HEVC_SLICE_PARAMS:

>  		elem_size = sizeof(struct v4l2_ctrl_hevc_slice_params);

>  		break;

> +	case V4L2_CTRL_TYPE_HEVC_SCALING_MATRIX:

> +		elem_size = sizeof(struct v4l2_ctrl_hevc_scaling_matrix);

> +		break;

>  	case V4L2_CTRL_TYPE_HEVC_DECODE_PARAMS:

>  		elem_size = sizeof(struct v4l2_ctrl_hevc_decode_params);

>  		break;

> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c

> index b6344bbf1e00..cb29c2a7fabe 100644

> --- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c

> +++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c

> @@ -996,6 +996,7 @@ const char *v4l2_ctrl_get_name(u32 id)

>  	case V4L2_CID_MPEG_VIDEO_HEVC_SPS:			return "HEVC Sequence Parameter Set";

>  	case V4L2_CID_MPEG_VIDEO_HEVC_PPS:			return "HEVC Picture Parameter Set";

>  	case V4L2_CID_MPEG_VIDEO_HEVC_SLICE_PARAMS:		return "HEVC Slice Parameters";

> +	case V4L2_CID_MPEG_VIDEO_HEVC_SCALING_MATRIX:		return "HEVC Scaling Matrix";

>  	case V4L2_CID_MPEG_VIDEO_HEVC_DECODE_PARAMS:		return "HEVC Decode Parameters";

>  	case V4L2_CID_MPEG_VIDEO_HEVC_DECODE_MODE:		return "HEVC Decode Mode";

>  	case V4L2_CID_MPEG_VIDEO_HEVC_START_CODE:		return "HEVC Start Code";

> @@ -1488,6 +1489,9 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,

>  	case V4L2_CID_MPEG_VIDEO_HEVC_SLICE_PARAMS:

>  		*type = V4L2_CTRL_TYPE_HEVC_SLICE_PARAMS;

>  		break;

> +	case V4L2_CID_MPEG_VIDEO_HEVC_SCALING_MATRIX:

> +		*type = V4L2_CTRL_TYPE_HEVC_SCALING_MATRIX;

> +		break;

>  	case V4L2_CID_MPEG_VIDEO_HEVC_DECODE_PARAMS:

>  		*type = V4L2_CTRL_TYPE_HEVC_DECODE_PARAMS;

>  		break;

> diff --git a/include/media/hevc-ctrls.h b/include/media/hevc-ctrls.h

> index 53c0038c792b..0e5c4a2eecff 100644

> --- a/include/media/hevc-ctrls.h

> +++ b/include/media/hevc-ctrls.h

> @@ -19,6 +19,7 @@

>  #define V4L2_CID_MPEG_VIDEO_HEVC_SPS		(V4L2_CID_CODEC_BASE + 1008)

>  #define V4L2_CID_MPEG_VIDEO_HEVC_PPS		(V4L2_CID_CODEC_BASE + 1009)

>  #define V4L2_CID_MPEG_VIDEO_HEVC_SLICE_PARAMS	(V4L2_CID_CODEC_BASE + 1010)

> +#define V4L2_CID_MPEG_VIDEO_HEVC_SCALING_MATRIX	(V4L2_CID_CODEC_BASE + 1011)

>  #define V4L2_CID_MPEG_VIDEO_HEVC_DECODE_PARAMS	(V4L2_CID_CODEC_BASE + 1012)

>  #define V4L2_CID_MPEG_VIDEO_HEVC_DECODE_MODE	(V4L2_CID_CODEC_BASE + 1015)

>  #define V4L2_CID_MPEG_VIDEO_HEVC_START_CODE	(V4L2_CID_CODEC_BASE + 1016)

> @@ -27,6 +28,7 @@

>  #define V4L2_CTRL_TYPE_HEVC_SPS 0x0120

>  #define V4L2_CTRL_TYPE_HEVC_PPS 0x0121

>  #define V4L2_CTRL_TYPE_HEVC_SLICE_PARAMS 0x0122

> +#define V4L2_CTRL_TYPE_HEVC_SCALING_MATRIX 0x0123

>  #define V4L2_CTRL_TYPE_HEVC_DECODE_PARAMS 0x0124

>  

>  enum v4l2_mpeg_video_hevc_decode_mode {

> @@ -224,6 +226,15 @@ struct v4l2_ctrl_hevc_decode_params {

>  	__u64	flags;

>  };

>  

> +struct v4l2_ctrl_hevc_scaling_matrix {

> +	__u8	scaling_list_4x4[6][16];

> +	__u8	scaling_list_8x8[6][64];

> +	__u8	scaling_list_16x16[6][64];

> +	__u8	scaling_list_32x32[2][64];

> +	__u8	scaling_list_dc_coef_16x16[6];

> +	__u8	scaling_list_dc_coef_32x32[2];

> +};

> +

>  /*  MPEG-class control IDs specific to the Hantro driver as defined by V4L2 */

>  #define V4L2_CID_CODEC_HANTRO_BASE				(V4L2_CTRL_CLASS_CODEC | 0x1200)

>  /*

>
Benjamin Gaignard June 14, 2021, 7:43 a.m. UTC | #2
Le 14/06/2021 à 09:27, Hans Verkuil a écrit :
> On 10/06/2021 17:44, Benjamin Gaignard wrote:

>> HEVC scaling lists are used for the scaling process for transform

>> coefficients.

>> V4L2_HEVC_SPS_FLAG_SCALING_LIST_ENABLED has to set when they are

>> encoded in the bitstream.

> Comparing H264 with HEVC I noticed that the corresponding flag for H264 is

> called V4L2_H264_PPS_FLAG_SCALING_MATRIX_PRESENT.

>

> Should those names be aligned? Also, it is part of PPS for H264 and SPS in HEVC,

> is that difference correct?


In ITU specifications ("7.4.3.2.1 General sequence parameter set RBSP semantics") this flag is define like that:
scaling_list_enabled_flag equal to 1 specifies that a scaling list is used for the scaling process for transform coefficients.
scaling_list_enabled_flag equal to 0 specifies that scaling list is not used for the scaling process for transform coefficients.

So for me the naming is correct.

Regards,
Benjamin

>

> Regards,

>

> 	Hans

>

>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>

>> ---

>> version 2:

>>   - Fix structure name in ext-ctrls-codec.rst

>>

>>   .../media/v4l/ext-ctrls-codec.rst             | 45 +++++++++++++++++++

>>   .../media/v4l/vidioc-queryctrl.rst            |  6 +++

>>   drivers/media/v4l2-core/v4l2-ctrls-core.c     |  6 +++

>>   drivers/media/v4l2-core/v4l2-ctrls-defs.c     |  4 ++

>>   include/media/hevc-ctrls.h                    | 11 +++++

>>   5 files changed, 72 insertions(+)

>>

>> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst

>> index 8c6e2a11ed95..d4f40bb85263 100644

>> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst

>> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst

>> @@ -3068,6 +3068,51 @@ enum v4l2_mpeg_video_hevc_size_of_length_field -

>>   

>>       \normalsize

>>   

>> +``V4L2_CID_MPEG_VIDEO_HEVC_SCALING_MATRIX (struct)``

>> +    Specifies the HEVC scaling matrix parameters used for the scaling process

>> +    for transform coefficients.

>> +    These matrix and parameters are defined according to :ref:`hevc`.

>> +    They are described in section 7.4.5 "Scaling list data semantics" of

>> +    the specification.

>> +

>> +.. c:type:: v4l2_ctrl_hevc_scaling_matrix

>> +

>> +.. raw:: latex

>> +

>> +    \scriptsize

>> +

>> +.. tabularcolumns:: |p{5.4cm}|p{6.8cm}|p{5.1cm}|

>> +

>> +.. cssclass:: longtable

>> +

>> +.. flat-table:: struct v4l2_ctrl_hevc_scaling_matrix

>> +    :header-rows:  0

>> +    :stub-columns: 0

>> +    :widths:       1 1 2

>> +

>> +    * - __u8

>> +      - ``scaling_list_4x4[6][16]``

>> +      -

>> +    * - __u8

>> +      - ``scaling_list_8x8[6][64]``

>> +      -

>> +    * - __u8

>> +      - ``scaling_list_16x16[6][64]``

>> +      -

>> +    * - __u8

>> +      - ``scaling_list_32x32[2][64]``

>> +      -

>> +    * - __u8

>> +      - ``scaling_list_dc_coef_16x16[6]``

>> +      -

>> +    * - __u8

>> +      - ``scaling_list_dc_coef_32x32[2]``

>> +      -

>> +

>> +.. raw:: latex

>> +

>> +    \normalsize

>> +

>>   .. c:type:: v4l2_hevc_dpb_entry

>>   

>>   .. raw:: latex

>> diff --git a/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst b/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst

>> index f9ecf6276129..2f491c17dd5d 100644

>> --- a/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst

>> +++ b/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst

>> @@ -495,6 +495,12 @@ See also the examples in :ref:`control`.

>>         - n/a

>>         - A struct :c:type:`v4l2_ctrl_hevc_slice_params`, containing HEVC

>>   	slice parameters for stateless video decoders.

>> +    * - ``V4L2_CTRL_TYPE_HEVC_SCALING_MATRIX``

>> +      - n/a

>> +      - n/a

>> +      - n/a

>> +      - A struct :c:type:`v4l2_ctrl_hevc_scaling_matrix`, containing HEVC

>> +	scaling matrix for stateless video decoders.

>>       * - ``V4L2_CTRL_TYPE_VP8_FRAME``

>>         - n/a

>>         - n/a

>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-core.c b/drivers/media/v4l2-core/v4l2-ctrls-core.c

>> index c4b5082849b6..70adfc1b9c81 100644

>> --- a/drivers/media/v4l2-core/v4l2-ctrls-core.c

>> +++ b/drivers/media/v4l2-core/v4l2-ctrls-core.c

>> @@ -687,6 +687,9 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,

>>   

>>   		break;

>>   

>> +	case V4L2_CTRL_TYPE_HEVC_SCALING_MATRIX:

>> +		break;

>> +

>>   	case V4L2_CTRL_TYPE_AREA:

>>   		area = p;

>>   		if (!area->width || !area->height)

>> @@ -1240,6 +1243,9 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,

>>   	case V4L2_CTRL_TYPE_HEVC_SLICE_PARAMS:

>>   		elem_size = sizeof(struct v4l2_ctrl_hevc_slice_params);

>>   		break;

>> +	case V4L2_CTRL_TYPE_HEVC_SCALING_MATRIX:

>> +		elem_size = sizeof(struct v4l2_ctrl_hevc_scaling_matrix);

>> +		break;

>>   	case V4L2_CTRL_TYPE_HEVC_DECODE_PARAMS:

>>   		elem_size = sizeof(struct v4l2_ctrl_hevc_decode_params);

>>   		break;

>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c

>> index b6344bbf1e00..cb29c2a7fabe 100644

>> --- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c

>> +++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c

>> @@ -996,6 +996,7 @@ const char *v4l2_ctrl_get_name(u32 id)

>>   	case V4L2_CID_MPEG_VIDEO_HEVC_SPS:			return "HEVC Sequence Parameter Set";

>>   	case V4L2_CID_MPEG_VIDEO_HEVC_PPS:			return "HEVC Picture Parameter Set";

>>   	case V4L2_CID_MPEG_VIDEO_HEVC_SLICE_PARAMS:		return "HEVC Slice Parameters";

>> +	case V4L2_CID_MPEG_VIDEO_HEVC_SCALING_MATRIX:		return "HEVC Scaling Matrix";

>>   	case V4L2_CID_MPEG_VIDEO_HEVC_DECODE_PARAMS:		return "HEVC Decode Parameters";

>>   	case V4L2_CID_MPEG_VIDEO_HEVC_DECODE_MODE:		return "HEVC Decode Mode";

>>   	case V4L2_CID_MPEG_VIDEO_HEVC_START_CODE:		return "HEVC Start Code";

>> @@ -1488,6 +1489,9 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,

>>   	case V4L2_CID_MPEG_VIDEO_HEVC_SLICE_PARAMS:

>>   		*type = V4L2_CTRL_TYPE_HEVC_SLICE_PARAMS;

>>   		break;

>> +	case V4L2_CID_MPEG_VIDEO_HEVC_SCALING_MATRIX:

>> +		*type = V4L2_CTRL_TYPE_HEVC_SCALING_MATRIX;

>> +		break;

>>   	case V4L2_CID_MPEG_VIDEO_HEVC_DECODE_PARAMS:

>>   		*type = V4L2_CTRL_TYPE_HEVC_DECODE_PARAMS;

>>   		break;

>> diff --git a/include/media/hevc-ctrls.h b/include/media/hevc-ctrls.h

>> index 53c0038c792b..0e5c4a2eecff 100644

>> --- a/include/media/hevc-ctrls.h

>> +++ b/include/media/hevc-ctrls.h

>> @@ -19,6 +19,7 @@

>>   #define V4L2_CID_MPEG_VIDEO_HEVC_SPS		(V4L2_CID_CODEC_BASE + 1008)

>>   #define V4L2_CID_MPEG_VIDEO_HEVC_PPS		(V4L2_CID_CODEC_BASE + 1009)

>>   #define V4L2_CID_MPEG_VIDEO_HEVC_SLICE_PARAMS	(V4L2_CID_CODEC_BASE + 1010)

>> +#define V4L2_CID_MPEG_VIDEO_HEVC_SCALING_MATRIX	(V4L2_CID_CODEC_BASE + 1011)

>>   #define V4L2_CID_MPEG_VIDEO_HEVC_DECODE_PARAMS	(V4L2_CID_CODEC_BASE + 1012)

>>   #define V4L2_CID_MPEG_VIDEO_HEVC_DECODE_MODE	(V4L2_CID_CODEC_BASE + 1015)

>>   #define V4L2_CID_MPEG_VIDEO_HEVC_START_CODE	(V4L2_CID_CODEC_BASE + 1016)

>> @@ -27,6 +28,7 @@

>>   #define V4L2_CTRL_TYPE_HEVC_SPS 0x0120

>>   #define V4L2_CTRL_TYPE_HEVC_PPS 0x0121

>>   #define V4L2_CTRL_TYPE_HEVC_SLICE_PARAMS 0x0122

>> +#define V4L2_CTRL_TYPE_HEVC_SCALING_MATRIX 0x0123

>>   #define V4L2_CTRL_TYPE_HEVC_DECODE_PARAMS 0x0124

>>   

>>   enum v4l2_mpeg_video_hevc_decode_mode {

>> @@ -224,6 +226,15 @@ struct v4l2_ctrl_hevc_decode_params {

>>   	__u64	flags;

>>   };

>>   

>> +struct v4l2_ctrl_hevc_scaling_matrix {

>> +	__u8	scaling_list_4x4[6][16];

>> +	__u8	scaling_list_8x8[6][64];

>> +	__u8	scaling_list_16x16[6][64];

>> +	__u8	scaling_list_32x32[2][64];

>> +	__u8	scaling_list_dc_coef_16x16[6];

>> +	__u8	scaling_list_dc_coef_32x32[2];

>> +};

>> +

>>   /*  MPEG-class control IDs specific to the Hantro driver as defined by V4L2 */

>>   #define V4L2_CID_CODEC_HANTRO_BASE				(V4L2_CTRL_CLASS_CODEC | 0x1200)

>>   /*

>>
Ezequiel Garcia June 14, 2021, 12:50 p.m. UTC | #3
On Mon, 2021-06-14 at 09:43 +0200, Benjamin Gaignard wrote:
> 

> Le 14/06/2021 à 09:27, Hans Verkuil a écrit :

> > On 10/06/2021 17:44, Benjamin Gaignard wrote:

> > > HEVC scaling lists are used for the scaling process for transform

> > > coefficients.

> > > V4L2_HEVC_SPS_FLAG_SCALING_LIST_ENABLED has to set when they are

> > > encoded in the bitstream.

> > Comparing H264 with HEVC I noticed that the corresponding flag for H264 is

> > called V4L2_H264_PPS_FLAG_SCALING_MATRIX_PRESENT.

> > 

> > Should those names be aligned? Also, it is part of PPS for H264 and SPS in HEVC,

> > is that difference correct?

> 

> In ITU specifications ("7.4.3.2.1 General sequence parameter set RBSP semantics") this flag is define like that:

> scaling_list_enabled_flag equal to 1 specifies that a scaling list is used for the scaling process for transform coefficients.

> scaling_list_enabled_flag equal to 0 specifies that scaling list is not used for the scaling process for transform coefficients.

> 

> So for me the naming is correct.

> 


The bitstream is really parsed in userspace (gstreamer, ffmpeg, chromium).
Not all bitstream parameters need to be passed as-is, because the kernel
is actually a sort of low-level layer in the decoder stack.

I think it's probably most appropriate to follow our H.264 interface
semantics, see https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=54889c51b833d236228f983be16212fbe806bb89.

The H.264 story goes like this:

* Default scaling list is used for decoding, but can be modified
  by a bitstream-provided scaling list.
   
* The scaling list modification can be in SPS or in PPS.

* For each frame, the SPS and PPS headers will tell if
  a modified scaling list must be used for decoding,
  and if it's provided in the SPS header or the PPS header.

The userspace parser must take care of this, and pass
a scaling matrix to the kernel via V4L2_CID_MPEG_VIDEO_H264_SCALING_MATRIX,
setting PPS_FLAG_SCALING_MATRIX_PRESENT.

This flag is at the PPS control, so the scaling modification
can be applied on each frame.

In other words, the kernel interface is simpler, it just
receives a scaling matrix and a flag telling if it must be used or not. 

We should probably clarify the documentation, so this process is more clear.

From the HEVC spec, it seems the process should be similar.
The only difference is the SPS "scaling_list_enabled_flag" parameter,
which doesn't exist in H.264.

Nicolas: what do you think?

Thanks,
Ezequiel
Nicolas Dufresne June 14, 2021, 1:33 p.m. UTC | #4
Le lundi 14 juin 2021 à 09:50 -0300, Ezequiel Garcia a écrit :
> On Mon, 2021-06-14 at 09:43 +0200, Benjamin Gaignard wrote:

> > 

> > Le 14/06/2021 à 09:27, Hans Verkuil a écrit :

> > > On 10/06/2021 17:44, Benjamin Gaignard wrote:

> > > > HEVC scaling lists are used for the scaling process for transform

> > > > coefficients.

> > > > V4L2_HEVC_SPS_FLAG_SCALING_LIST_ENABLED has to set when they are

> > > > encoded in the bitstream.

> > > Comparing H264 with HEVC I noticed that the corresponding flag for H264 is

> > > called V4L2_H264_PPS_FLAG_SCALING_MATRIX_PRESENT.

> > > 

> > > Should those names be aligned? Also, it is part of PPS for H264 and SPS in HEVC,

> > > is that difference correct?

> > 

> > In ITU specifications ("7.4.3.2.1 General sequence parameter set RBSP semantics") this flag is define like that:

> > scaling_list_enabled_flag equal to 1 specifies that a scaling list is used for the scaling process for transform coefficients.

> > scaling_list_enabled_flag equal to 0 specifies that scaling list is not used for the scaling process for transform coefficients.

> > 

> > So for me the naming is correct.

> > 

> 

> The bitstream is really parsed in userspace (gstreamer, ffmpeg, chromium).

> Not all bitstream parameters need to be passed as-is, because the kernel

> is actually a sort of low-level layer in the decoder stack.

> 

> I think it's probably most appropriate to follow our H.264 interface

> semantics, see https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=54889c51b833d236228f983be16212fbe806bb89.

> 

> The H.264 story goes like this:

> 

> * Default scaling list is used for decoding, but can be modified

>   by a bitstream-provided scaling list.

>    

> * The scaling list modification can be in SPS or in PPS.

> 

> * For each frame, the SPS and PPS headers will tell if

>   a modified scaling list must be used for decoding,

>   and if it's provided in the SPS header or the PPS header.

> 

> The userspace parser must take care of this, and pass

> a scaling matrix to the kernel via V4L2_CID_MPEG_VIDEO_H264_SCALING_MATRIX,

> setting PPS_FLAG_SCALING_MATRIX_PRESENT.

> 

> This flag is at the PPS control, so the scaling modification

> can be applied on each frame.

> 

> In other words, the kernel interface is simpler, it just

> receives a scaling matrix and a flag telling if it must be used or not. 

> 

> We should probably clarify the documentation, so this process is more clear.

> 

> From the HEVC spec, it seems the process should be similar.

> The only difference is the SPS "scaling_list_enabled_flag" parameter,

> which doesn't exist in H.264.

> 

> Nicolas: what do you think?


I believe its a fine "driver convenience". In the sense that the flag might not
have been strictly needed, but may make the driver code simpler. Whenever
possible, and within the spec, I'd agree to keep things as consistant as
possible.

From the doc, there seems to be a "default" or "flat" version of the scaling
matrix in H264. I think if I had notice this before, I would have pushed forward
the same semantic as the MPEG2 quantisation. In MPEG2, the quantisation control
is set to it's default value in the control framework. What I like of this
approach is that the control becomes always valid. Perhaps there is some
differences here and there I'm not noticing though.

Semantically, it would also be totally different if there is a HW fast path to
the "flat" scaling matrix, in which case you need that flag to enable it. I
think the fact one prepend SPS/PPS is simply to help locate the relevant part of
the specification.

> 

> Thanks,

> Ezequiel

>
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 8c6e2a11ed95..d4f40bb85263 100644
--- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
+++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
@@ -3068,6 +3068,51 @@  enum v4l2_mpeg_video_hevc_size_of_length_field -
 
     \normalsize
 
+``V4L2_CID_MPEG_VIDEO_HEVC_SCALING_MATRIX (struct)``
+    Specifies the HEVC scaling matrix parameters used for the scaling process
+    for transform coefficients.
+    These matrix and parameters are defined according to :ref:`hevc`.
+    They are described in section 7.4.5 "Scaling list data semantics" of
+    the specification.
+
+.. c:type:: v4l2_ctrl_hevc_scaling_matrix
+
+.. raw:: latex
+
+    \scriptsize
+
+.. tabularcolumns:: |p{5.4cm}|p{6.8cm}|p{5.1cm}|
+
+.. cssclass:: longtable
+
+.. flat-table:: struct v4l2_ctrl_hevc_scaling_matrix
+    :header-rows:  0
+    :stub-columns: 0
+    :widths:       1 1 2
+
+    * - __u8
+      - ``scaling_list_4x4[6][16]``
+      -
+    * - __u8
+      - ``scaling_list_8x8[6][64]``
+      -
+    * - __u8
+      - ``scaling_list_16x16[6][64]``
+      -
+    * - __u8
+      - ``scaling_list_32x32[2][64]``
+      -
+    * - __u8
+      - ``scaling_list_dc_coef_16x16[6]``
+      -
+    * - __u8
+      - ``scaling_list_dc_coef_32x32[2]``
+      -
+
+.. raw:: latex
+
+    \normalsize
+
 .. c:type:: v4l2_hevc_dpb_entry
 
 .. raw:: latex
diff --git a/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst b/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst
index f9ecf6276129..2f491c17dd5d 100644
--- a/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst
+++ b/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst
@@ -495,6 +495,12 @@  See also the examples in :ref:`control`.
       - n/a
       - A struct :c:type:`v4l2_ctrl_hevc_slice_params`, containing HEVC
 	slice parameters for stateless video decoders.
+    * - ``V4L2_CTRL_TYPE_HEVC_SCALING_MATRIX``
+      - n/a
+      - n/a
+      - n/a
+      - A struct :c:type:`v4l2_ctrl_hevc_scaling_matrix`, containing HEVC
+	scaling matrix for stateless video decoders.
     * - ``V4L2_CTRL_TYPE_VP8_FRAME``
       - n/a
       - n/a
diff --git a/drivers/media/v4l2-core/v4l2-ctrls-core.c b/drivers/media/v4l2-core/v4l2-ctrls-core.c
index c4b5082849b6..70adfc1b9c81 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls-core.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls-core.c
@@ -687,6 +687,9 @@  static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
 
 		break;
 
+	case V4L2_CTRL_TYPE_HEVC_SCALING_MATRIX:
+		break;
+
 	case V4L2_CTRL_TYPE_AREA:
 		area = p;
 		if (!area->width || !area->height)
@@ -1240,6 +1243,9 @@  static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
 	case V4L2_CTRL_TYPE_HEVC_SLICE_PARAMS:
 		elem_size = sizeof(struct v4l2_ctrl_hevc_slice_params);
 		break;
+	case V4L2_CTRL_TYPE_HEVC_SCALING_MATRIX:
+		elem_size = sizeof(struct v4l2_ctrl_hevc_scaling_matrix);
+		break;
 	case V4L2_CTRL_TYPE_HEVC_DECODE_PARAMS:
 		elem_size = sizeof(struct v4l2_ctrl_hevc_decode_params);
 		break;
diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
index b6344bbf1e00..cb29c2a7fabe 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
@@ -996,6 +996,7 @@  const char *v4l2_ctrl_get_name(u32 id)
 	case V4L2_CID_MPEG_VIDEO_HEVC_SPS:			return "HEVC Sequence Parameter Set";
 	case V4L2_CID_MPEG_VIDEO_HEVC_PPS:			return "HEVC Picture Parameter Set";
 	case V4L2_CID_MPEG_VIDEO_HEVC_SLICE_PARAMS:		return "HEVC Slice Parameters";
+	case V4L2_CID_MPEG_VIDEO_HEVC_SCALING_MATRIX:		return "HEVC Scaling Matrix";
 	case V4L2_CID_MPEG_VIDEO_HEVC_DECODE_PARAMS:		return "HEVC Decode Parameters";
 	case V4L2_CID_MPEG_VIDEO_HEVC_DECODE_MODE:		return "HEVC Decode Mode";
 	case V4L2_CID_MPEG_VIDEO_HEVC_START_CODE:		return "HEVC Start Code";
@@ -1488,6 +1489,9 @@  void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_MPEG_VIDEO_HEVC_SLICE_PARAMS:
 		*type = V4L2_CTRL_TYPE_HEVC_SLICE_PARAMS;
 		break;
+	case V4L2_CID_MPEG_VIDEO_HEVC_SCALING_MATRIX:
+		*type = V4L2_CTRL_TYPE_HEVC_SCALING_MATRIX;
+		break;
 	case V4L2_CID_MPEG_VIDEO_HEVC_DECODE_PARAMS:
 		*type = V4L2_CTRL_TYPE_HEVC_DECODE_PARAMS;
 		break;
diff --git a/include/media/hevc-ctrls.h b/include/media/hevc-ctrls.h
index 53c0038c792b..0e5c4a2eecff 100644
--- a/include/media/hevc-ctrls.h
+++ b/include/media/hevc-ctrls.h
@@ -19,6 +19,7 @@ 
 #define V4L2_CID_MPEG_VIDEO_HEVC_SPS		(V4L2_CID_CODEC_BASE + 1008)
 #define V4L2_CID_MPEG_VIDEO_HEVC_PPS		(V4L2_CID_CODEC_BASE + 1009)
 #define V4L2_CID_MPEG_VIDEO_HEVC_SLICE_PARAMS	(V4L2_CID_CODEC_BASE + 1010)
+#define V4L2_CID_MPEG_VIDEO_HEVC_SCALING_MATRIX	(V4L2_CID_CODEC_BASE + 1011)
 #define V4L2_CID_MPEG_VIDEO_HEVC_DECODE_PARAMS	(V4L2_CID_CODEC_BASE + 1012)
 #define V4L2_CID_MPEG_VIDEO_HEVC_DECODE_MODE	(V4L2_CID_CODEC_BASE + 1015)
 #define V4L2_CID_MPEG_VIDEO_HEVC_START_CODE	(V4L2_CID_CODEC_BASE + 1016)
@@ -27,6 +28,7 @@ 
 #define V4L2_CTRL_TYPE_HEVC_SPS 0x0120
 #define V4L2_CTRL_TYPE_HEVC_PPS 0x0121
 #define V4L2_CTRL_TYPE_HEVC_SLICE_PARAMS 0x0122
+#define V4L2_CTRL_TYPE_HEVC_SCALING_MATRIX 0x0123
 #define V4L2_CTRL_TYPE_HEVC_DECODE_PARAMS 0x0124
 
 enum v4l2_mpeg_video_hevc_decode_mode {
@@ -224,6 +226,15 @@  struct v4l2_ctrl_hevc_decode_params {
 	__u64	flags;
 };
 
+struct v4l2_ctrl_hevc_scaling_matrix {
+	__u8	scaling_list_4x4[6][16];
+	__u8	scaling_list_8x8[6][64];
+	__u8	scaling_list_16x16[6][64];
+	__u8	scaling_list_32x32[2][64];
+	__u8	scaling_list_dc_coef_16x16[6];
+	__u8	scaling_list_dc_coef_32x32[2];
+};
+
 /*  MPEG-class control IDs specific to the Hantro driver as defined by V4L2 */
 #define V4L2_CID_CODEC_HANTRO_BASE				(V4L2_CTRL_CLASS_CODEC | 0x1200)
 /*