| Message ID | 20210625141143.577998-9-benjamin.gaignard@collabora.com |
|---|---|
| State | Accepted |
| Commit | 7ba59fb6c3b473dc0c76e87cd493388480c6dd27 |
| Headers | show |
| Series | Additional features for Hantro HEVC | expand |
Hi Benjamin, I believe the scaling matrix uAPI patch(es) and the support in Hantro G2 could be moved to its own series and maybe merged sooner than the rest (which may need more discussion). Couple remarks below. On Fri, 2021-06-25 at 16:11 +0200, 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. > > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com> > --- > Note: V4L2_HEVC_SPS_FLAG_SCALING_LIST_ENABLED is not change by this > patch. There is a thread about the naming/usage of this flag here: > https://lore.kernel.org/linux-arm-kernel/20210610154442.806107-8-benjamin.gaignard@collabora.com/ > but that doesn't concern the scaling matrix control by itself. > If I am reading the spec correctly, we have fields in SPS (scaling_list_enabled_flag, scaling_list_data_present_flag) and PPS (scaling_list_data_present_flag). We don't need all that, since all a driver needs to know is whether a scaling list is to be applied for the current frame. Would you mind adding a patch moving the flag to the PPS? > 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 dc096a5562cd..3865acb9e0fd 100644 > --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst > +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst > @@ -3071,6 +3071,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. > + This needs some additional documentation about the order of the lists. See the docs that we've added for the scaling_list_{} fields in V4L2_CID_STATELESS_H264_SCALING_MATRIX. Thanks! -- Kindly, Ezequiel
Le 13/07/2021 à 00:21, Ezequiel Garcia a écrit : > Hi Benjamin, > > I believe the scaling matrix uAPI patch(es) and the support > in Hantro G2 could be moved to its own series and maybe > merged sooner than the rest (which may need more discussion). > > Couple remarks below. > > On Fri, 2021-06-25 at 16:11 +0200, 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. >> >> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com> >> --- >> Note: V4L2_HEVC_SPS_FLAG_SCALING_LIST_ENABLED is not change by this >> patch. There is a thread about the naming/usage of this flag here: >> https://lore.kernel.org/linux-arm-kernel/20210610154442.806107-8-benjamin.gaignard@collabora.com/ >> but that doesn't concern the scaling matrix control by itself. >> > If I am reading the spec correctly, we have fields in > SPS (scaling_list_enabled_flag, scaling_list_data_present_flag) > and PPS (scaling_list_data_present_flag). > > We don't need all that, since all a driver needs to know > is whether a scaling list is to be applied for the current frame. > > Would you mind adding a patch moving the flag to the PPS? Extract from the spec: - scaling_list_enabled_flag equal 1 specifies that a scaling list is used for scaling process for transform coefficients. - sps_scaling_list_data_present_flag equal to 1 specifies that the scaling_list_data( ) syntax structure is present in the SPS. - pps_scaling_list_data_present_flag equal to 1 specifies that parameters are present in the PPS to modify the scaling lists specified by the active SPS. For me V4L2_HEVC_SPS_FLAG_SCALING_LIST_ENABLED represent scaling_list_enabled_flag while the other are used to build the scaling lists values. So it is good named from my point of view. > >> 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 dc096a5562cd..3865acb9e0fd 100644 >> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst >> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst >> @@ -3071,6 +3071,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. >> + > This needs some additional documentation about the order of the lists. > See the docs that we've added for the scaling_list_{} fields in > V4L2_CID_STATELESS_H264_SCALING_MATRIX. I will do dedicated patches for scaling lists feature and add that. Benjamin > > Thanks!
diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst index dc096a5562cd..3865acb9e0fd 100644 --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst @@ -3071,6 +3071,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 781371bff2ad..ef63bc205756 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 { @@ -225,6 +227,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) /*
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> --- Note: V4L2_HEVC_SPS_FLAG_SCALING_LIST_ENABLED is not change by this patch. There is a thread about the naming/usage of this flag here: https://lore.kernel.org/linux-arm-kernel/20210610154442.806107-8-benjamin.gaignard@collabora.com/ but that doesn't concern the scaling matrix control by itself. 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(+)