Re: [PATCH v3 1/9] media: hevc: Modify structures to follow H265 ITU spec
Le jeudi 25 février 2021 à 18:34 +, John Cox a écrit : > On Thu, 25 Feb 2021 19:05:55 +0100, you wrote: > > > Dne ?etrtek, 25. februar 2021 ob 18:34:48 CET je Ezequiel Garcia napisal(a): > > > Hey Jernej, > > > > > > On Thu, 2021-02-25 at 18:01 +0100, Jernej Škrabec wrote: > > > > Hi Ezequiel, > > > > > > > > Dne ?etrtek, 25. februar 2021 ob 14:09:52 CET je Ezequiel Garcia > > napisal(a): > > > > > Hi Benjamin, > > > > > > > > > > Thanks for the good work. > > > > > > > > > > On Mon, 2021-02-22 at 13:23 +0100, Benjamin Gaignard wrote: > > > > > > The H.265 ITU specification (section 7.4) define the general > > > > > > slice segment header semantics. > > > > > > Modified/added fields are: > > > > > > - video_parameter_set_id: (7.4.3.1) identifies the VPS for > > > > > > reference by other syntax elements. > > > > > > - seq_parameter_set_id: (7.4.3.2.1) specifies the value of > > > > > > the vps_video_parameter_set_id of the active VPS. > > > > > > - chroma_format_idc: (7.4.3.2.1) specifies the chroma sampling > > > > > > relative to the luma sampling > > > > > > - pic_parameter_set_id: (7.4.3.3.1) identifies the PPS for > > > > > > reference by other syntax elements > > > > > > - num_ref_idx_l0_default_active_minus1: (7.4.3.3.1) specifies > > > > > > the inferred value of num_ref_idx_l0_active_minus1 > > > > > > - num_ref_idx_l1_default_active_minus1: (7.4.3.3.1) specifies > > > > > > the inferred value of num_ref_idx_l1_active_minus1 > > > > > > - slice_segment_addr: (7.4.7.1) specifies the address of > > > > > > the first coding tree block in the slice segment > > > > > > - num_entry_point_offsets: (7.4.7.1) specifies the number of > > > > > > entry_point_offset_minus1[ i ] syntax elements in the slice header > > > > > > > > > > > > Add HEVC decode params contains the information used in section > > > > > > "8.3 Slice decoding process" of the specification to let the > > > > > > hardware > > > > > > perform decoding of a slices. > > > > > > > > > > > > Adapt Cedrus driver according to these changes. > > > > > > > > > > > > Signed-off-by: Benjamin Gaignard > > > > > > --- > > > > > > version 3: > > > > > > - Add documentation about the new structuers and fields. > > > > > > > > > > > > version 2: > > > > > > - remove all change related to scaling > > > > > > - squash commits to a coherent split > > > > > > - be more verbose about the added fields > > > > > > > > > > > > .../media/v4l/ext-ctrls-codec.rst | 126 ++ > > +--- > > > > > > .../media/v4l/vidioc-queryctrl.rst | 6 + > > > > > > drivers/media/v4l2-core/v4l2-ctrls.c | 26 +++- > > > > > > drivers/staging/media/sunxi/cedrus/cedrus.c | 6 + > > > > > > drivers/staging/media/sunxi/cedrus/cedrus.h | 1 + > > > > > > .../staging/media/sunxi/cedrus/cedrus_dec.c | 2 + > > > > > > .../staging/media/sunxi/cedrus/cedrus_h265.c | 6 +- > > > > > > include/media/hevc-ctrls.h | 45 +-- > > > > > > 8 files changed, 186 insertions(+), 32 deletions(-) > > > > > > > > > > > > diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls- > > > > > > codec.rst > > b/ > > > > Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst > > > > > > index 00944e97d638..5e6d77e858c0 100644 > > > > > > --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst > > > > > > +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst > > > > > > @@ -3109,6 +3109,15 @@ enum > > > > > > v4l2_mpeg_video_hevc_size_of_length_field - > > > > > > :stub-columns: 0 > > > > > > :widths: 1 1 2 > > > > > > > > > > > > + * - __u8 > > > > > > + - ``video_parameter_set_id`` > > > > > > + - Identifies the VPS for reference by other syntax elements > > > > > > + * - __u8 > > > > > > + - ``seq_parameter_set_id?`` > > > > > > + - Specifies the value of the vps_video_parameter_set_id of > > > > > > the > > > > active VPS > > > > > > + * - __u8 > > > > > > + - ``chroma_format_idc`` > > > > > > + - Specifies the chroma sampling relative to the luma sampling > > > > > > > > > > None of these fields seem needed for the Hantro G2 driver, > > > > > so I suggest you drop them for now. > > > > > > > > > > > * - __u16 > > > > > > - ``pic_width_in_luma_samples`` > > > > > > - > > > > > > @@ -3172,6 +3181,9 @@ enum v4l2_mpeg_video_hevc_size_of_length_field > > > > > > - > > > > > > * - __u8 > > > > > > - ``chroma_format_idc`` > > > > > > - > > > > > > + * - __u8 > > > > > > + - ``num_slices`` > > > > > > + > > > > > > > > > > Not used, but also doesn't seem part of the SPS syntax. If we have to > > > > > pass the number of slices, we'll need another mechanism. > > > > > > > > > > > - > > > > > > * - __u64 > > > > > > - ``flags`` > > > > > > - See :ref:`Sequence Parameter Set Flags ` > > > > > > @@ -3231,9 +3243,18 @@ enum > > > > > >
Re: [PATCH v3 1/9] media: hevc: Modify structures to follow H265 ITU spec
Le jeudi 25 février 2021 à 10:09 -0300, Ezequiel Garcia a écrit : > Hi Benjamin, > > Thanks for the good work. > > On Mon, 2021-02-22 at 13:23 +0100, Benjamin Gaignard wrote: > > The H.265 ITU specification (section 7.4) define the general > > slice segment header semantics. > > Modified/added fields are: > > - video_parameter_set_id: (7.4.3.1) identifies the VPS for > > reference by other syntax elements. > > - seq_parameter_set_id: (7.4.3.2.1) specifies the value of > > the vps_video_parameter_set_id of the active VPS. > > - chroma_format_idc: (7.4.3.2.1) specifies the chroma sampling > > relative to the luma sampling > > - pic_parameter_set_id: (7.4.3.3.1) identifies the PPS for > > reference by other syntax elements > > - num_ref_idx_l0_default_active_minus1: (7.4.3.3.1) specifies > > the inferred value of num_ref_idx_l0_active_minus1 > > - num_ref_idx_l1_default_active_minus1: (7.4.3.3.1) specifies > > the inferred value of num_ref_idx_l1_active_minus1 > > - slice_segment_addr: (7.4.7.1) specifies the address of > > the first coding tree block in the slice segment > > - num_entry_point_offsets: (7.4.7.1) specifies the number of > > entry_point_offset_minus1[ i ] syntax elements in the slice header > > > > Add HEVC decode params contains the information used in section > > "8.3 Slice decoding process" of the specification to let the hardware > > perform decoding of a slices. > > > > Adapt Cedrus driver according to these changes. > > > > Signed-off-by: Benjamin Gaignard > > --- > > version 3: > > - Add documentation about the new structuers and fields. > > > > version 2: > > - remove all change related to scaling > > - squash commits to a coherent split > > - be more verbose about the added fields > > > > .../media/v4l/ext-ctrls-codec.rst | 126 +++--- > > .../media/v4l/vidioc-queryctrl.rst | 6 + > > drivers/media/v4l2-core/v4l2-ctrls.c | 26 +++- > > drivers/staging/media/sunxi/cedrus/cedrus.c | 6 + > > drivers/staging/media/sunxi/cedrus/cedrus.h | 1 + > > .../staging/media/sunxi/cedrus/cedrus_dec.c | 2 + > > .../staging/media/sunxi/cedrus/cedrus_h265.c | 6 +- > > include/media/hevc-ctrls.h | 45 +-- > > 8 files changed, 186 insertions(+), 32 deletions(-) > > > > diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst > > b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst > > index 00944e97d638..5e6d77e858c0 100644 > > --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst > > +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst > > @@ -3109,6 +3109,15 @@ enum v4l2_mpeg_video_hevc_size_of_length_field - > > :stub-columns: 0 > > :widths: 1 1 2 > > > > + * - __u8 > > + - ``video_parameter_set_id`` > > + - Identifies the VPS for reference by other syntax elements > > + * - __u8 > > + - ``seq_parameter_set_id̀`` > > + - Specifies the value of the vps_video_parameter_set_id of the active > > VPS > > + * - __u8 > > + - ``chroma_format_idc`` > > + - Specifies the chroma sampling relative to the luma sampling > > None of these fields seem needed for the Hantro G2 driver, > so I suggest you drop them for now. You should be using chroma_format_idc in your validation. I think Hantro G2 does not do YUV 4:4:4, which is what chroma_format_idc tells the driver. Our implementation might also be missing 4:0:0 (black and white). As this value will aftect the buffer size, please keep them, and ideally validate them. Note that it feels rather problematic / unsafe situation as we endup having to trust userspace, hopefully the HW validates it's buffer size ? (do we tell the HW the buffer sizes ?) > > > * - __u16 > > - ``pic_width_in_luma_samples`` > > - > > @@ -3172,6 +3181,9 @@ enum v4l2_mpeg_video_hevc_size_of_length_field - > > * - __u8 > > - ``chroma_format_idc`` > > - > > + * - __u8 > > + - ``num_slices`` > > + > > Not used, but also doesn't seem part of the SPS syntax. If we have to > pass the number of slices, we'll need another mechanism. > > > - > > * - __u64 > > - ``flags`` > > - See :ref:`Sequence Parameter Set Flags ` > > @@ -3231,9 +3243,18 @@ 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 > > Not used. > > > * - __u8 > > - ``num_extra_slice_header_bits`` > > - > > + * - __u8 > > + - ``num_ref_idx_l0_default_active_minus1`` > > + - Specifies the inferred value of num_ref_idx_l0_active_minus1 > > + * - __u8 > > + - ``num_ref_idx_l1_default_active_minus1`` > > + - Specifies the inferred value of num_ref_idx_l1_active_minus1 > > * - __s8 > > - ``init_qp_minus26`` > > - > > @@ -3342,6
Re: [PATCH v3 1/9] media: hevc: Modify structures to follow H265 ITU spec
On Thu, 25 Feb 2021 19:05:55 +0100, you wrote: >Dne ?etrtek, 25. februar 2021 ob 18:34:48 CET je Ezequiel Garcia napisal(a): >> Hey Jernej, >> >> On Thu, 2021-02-25 at 18:01 +0100, Jernej Škrabec wrote: >> > Hi Ezequiel, >> > >> > Dne ?etrtek, 25. februar 2021 ob 14:09:52 CET je Ezequiel Garcia >napisal(a): >> > > Hi Benjamin, >> > > >> > > Thanks for the good work. >> > > >> > > On Mon, 2021-02-22 at 13:23 +0100, Benjamin Gaignard wrote: >> > > > The H.265 ITU specification (section 7.4) define the general >> > > > slice segment header semantics. >> > > > Modified/added fields are: >> > > > - video_parameter_set_id: (7.4.3.1) identifies the VPS for >> > > > reference by other syntax elements. >> > > > - seq_parameter_set_id: (7.4.3.2.1) specifies the value of >> > > > the vps_video_parameter_set_id of the active VPS. >> > > > - chroma_format_idc: (7.4.3.2.1) specifies the chroma sampling >> > > > relative to the luma sampling >> > > > - pic_parameter_set_id: (7.4.3.3.1) identifies the PPS for >> > > > reference by other syntax elements >> > > > - num_ref_idx_l0_default_active_minus1: (7.4.3.3.1) specifies >> > > > the inferred value of num_ref_idx_l0_active_minus1 >> > > > - num_ref_idx_l1_default_active_minus1: (7.4.3.3.1) specifies >> > > > the inferred value of num_ref_idx_l1_active_minus1 >> > > > - slice_segment_addr: (7.4.7.1) specifies the address of >> > > > the first coding tree block in the slice segment >> > > > - num_entry_point_offsets: (7.4.7.1) specifies the number of >> > > > entry_point_offset_minus1[ i ] syntax elements in the slice header >> > > > >> > > > Add HEVC decode params contains the information used in section >> > > > "8.3 Slice decoding process" of the specification to let the hardware >> > > > perform decoding of a slices. >> > > > >> > > > Adapt Cedrus driver according to these changes. >> > > > >> > > > Signed-off-by: Benjamin Gaignard >> > > > --- >> > > > version 3: >> > > > - Add documentation about the new structuers and fields. >> > > > >> > > > version 2: >> > > > - remove all change related to scaling >> > > > - squash commits to a coherent split >> > > > - be more verbose about the added fields >> > > > >> > > > .../media/v4l/ext-ctrls-codec.rst | 126 ++ >+--- >> > > > .../media/v4l/vidioc-queryctrl.rst| 6 + >> > > > drivers/media/v4l2-core/v4l2-ctrls.c | 26 +++- >> > > > drivers/staging/media/sunxi/cedrus/cedrus.c | 6 + >> > > > drivers/staging/media/sunxi/cedrus/cedrus.h | 1 + >> > > > .../staging/media/sunxi/cedrus/cedrus_dec.c | 2 + >> > > > .../staging/media/sunxi/cedrus/cedrus_h265.c | 6 +- >> > > > include/media/hevc-ctrls.h| 45 +-- >> > > > 8 files changed, 186 insertions(+), 32 deletions(-) >> > > > >> > > > diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst >b/ >> > Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst >> > > > index 00944e97d638..5e6d77e858c0 100644 >> > > > --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst >> > > > +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst >> > > > @@ -3109,6 +3109,15 @@ enum v4l2_mpeg_video_hevc_size_of_length_field - >> > > > :stub-columns: 0 >> > > > :widths: 1 1 2 >> > > > >> > > > +* - __u8 >> > > > + - ``video_parameter_set_id`` >> > > > + - Identifies the VPS for reference by other syntax elements >> > > > +* - __u8 >> > > > + - ``seq_parameter_set_id?`` >> > > > + - Specifies the value of the vps_video_parameter_set_id of the >> > active VPS >> > > > +* - __u8 >> > > > + - ``chroma_format_idc`` >> > > > + - Specifies the chroma sampling relative to the luma sampling >> > > >> > > None of these fields seem needed for the Hantro G2 driver, >> > > so I suggest you drop them for now. >> > > >> > > > * - __u16 >> > > >- ``pic_width_in_luma_samples`` >> > > >- >> > > > @@ -3172,6 +3181,9 @@ enum v4l2_mpeg_video_hevc_size_of_length_field - >> > > > * - __u8 >> > > >- ``chroma_format_idc`` >> > > >- >> > > > +* - __u8 >> > > > + - ``num_slices`` >> > > > + >> > > >> > > Not used, but also doesn't seem part of the SPS syntax. If we have to >> > > pass the number of slices, we'll need another mechanism. >> > > >> > > > - >> > > > * - __u64 >> > > >- ``flags`` >> > > >- See :ref:`Sequence Parameter Set Flags ` >> > > > @@ -3231,9 +3243,18 @@ 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 >> > > >> > > Not used. >> > > >> > > > * - __u8 >> > > >- ``num_extra_slice_header_bits`` >> > > >- >> > > > +* - __u8 >> > > > + - ``num_ref_idx_l0_default_active_minus1`` >> > >
Re: Re: Re: [PATCH v3 1/9] media: hevc: Modify structures to follow H265 ITU spec
Dne četrtek, 25. februar 2021 ob 18:34:48 CET je Ezequiel Garcia napisal(a): > Hey Jernej, > > On Thu, 2021-02-25 at 18:01 +0100, Jernej Škrabec wrote: > > Hi Ezequiel, > > > > Dne četrtek, 25. februar 2021 ob 14:09:52 CET je Ezequiel Garcia napisal(a): > > > Hi Benjamin, > > > > > > Thanks for the good work. > > > > > > On Mon, 2021-02-22 at 13:23 +0100, Benjamin Gaignard wrote: > > > > The H.265 ITU specification (section 7.4) define the general > > > > slice segment header semantics. > > > > Modified/added fields are: > > > > - video_parameter_set_id: (7.4.3.1) identifies the VPS for > > > > reference by other syntax elements. > > > > - seq_parameter_set_id: (7.4.3.2.1) specifies the value of > > > > the vps_video_parameter_set_id of the active VPS. > > > > - chroma_format_idc: (7.4.3.2.1) specifies the chroma sampling > > > > relative to the luma sampling > > > > - pic_parameter_set_id: (7.4.3.3.1) identifies the PPS for > > > > reference by other syntax elements > > > > - num_ref_idx_l0_default_active_minus1: (7.4.3.3.1) specifies > > > > the inferred value of num_ref_idx_l0_active_minus1 > > > > - num_ref_idx_l1_default_active_minus1: (7.4.3.3.1) specifies > > > > the inferred value of num_ref_idx_l1_active_minus1 > > > > - slice_segment_addr: (7.4.7.1) specifies the address of > > > > the first coding tree block in the slice segment > > > > - num_entry_point_offsets: (7.4.7.1) specifies the number of > > > > entry_point_offset_minus1[ i ] syntax elements in the slice header > > > > > > > > Add HEVC decode params contains the information used in section > > > > "8.3 Slice decoding process" of the specification to let the hardware > > > > perform decoding of a slices. > > > > > > > > Adapt Cedrus driver according to these changes. > > > > > > > > Signed-off-by: Benjamin Gaignard > > > > --- > > > > version 3: > > > > - Add documentation about the new structuers and fields. > > > > > > > > version 2: > > > > - remove all change related to scaling > > > > - squash commits to a coherent split > > > > - be more verbose about the added fields > > > > > > > > .../media/v4l/ext-ctrls-codec.rst | 126 ++ +--- > > > > .../media/v4l/vidioc-queryctrl.rst| 6 + > > > > drivers/media/v4l2-core/v4l2-ctrls.c | 26 +++- > > > > drivers/staging/media/sunxi/cedrus/cedrus.c | 6 + > > > > drivers/staging/media/sunxi/cedrus/cedrus.h | 1 + > > > > .../staging/media/sunxi/cedrus/cedrus_dec.c | 2 + > > > > .../staging/media/sunxi/cedrus/cedrus_h265.c | 6 +- > > > > include/media/hevc-ctrls.h| 45 +-- > > > > 8 files changed, 186 insertions(+), 32 deletions(-) > > > > > > > > diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/ > > Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst > > > > index 00944e97d638..5e6d77e858c0 100644 > > > > --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst > > > > +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst > > > > @@ -3109,6 +3109,15 @@ enum v4l2_mpeg_video_hevc_size_of_length_field - > > > > :stub-columns: 0 > > > > :widths: 1 1 2 > > > > > > > > +* - __u8 > > > > + - ``video_parameter_set_id`` > > > > + - Identifies the VPS for reference by other syntax elements > > > > +* - __u8 > > > > + - ``seq_parameter_set_id̀`` > > > > + - Specifies the value of the vps_video_parameter_set_id of the > > active VPS > > > > +* - __u8 > > > > + - ``chroma_format_idc`` > > > > + - Specifies the chroma sampling relative to the luma sampling > > > > > > None of these fields seem needed for the Hantro G2 driver, > > > so I suggest you drop them for now. > > > > > > > * - __u16 > > > >- ``pic_width_in_luma_samples`` > > > >- > > > > @@ -3172,6 +3181,9 @@ enum v4l2_mpeg_video_hevc_size_of_length_field - > > > > * - __u8 > > > >- ``chroma_format_idc`` > > > >- > > > > +* - __u8 > > > > + - ``num_slices`` > > > > + > > > > > > Not used, but also doesn't seem part of the SPS syntax. If we have to > > > pass the number of slices, we'll need another mechanism. > > > > > > > - > > > > * - __u64 > > > >- ``flags`` > > > >- See :ref:`Sequence Parameter Set Flags ` > > > > @@ -3231,9 +3243,18 @@ 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 > > > > > > Not used. > > > > > > > * - __u8 > > > >- ``num_extra_slice_header_bits`` > > > >- > > > > +* - __u8 > > > > + - ``num_ref_idx_l0_default_active_minus1`` > > > > + - Specifies the inferred value of num_ref_idx_l0_active_minus1 > > > > +* - __u8 > > > > + - ``num_ref_idx_l1_default_active_minus1`` > > > > +
Re: Re: [PATCH v3 1/9] media: hevc: Modify structures to follow H265 ITU spec
Hey Jernej, On Thu, 2021-02-25 at 18:01 +0100, Jernej Škrabec wrote: > Hi Ezequiel, > > Dne četrtek, 25. februar 2021 ob 14:09:52 CET je Ezequiel Garcia napisal(a): > > Hi Benjamin, > > > > Thanks for the good work. > > > > On Mon, 2021-02-22 at 13:23 +0100, Benjamin Gaignard wrote: > > > The H.265 ITU specification (section 7.4) define the general > > > slice segment header semantics. > > > Modified/added fields are: > > > - video_parameter_set_id: (7.4.3.1) identifies the VPS for > > > reference by other syntax elements. > > > - seq_parameter_set_id: (7.4.3.2.1) specifies the value of > > > the vps_video_parameter_set_id of the active VPS. > > > - chroma_format_idc: (7.4.3.2.1) specifies the chroma sampling > > > relative to the luma sampling > > > - pic_parameter_set_id: (7.4.3.3.1) identifies the PPS for > > > reference by other syntax elements > > > - num_ref_idx_l0_default_active_minus1: (7.4.3.3.1) specifies > > > the inferred value of num_ref_idx_l0_active_minus1 > > > - num_ref_idx_l1_default_active_minus1: (7.4.3.3.1) specifies > > > the inferred value of num_ref_idx_l1_active_minus1 > > > - slice_segment_addr: (7.4.7.1) specifies the address of > > > the first coding tree block in the slice segment > > > - num_entry_point_offsets: (7.4.7.1) specifies the number of > > > entry_point_offset_minus1[ i ] syntax elements in the slice header > > > > > > Add HEVC decode params contains the information used in section > > > "8.3 Slice decoding process" of the specification to let the hardware > > > perform decoding of a slices. > > > > > > Adapt Cedrus driver according to these changes. > > > > > > Signed-off-by: Benjamin Gaignard > > > --- > > > version 3: > > > - Add documentation about the new structuers and fields. > > > > > > version 2: > > > - remove all change related to scaling > > > - squash commits to a coherent split > > > - be more verbose about the added fields > > > > > > .../media/v4l/ext-ctrls-codec.rst | 126 +++--- > > > .../media/v4l/vidioc-queryctrl.rst | 6 + > > > drivers/media/v4l2-core/v4l2-ctrls.c | 26 +++- > > > drivers/staging/media/sunxi/cedrus/cedrus.c | 6 + > > > drivers/staging/media/sunxi/cedrus/cedrus.h | 1 + > > > .../staging/media/sunxi/cedrus/cedrus_dec.c | 2 + > > > .../staging/media/sunxi/cedrus/cedrus_h265.c | 6 +- > > > include/media/hevc-ctrls.h | 45 +-- > > > 8 files changed, 186 insertions(+), 32 deletions(-) > > > > > > diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/ > Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst > > > index 00944e97d638..5e6d77e858c0 100644 > > > --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst > > > +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst > > > @@ -3109,6 +3109,15 @@ enum v4l2_mpeg_video_hevc_size_of_length_field - > > > :stub-columns: 0 > > > :widths: 1 1 2 > > > > > > + * - __u8 > > > + - ``video_parameter_set_id`` > > > + - Identifies the VPS for reference by other syntax elements > > > + * - __u8 > > > + - ``seq_parameter_set_id̀`` > > > + - Specifies the value of the vps_video_parameter_set_id of the > active VPS > > > + * - __u8 > > > + - ``chroma_format_idc`` > > > + - Specifies the chroma sampling relative to the luma sampling > > > > None of these fields seem needed for the Hantro G2 driver, > > so I suggest you drop them for now. > > > > > * - __u16 > > > - ``pic_width_in_luma_samples`` > > > - > > > @@ -3172,6 +3181,9 @@ enum v4l2_mpeg_video_hevc_size_of_length_field - > > > * - __u8 > > > - ``chroma_format_idc`` > > > - > > > + * - __u8 > > > + - ``num_slices`` > > > + > > > > Not used, but also doesn't seem part of the SPS syntax. If we have to > > pass the number of slices, we'll need another mechanism. > > > > > - > > > * - __u64 > > > - ``flags`` > > > - See :ref:`Sequence Parameter Set Flags ` > > > @@ -3231,9 +3243,18 @@ 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 > > > > Not used. > > > > > * - __u8 > > > - ``num_extra_slice_header_bits`` > > > - > > > + * - __u8 > > > + - ``num_ref_idx_l0_default_active_minus1`` > > > + - Specifies the inferred value of num_ref_idx_l0_active_minus1 > > > + * - __u8 > > > + - ``num_ref_idx_l1_default_active_minus1`` > > > + - Specifies the inferred value of num_ref_idx_l1_active_minus1 > > > * - __s8 > > > - ``init_qp_minus26`` > > > - > > > @@ -3342,6 +3363,12 @@ enum v4l2_mpeg_video_hevc_size_of_length_field - > > > * - ``V4L2_HEVC_PPS_FLAG_SLICE_SEGMENT_HEADER_EXTENSION_PRESENT`` > > > -
Re: Re: [PATCH v3 1/9] media: hevc: Modify structures to follow H265 ITU spec
Hi Ezequiel, Dne četrtek, 25. februar 2021 ob 14:09:52 CET je Ezequiel Garcia napisal(a): > Hi Benjamin, > > Thanks for the good work. > > On Mon, 2021-02-22 at 13:23 +0100, Benjamin Gaignard wrote: > > The H.265 ITU specification (section 7.4) define the general > > slice segment header semantics. > > Modified/added fields are: > > - video_parameter_set_id: (7.4.3.1) identifies the VPS for > > reference by other syntax elements. > > - seq_parameter_set_id: (7.4.3.2.1) specifies the value of > > the vps_video_parameter_set_id of the active VPS. > > - chroma_format_idc: (7.4.3.2.1) specifies the chroma sampling > > relative to the luma sampling > > - pic_parameter_set_id: (7.4.3.3.1) identifies the PPS for > > reference by other syntax elements > > - num_ref_idx_l0_default_active_minus1: (7.4.3.3.1) specifies > > the inferred value of num_ref_idx_l0_active_minus1 > > - num_ref_idx_l1_default_active_minus1: (7.4.3.3.1) specifies > > the inferred value of num_ref_idx_l1_active_minus1 > > - slice_segment_addr: (7.4.7.1) specifies the address of > > the first coding tree block in the slice segment > > - num_entry_point_offsets: (7.4.7.1) specifies the number of > > entry_point_offset_minus1[ i ] syntax elements in the slice header > > > > Add HEVC decode params contains the information used in section > > "8.3 Slice decoding process" of the specification to let the hardware > > perform decoding of a slices. > > > > Adapt Cedrus driver according to these changes. > > > > Signed-off-by: Benjamin Gaignard > > --- > > version 3: > > - Add documentation about the new structuers and fields. > > > > version 2: > > - remove all change related to scaling > > - squash commits to a coherent split > > - be more verbose about the added fields > > > > .../media/v4l/ext-ctrls-codec.rst | 126 +++--- > > .../media/v4l/vidioc-queryctrl.rst| 6 + > > drivers/media/v4l2-core/v4l2-ctrls.c | 26 +++- > > drivers/staging/media/sunxi/cedrus/cedrus.c | 6 + > > drivers/staging/media/sunxi/cedrus/cedrus.h | 1 + > > .../staging/media/sunxi/cedrus/cedrus_dec.c | 2 + > > .../staging/media/sunxi/cedrus/cedrus_h265.c | 6 +- > > include/media/hevc-ctrls.h| 45 +-- > > 8 files changed, 186 insertions(+), 32 deletions(-) > > > > diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/ Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst > > index 00944e97d638..5e6d77e858c0 100644 > > --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst > > +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst > > @@ -3109,6 +3109,15 @@ enum v4l2_mpeg_video_hevc_size_of_length_field - > > :stub-columns: 0 > > :widths: 1 1 2 > > > > +* - __u8 > > + - ``video_parameter_set_id`` > > + - Identifies the VPS for reference by other syntax elements > > +* - __u8 > > + - ``seq_parameter_set_id̀`` > > + - Specifies the value of the vps_video_parameter_set_id of the active VPS > > +* - __u8 > > + - ``chroma_format_idc`` > > + - Specifies the chroma sampling relative to the luma sampling > > None of these fields seem needed for the Hantro G2 driver, > so I suggest you drop them for now. > > > * - __u16 > >- ``pic_width_in_luma_samples`` > >- > > @@ -3172,6 +3181,9 @@ enum v4l2_mpeg_video_hevc_size_of_length_field - > > * - __u8 > >- ``chroma_format_idc`` > >- > > +* - __u8 > > + - ``num_slices`` > > + > > Not used, but also doesn't seem part of the SPS syntax. If we have to > pass the number of slices, we'll need another mechanism. > > > - > > * - __u64 > >- ``flags`` > >- See :ref:`Sequence Parameter Set Flags ` > > @@ -3231,9 +3243,18 @@ 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 > > Not used. > > > * - __u8 > >- ``num_extra_slice_header_bits`` > >- > > +* - __u8 > > + - ``num_ref_idx_l0_default_active_minus1`` > > + - Specifies the inferred value of num_ref_idx_l0_active_minus1 > > +* - __u8 > > + - ``num_ref_idx_l1_default_active_minus1`` > > + - Specifies the inferred value of num_ref_idx_l1_active_minus1 > > * - __s8 > >- ``init_qp_minus26`` > >- > > @@ -3342,6 +3363,12 @@ enum v4l2_mpeg_video_hevc_size_of_length_field - > > * - ``V4L2_HEVC_PPS_FLAG_SLICE_SEGMENT_HEADER_EXTENSION_PRESENT`` > >- 0x0004 > >- > > +* - ``V4L2_HEVC_PPS_FLAG_DEBLOCKING_FILTER_CONTROL_PRESENT`` > > + - 0x0008 > > + - > > +* - ``V4L2_HEVC_PPS_FLAG_UNIFORM_SPACING`` > > + - 0x0010 > > + - > > > > I suggest to do all the PPS control changes in a separate patch, > feels easier to
Re: [PATCH v3 1/9] media: hevc: Modify structures to follow H265 ITU spec
Hi Benjamin, Thanks for the good work. On Mon, 2021-02-22 at 13:23 +0100, Benjamin Gaignard wrote: > The H.265 ITU specification (section 7.4) define the general > slice segment header semantics. > Modified/added fields are: > - video_parameter_set_id: (7.4.3.1) identifies the VPS for > reference by other syntax elements. > - seq_parameter_set_id: (7.4.3.2.1) specifies the value of > the vps_video_parameter_set_id of the active VPS. > - chroma_format_idc: (7.4.3.2.1) specifies the chroma sampling > relative to the luma sampling > - pic_parameter_set_id: (7.4.3.3.1) identifies the PPS for > reference by other syntax elements > - num_ref_idx_l0_default_active_minus1: (7.4.3.3.1) specifies > the inferred value of num_ref_idx_l0_active_minus1 > - num_ref_idx_l1_default_active_minus1: (7.4.3.3.1) specifies > the inferred value of num_ref_idx_l1_active_minus1 > - slice_segment_addr: (7.4.7.1) specifies the address of > the first coding tree block in the slice segment > - num_entry_point_offsets: (7.4.7.1) specifies the number of > entry_point_offset_minus1[ i ] syntax elements in the slice header > > Add HEVC decode params contains the information used in section > "8.3 Slice decoding process" of the specification to let the hardware > perform decoding of a slices. > > Adapt Cedrus driver according to these changes. > > Signed-off-by: Benjamin Gaignard > --- > version 3: > - Add documentation about the new structuers and fields. > > version 2: > - remove all change related to scaling > - squash commits to a coherent split > - be more verbose about the added fields > > .../media/v4l/ext-ctrls-codec.rst | 126 +++--- > .../media/v4l/vidioc-queryctrl.rst | 6 + > drivers/media/v4l2-core/v4l2-ctrls.c | 26 +++- > drivers/staging/media/sunxi/cedrus/cedrus.c | 6 + > drivers/staging/media/sunxi/cedrus/cedrus.h | 1 + > .../staging/media/sunxi/cedrus/cedrus_dec.c | 2 + > .../staging/media/sunxi/cedrus/cedrus_h265.c | 6 +- > include/media/hevc-ctrls.h | 45 +-- > 8 files changed, 186 insertions(+), 32 deletions(-) > > diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst > b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst > index 00944e97d638..5e6d77e858c0 100644 > --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst > +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst > @@ -3109,6 +3109,15 @@ enum v4l2_mpeg_video_hevc_size_of_length_field - > :stub-columns: 0 > :widths: 1 1 2 > > + * - __u8 > + - ``video_parameter_set_id`` > + - Identifies the VPS for reference by other syntax elements > + * - __u8 > + - ``seq_parameter_set_id̀`` > + - Specifies the value of the vps_video_parameter_set_id of the active > VPS > + * - __u8 > + - ``chroma_format_idc`` > + - Specifies the chroma sampling relative to the luma sampling None of these fields seem needed for the Hantro G2 driver, so I suggest you drop them for now. > * - __u16 > - ``pic_width_in_luma_samples`` > - > @@ -3172,6 +3181,9 @@ enum v4l2_mpeg_video_hevc_size_of_length_field - > * - __u8 > - ``chroma_format_idc`` > - > + * - __u8 > + - ``num_slices`` > + Not used, but also doesn't seem part of the SPS syntax. If we have to pass the number of slices, we'll need another mechanism. > - > * - __u64 > - ``flags`` > - See :ref:`Sequence Parameter Set Flags ` > @@ -3231,9 +3243,18 @@ 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 Not used. > * - __u8 > - ``num_extra_slice_header_bits`` > - > + * - __u8 > + - ``num_ref_idx_l0_default_active_minus1`` > + - Specifies the inferred value of num_ref_idx_l0_active_minus1 > + * - __u8 > + - ``num_ref_idx_l1_default_active_minus1`` > + - Specifies the inferred value of num_ref_idx_l1_active_minus1 > * - __s8 > - ``init_qp_minus26`` > - > @@ -3342,6 +3363,12 @@ enum v4l2_mpeg_video_hevc_size_of_length_field - > * - ``V4L2_HEVC_PPS_FLAG_SLICE_SEGMENT_HEADER_EXTENSION_PRESENT`` > - 0x0004 > - > + * - ``V4L2_HEVC_PPS_FLAG_DEBLOCKING_FILTER_CONTROL_PRESENT`` > + - 0x0008 > + - > + * - ``V4L2_HEVC_PPS_FLAG_UNIFORM_SPACING`` > + - 0x0010 > + - > I suggest to do all the PPS control changes in a separate patch, feels easier to review and cleaner as you can explain the changes with more detail in the commit description. Looking at the PPS syntax for tiles, I'm wondering if these deserve their own control, which would be used if tiles are enabled, i.e. V4L2_HEVC_PPS_FLAG_TILES_ENABLED is set. __u8num_tile_columns_minus1;
