[PATCH v5 04/17] media: uapi: HEVC: Add missing fields in HEVC controls
Sebastian Fricke
sebastian.fricke at collabora.com
Tue Apr 26 00:52:34 PDT 2022
Hey Benjamin,
On 25.04.2022 18:16, Benjamin Gaignard wrote:
>
>Le 25/04/2022 à 15:54, Sebastian Fricke a écrit :
>>On 07.04.2022 17:29, Benjamin Gaignard wrote:
>>>Complete the HEVC controls with missing fields from H.265
>>>specifications.
>>>Even if these fields aren't used by the current mainlined drivers
>>>they will be need for (at least) rkvdec driver.
>>
>>s/be need/be required/
>>or
>>s/be need/be needed/
>>
>>s/rkvdec/the rkvdec/
>>
>>>
>>>Signed-off-by: Benjamin Gaignard <benjamin.gaignard at collabora.com>
>>>---
>>>.../media/v4l/ext-ctrls-codec.rst | 19 +++++++++++++++++++
>>>include/media/hevc-ctrls.h | 6 +++++-
>>>2 files changed, 24 insertions(+), 1 deletion(-)
>>>
>>>diff --git
>>>a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>>>b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>>>index 4cd7c541fc30..dbb08603217b 100644
>>>--- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>>>+++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>>>@@ -2661,6 +2661,16 @@ enum v4l2_mpeg_video_hevc_size_of_length_field -
>>> :stub-columns: 0
>>> :widths: 1 1 2
>>>
>>>+ * - __u8
>>>+ - ``video_parameter_set_id``
>>>+ - Specifies the value of the vps_video_parameter_set_id of
>>>the active VPS
>>>+ as descibed in section "7.4.3.2.1 General sequence
>>>parameter set RBSP semantics"
>>>+ of H.265 specifications.
>>>+ * - __u8
>>>+ - ``seq_parameter_set_id``
>>>+ - Provides an identifier for the SPS for reference by other
>>>syntax elements
>>>+ as descibed in section "7.4.3.2.1 General sequence
>>>parameter set RBSP semantics"
>>>+ of H.265 specifications.
>>> * - __u16
>>> - ``pic_width_in_luma_samples``
>>> -
>>>@@ -2800,6 +2810,9 @@ enum v4l2_mpeg_video_hevc_size_of_length_field -
>>> :stub-columns: 0
>>> :widths: 1 1 2
>>>
>>>+ * - __u8
>>>+ - ``pic_parameter_set_id``
>>>+ - Identifies the PPS for reference by other syntax elements.
>>> * - __u8
>>> - ``num_extra_slice_header_bits``
>>> -
>>>@@ -3026,6 +3039,12 @@ enum v4l2_mpeg_video_hevc_size_of_length_field -
>>> * - __u8
>>> - ``ref_idx_l1[V4L2_HEVC_DPB_ENTRIES_NUM_MAX]``
>>> - The list of L1 reference elements as indices in the DPB.
>>>+ * - __u16
>>>+ - ``short_term_ref_pic_set_size``
>>>+ - Specifies the size of short-term reference pictures set
>>>included in the SPS.
>>
>>s/size of/size of the/
>>
>>Section 7.4.8 depicts that the st_ref_pic_set syntax
>>structure can be part of the SPS or the slice header.
>>
>>I think we should mention that we talk about the size of the
>>st_ref_pic_set
>>syntax structure from section 7.4.8 of the specification.
>>>+ * - __u16
>>>+ - ``long_term_ref_pic_set_size``
>>>+ - Specifies the size of long-term reference pictures set
>>>include in the SPS.
>>
>>s/size of/size of the/
>>
>>Can we make this a bit more helpful? The specification doesn't contain
>>a similar structure to `st_ref_pic_set` for long term pictures. So, as a
>>programmer this leaves me guessing:
>>- Which syntax structure's size are we talking about?
>>- Does this correlate to any of the existing sections of the
>>specification?
>>Because in the end, I feel like this documentation should be able to
>>help a programmer to provide the correct data for the uABI.
>
>I will reword it like that:
>
> * - __u16
> - ``short_term_ref_pic_set_size``
> - Specifies the size, in bits, of the short-term reference pictures set, described as st_ref_pic_set()
Just ...
s/reference pictures set/reference picture set/
> in the specification, included in the slice header (section 7.3.6.1).
s/slice header/slice header or SPS/
>
> * - __u16
> - ``long_term_ref_pic_set_size``
> - Specifies the size, in bits, of the long-term reference pictures set include in the slice header.
s/reference pictures set/reference picture set/
Looking at the documentation it looks like: s/slice header/slice header or SPS/
(For example in section 7.3.6.1:
```
if( long_term_ref_pics_present_flag ) {
if( num_long_term_ref_pics_sps > 0 )
num_long_term_sps
num_long_term_pics
for( i = 0; i < num_long_term_sps + num_long_term_pics; i++ ) {
...
```
)
... and then it looks very good!
Greetings,
Sebastian
> It is the number of bits in the conditional block if( long_term_ref_pics_present_flag ) {...}
> in section 7.3.6.1 of the specification.
>
>Benjamin
>
>
>>
>>Greetings,
>>Sebastian
>>
>>> * - __u8
>>> - ``padding``
>>> - Applications and drivers must set this to zero.
>>>diff --git a/include/media/hevc-ctrls.h b/include/media/hevc-ctrls.h
>>>index 01ccda48d8c5..a329e086a89a 100644
>>>--- a/include/media/hevc-ctrls.h
>>>+++ b/include/media/hevc-ctrls.h
>>>@@ -58,6 +58,8 @@ enum v4l2_mpeg_video_hevc_start_code {
>>>/* The controls are not stable at the moment and will likely be
>>>reworked. */
>>>struct v4l2_ctrl_hevc_sps {
>>> /* ISO/IEC 23008-2, ITU-T Rec. H.265: Sequence parameter set */
>>>+ __u8 video_parameter_set_id;
>>>+ __u8 seq_parameter_set_id;
>>> __u16 pic_width_in_luma_samples;
>>> __u16 pic_height_in_luma_samples;
>>> __u8 bit_depth_luma_minus8;
>>>@@ -108,6 +110,7 @@ struct v4l2_ctrl_hevc_sps {
>>>
>>>struct v4l2_ctrl_hevc_pps {
>>> /* ISO/IEC 23008-2, ITU-T Rec. H.265: Picture parameter set */
>>>+ __u8 pic_parameter_set_id;
>>> __u8 num_extra_slice_header_bits;
>>> __u8 num_ref_idx_l0_default_active_minus1;
>>> __u8 num_ref_idx_l1_default_active_minus1;
>>>@@ -199,7 +202,8 @@ struct v4l2_ctrl_hevc_slice_params {
>>> __u32 slice_segment_addr;
>>> __u8 ref_idx_l0[V4L2_HEVC_DPB_ENTRIES_NUM_MAX];
>>> __u8 ref_idx_l1[V4L2_HEVC_DPB_ENTRIES_NUM_MAX];
>>>-
>>>+ __u16 short_term_ref_pic_set_size;
>>>+ __u16 long_term_ref_pic_set_size;
>>> __u8 padding;
>>>
>>> /* ISO/IEC 23008-2, ITU-T Rec. H.265: Weighted prediction
>>>parameter */
>>>--
>>>2.32.0
>>>
More information about the Linux-rockchip
mailing list