Re: [PATCH 1/2] media: videobuf2: use dmabuf size for length
Hi Helen
>On 3/26/21 10:03 AM, John Cox wrote:
>> Hi Helen
>>
>>> Hi John,
>>>
>>> On 3/25/21 7:20 AM, John Cox wrote:
>>>> Hi
>>>>
>>>>> Always use dmabuf size when considering the length of the buffer.
>>>>> Discard userspace provided length.
>>>>> Fix length check error in _verify_length(), which was handling single and
>>>>> multiplanar diferently, and also not catching the case where userspace
>>>>> provides a bigger length and bytesused then the underlying buffer.
>>>>>
>>>>> Suggested-by: Hans Verkuil
>>>>> Signed-off-by: Helen Koike
>>>>> ---
>>>>>
>>>>> Hello,
>>>>>
>>>>> As discussed on
>>>>> https://patchwork.linuxtv.org/project/linux-media/patch/gh5kef5bkeel3o6b2dkgc2dfagu9klj...@4ax.com/
>>>>>
>>>>> This patch also helps the conversion layer of the Ext API patchset,
>>>>> where we are not exposing the length field.
>>>>>
>>>>> It was discussed that userspace might use a smaller length field to
>>>>> limit the usage of the underlying buffer, but I'm not sure if this is
>>>>> really usefull and just complicates things.
>>>>>
>>>>> If this is usefull, then we should also expose a length field in the Ext
>>>>> API, and document this feature properly.
>>>>>
>>>>> What do you think?
>>>>> ---
>>>>> .../media/common/videobuf2/videobuf2-core.c | 21 ---
>>>>> .../media/common/videobuf2/videobuf2-v4l2.c | 8 +++
>>>>> include/uapi/linux/videodev2.h| 7 +--
>>>>> 3 files changed, 27 insertions(+), 9 deletions(-)
>>>>>
>>>>> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c
>>>>> b/drivers/media/common/videobuf2/videobuf2-core.c
>>>>> index 02281d13505f..2cbde14af051 100644
>>>>> --- a/drivers/media/common/videobuf2/videobuf2-core.c
>>>>> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
>>>>> @@ -1205,6 +1205,7 @@ static int __prepare_dmabuf(struct vb2_buffer *vb)
>>>>>
>>>>> for (plane = 0; plane < vb->num_planes; ++plane) {
>>>>> struct dma_buf *dbuf = dma_buf_get(planes[plane].m.fd);
>>>>> + unsigned int bytesused;
>>>>>
>>>>> if (IS_ERR_OR_NULL(dbuf)) {
>>>>> dprintk(q, 1, "invalid dmabuf fd for plane %d\n",
>>>>> @@ -1213,9 +1214,23 @@ static int __prepare_dmabuf(struct vb2_buffer *vb)
>>>>> goto err;
>>>>> }
>>>>>
>>>>> - /* use DMABUF size if length is not provided */
>>>>> - if (planes[plane].length == 0)
>>>>> - planes[plane].length = dbuf->size;
>>>>> + planes[plane].length = dbuf->size;
>>>>> + bytesused = planes[plane].bytesused ?
>>>>> + planes[plane].bytesused : dbuf->size;
>>>>> +
>>>>> + if (planes[plane].bytesused > planes[plane].length) {
>>>>> + dprintk(q, 1, "bytesused is bigger then dmabuf length
>>>>> for plane %d\n",
>>>>> + plane);
>>>>> + ret = -EINVAL;
>>>>> + goto err;
>>>>> + }
>>>>> +
>>>>> + if (planes[plane].data_offset >= bytesused) {
>>>>> + dprintk(q, 1, "data_offset >= bytesused for plane %d\n",
>>>>> + plane);
>>>>> + ret = -EINVAL;
>>>>> + goto err;
>>>>> + }
>>>>>
>>>>> if (planes[plane].length < vb->planes[plane].min_length) {
>>>>> dprintk(q, 1, "invalid dmabuf length %u for plane %d,
>>>>> minimum length %u\n",
>>>>> diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c
>>>>> b/drivers/media/common/videobuf2/videobuf2-v4l2.c
>>>>> index 7e96f67c60ba..ffc7ed46f74a 100644
>>>>> --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
>>>>> +++ b/drivers/
Re: [PATCH 1/2] media: videobuf2: use dmabuf size for length
Hi Helen
>Hi John,
>
>On 3/25/21 7:20 AM, John Cox wrote:
>> Hi
>>
>>> Always use dmabuf size when considering the length of the buffer.
>>> Discard userspace provided length.
>>> Fix length check error in _verify_length(), which was handling single and
>>> multiplanar diferently, and also not catching the case where userspace
>>> provides a bigger length and bytesused then the underlying buffer.
>>>
>>> Suggested-by: Hans Verkuil
>>> Signed-off-by: Helen Koike
>>> ---
>>>
>>> Hello,
>>>
>>> As discussed on
>>> https://patchwork.linuxtv.org/project/linux-media/patch/gh5kef5bkeel3o6b2dkgc2dfagu9klj...@4ax.com/
>>>
>>> This patch also helps the conversion layer of the Ext API patchset,
>>> where we are not exposing the length field.
>>>
>>> It was discussed that userspace might use a smaller length field to
>>> limit the usage of the underlying buffer, but I'm not sure if this is
>>> really usefull and just complicates things.
>>>
>>> If this is usefull, then we should also expose a length field in the Ext
>>> API, and document this feature properly.
>>>
>>> What do you think?
>>> ---
>>> .../media/common/videobuf2/videobuf2-core.c | 21 ---
>>> .../media/common/videobuf2/videobuf2-v4l2.c | 8 +++
>>> include/uapi/linux/videodev2.h| 7 +--
>>> 3 files changed, 27 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c
>>> b/drivers/media/common/videobuf2/videobuf2-core.c
>>> index 02281d13505f..2cbde14af051 100644
>>> --- a/drivers/media/common/videobuf2/videobuf2-core.c
>>> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
>>> @@ -1205,6 +1205,7 @@ static int __prepare_dmabuf(struct vb2_buffer *vb)
>>>
>>> for (plane = 0; plane < vb->num_planes; ++plane) {
>>> struct dma_buf *dbuf = dma_buf_get(planes[plane].m.fd);
>>> + unsigned int bytesused;
>>>
>>> if (IS_ERR_OR_NULL(dbuf)) {
>>> dprintk(q, 1, "invalid dmabuf fd for plane %d\n",
>>> @@ -1213,9 +1214,23 @@ static int __prepare_dmabuf(struct vb2_buffer *vb)
>>> goto err;
>>> }
>>>
>>> - /* use DMABUF size if length is not provided */
>>> - if (planes[plane].length == 0)
>>> - planes[plane].length = dbuf->size;
>>> + planes[plane].length = dbuf->size;
>>> + bytesused = planes[plane].bytesused ?
>>> + planes[plane].bytesused : dbuf->size;
>>> +
>>> + if (planes[plane].bytesused > planes[plane].length) {
>>> + dprintk(q, 1, "bytesused is bigger then dmabuf length
>>> for plane %d\n",
>>> + plane);
>>> + ret = -EINVAL;
>>> + goto err;
>>> + }
>>> +
>>> + if (planes[plane].data_offset >= bytesused) {
>>> + dprintk(q, 1, "data_offset >= bytesused for plane %d\n",
>>> + plane);
>>> + ret = -EINVAL;
>>> + goto err;
>>> + }
>>>
>>> if (planes[plane].length < vb->planes[plane].min_length) {
>>> dprintk(q, 1, "invalid dmabuf length %u for plane %d,
>>> minimum length %u\n",
>>> diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c
>>> b/drivers/media/common/videobuf2/videobuf2-v4l2.c
>>> index 7e96f67c60ba..ffc7ed46f74a 100644
>>> --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
>>> +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
>>> @@ -98,14 +98,14 @@ static int __verify_length(struct vb2_buffer *vb, const
>>> struct v4l2_buffer *b)
>>> unsigned int bytesused;
>>> unsigned int plane;
>>>
>>> - if (V4L2_TYPE_IS_CAPTURE(b->type))
>>> + /* length check for dmabuf is performed in _prepare_dmabuf() */
>>> + if (V4L2_TYPE_IS_CAPTURE(b->type) || b->memory == VB2_MEMORY_DMABUF)
>>> return 0;
>>>
>>> if (V4L2_TYPE_IS_MULTIPLANAR(b->type)) {
>>> for (plane = 0; plane < vb->num_planes; ++plane) {
>>> -
Re: [PATCH 1/2] media: videobuf2: use dmabuf size for length
7 +968,9 @@ struct v4l2_requestbuffers {
> /**
> * struct v4l2_plane - plane info for multi-planar buffers
> * @bytesused:number of bytes occupied by data in the plane
> (payload)
>- * @length: size of this plane (NOT the payload) in bytes
>+ * @length: size of this plane (NOT the payload) in bytes. Filled
>+ *by userspace for USERPTR and by the driver for DMABUF
>+ *and MMAP.
> * @mem_offset: when memory in the associated struct
> v4l2_buffer is
> *V4L2_MEMORY_MMAP, equals the offset from the start of
> *the device memory for this plane (or is a "cookie" that
>@@ -1025,7 +1027,8 @@ struct v4l2_plane {
> * @m:union of @offset, @userptr, @planes and @fd
> * @length: size in bytes of the buffer (NOT its payload) for single-plane
> *buffers (when type != *_MPLANE); number of elements in the
>- *planes array for multi-plane buffers
>+ *planes array for multi-plane buffers. Filled by userspace for
>+ *USERPTR and by the driver for DMABUF and MMAP.
> * @reserved2:drivers and applications must zero this field
> * @request_fd: fd of the request that this buffer should use
> * @reserved: for backwards compatibility with applications that do not know
I think this does what I want. But I'm going to restate my usage desires
and check that you agree that it covers them.
I'm interested in passing compressed bitstreams to a decoder. The size
of these buffers can be very variable and the worst case will nearly
always be much larger than the typical case and that size cannot be
known in advance of usage. It can be very wasteful to have to allocate
buffers that are over an order of magnitude bigger than are likely to
ever be used. If you have a fixed pool of fixed size buffers allocated
at the start of time this wastefulness is unavoidable, but dmabufs can
be dynamically sized to be as big as required and so there should be no
limitation on passing in buffers that are smaller than the maximum. It
also seems plausible that dmabufs that are larger than the maximum
should be allowed as long as their bytesused is smaller or equal.
As an aside, even when using dynamically sized dmabufs they are often
way larger than the data they contain and forcing cache flushes or maps
of their entire length rather than just the used portion is also
wasteful. This might be a use for the incoming size field.
Regards
John Cox
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: [PATCH v2 1/9] media: hevc: Modify structures to follow H265 ITU spec
ice_cb_qp_offset)
> |
>
> VE_DEC_H265_DEC_SLICE_HDR_INFO1_SLICE_QP_DELTA(slice_params->slice_qp_delta);
>@@ -528,7 +530,7 @@ static void cedrus_h265_setup(struct cedrus_ctx *ctx,
>
> /* Write decoded picture buffer in pic list. */
> cedrus_h265_frame_info_write_dpb(ctx, slice_params->dpb,
>- slice_params->num_active_dpb_entries);
>+ decode_params->num_active_dpb_entries);
>
> /* Output frame. */
>
>diff --git a/include/media/hevc-ctrls.h b/include/media/hevc-ctrls.h
>index b4cb2ef02f17..7fe704a08f77 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_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)
>
>@@ -26,6 +27,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_DECODE_PARAMS 0x0124
>
> enum v4l2_mpeg_video_hevc_decode_mode {
> V4L2_MPEG_VIDEO_HEVC_DECODE_MODE_SLICE_BASED,
>@@ -54,6 +56,9 @@ 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 */
>+ __u8video_parameter_set_id;
Whilst I don't object to the addition of vps id why do we need
it if the VPS is never passed?
>+ __u8seq_parameter_set_id;
>+ __u8chroma_format_idc;
> __u16 pic_width_in_luma_samples;
> __u16 pic_height_in_luma_samples;
> __u8bit_depth_luma_minus8;
>@@ -74,9 +79,9 @@ struct v4l2_ctrl_hevc_sps {
> __u8log2_diff_max_min_pcm_luma_coding_block_size;
> __u8num_short_term_ref_pic_sets;
> __u8num_long_term_ref_pics_sps;
>- __u8chroma_format_idc;
>
>- __u8padding;
>+ __u8num_slices;
>+ __u8padding[6];
>
> __u64 flags;
> };
>@@ -100,10 +105,15 @@ struct v4l2_ctrl_hevc_sps {
> #define V4L2_HEVC_PPS_FLAG_PPS_DISABLE_DEBLOCKING_FILTER (1ULL << 16)
> #define V4L2_HEVC_PPS_FLAG_LISTS_MODIFICATION_PRESENT (1ULL << 17)
> #define V4L2_HEVC_PPS_FLAG_SLICE_SEGMENT_HEADER_EXTENSION_PRESENT (1ULL << 18)
>+#define V4L2_HEVC_PPS_FLAG_DEBLOCKING_FILTER_CONTROL_PRESENT (1ULL << 19)
>+#define V4L2_HEVC_PPS_FLAG_UNIFORM_SPACING(1ULL << 20)
>
> struct v4l2_ctrl_hevc_pps {
> /* ISO/IEC 23008-2, ITU-T Rec. H.265: Picture parameter set */
>+ __u8pic_parameter_set_id;
> __u8num_extra_slice_header_bits;
>+ __u8num_ref_idx_l0_default_active_minus1;
>+ __u8num_ref_idx_l1_default_active_minus1;
> __s8init_qp_minus26;
> __u8diff_cu_qp_delta_depth;
> __s8pps_cb_qp_offset;
>@@ -116,7 +126,7 @@ struct v4l2_ctrl_hevc_pps {
> __s8pps_tc_offset_div2;
> __u8log2_parallel_merge_level_minus2;
>
>- __u8padding[4];
>+ __u8padding;
> __u64 flags;
> };
>
>@@ -165,6 +175,10 @@ struct v4l2_ctrl_hevc_slice_params {
> __u32 bit_size;
> __u32 data_bit_offset;
>
>+ /* ISO/IEC 23008-2, ITU-T Rec. H.265: General slice segment header */
>+ __u32 slice_segment_addr;
>+ __u32 num_entry_point_offsets;
>+
> /* ISO/IEC 23008-2, ITU-T Rec. H.265: NAL unit header */
> __u8nal_unit_type;
> __u8nuh_temporal_id_plus1;
>@@ -190,15 +204,13 @@ struct v4l2_ctrl_hevc_slice_params {
> __u8pic_struct;
>
> /* ISO/IEC 23008-2, ITU-T Rec. H.265: General slice segment header */
>- __u8num_active_dpb_entries;
> __u8ref_idx_l0[V4L2_HEVC_DPB_ENTRIES_NUM_MAX];
> __u8ref_idx_l1[V4L2_HEVC_DPB_ENTRIES_NUM_MAX];
>
>- __u8num_rps_poc_st_curr_before;
>- __u8num_rps_poc_st_curr_after;
>- __u8num_rps_poc_lt_curr;
>+ __u16 short_term_ref_pic_set_size;
>+ __u16 long_term_ref_pic_set_size;
>
>- __u8padding;
>+ __u8padding[5];
>
> /* ISO/IEC 23008-2, ITU-T Rec. H.265: General slice segment header */
> struct v4l2_hevc_dpb_entry dpb[V4L2_HEVC_DPB_ENTRIES_NUM_MAX];
>@@ -209,4 +221,21 @@ struct v4l2_ctrl_hevc_slice_params {
> __u64 flags;
> };
>
>+#define V4L2_HEVC_DECODE_PARAM_FLAG_IRAP_PIC 0x1
>+#define V4L2_HEVC_DECODE_PARAM_FLAG_IDR_PIC 0x2
>+#define V4L2_HEVC_DECODE_PARAM_FLAG_NO_OUTPUT_OF_PRIOR 0x4
>+
>+struct v4l2_ctrl_hevc_decode_params {
>+ __s32 pic_order_cnt_val;
>+ __u8num_active_dpb_entries;
>+ struct v4l2_hevc_dpb_entry dpb[V4L2_HEVC_DPB_ENTRIES_NUM_MAX];
>+ __u8num_rps_poc_st_curr_before;
>+ __u8num_rps_poc_st_curr_after;
>+ __u8num_rps_poc_lt_curr;
>+ __u8rps_st_curr_before[V4L2_HEVC_DPB_ENTRIES_NUM_MAX];
>+ __u8rps_st_curr_after[V4L2_HEVC_DPB_ENTRIES_NUM_MAX];
>+ __u8rps_lt_curr[V4L2_HEVC_DPB_ENTRIES_NUM_MAX];
>+ __u64 flags;
>+};
>+
> #endif
While you are adding stuff is there any chance you could also add:
#define V4L2_HEVC_SLICE_PARAMS_FLAG_DEPENDENT_SLICE_SEGMENT (1ULL << 9)
to the slice flags? The rpi H265 decoder needs it to deal with
cases where dependant_slice_segment is set in the slice header.
Thanks
John Cox
Re: [PATCH v2 4/9] media: uapi: Add a control for HANTRO driver
>The HEVC HANTRO driver needs to know the number of bits to skip at
>the beginning of the slice header.
>That is a hardware specific requirement so create a dedicated control
>that this purpose.
>
>Signed-off-by: Benjamin Gaignard
>---
> include/uapi/linux/hantro-v4l2-controls.h | 20
> include/uapi/linux/v4l2-controls.h| 5 +
> 2 files changed, 25 insertions(+)
> create mode 100644 include/uapi/linux/hantro-v4l2-controls.h
>
>diff --git a/include/uapi/linux/hantro-v4l2-controls.h
>b/include/uapi/linux/hantro-v4l2-controls.h
>new file mode 100644
>index ..30b1999b7af3
>--- /dev/null
>+++ b/include/uapi/linux/hantro-v4l2-controls.h
>@@ -0,0 +1,20 @@
>+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
>+
>+#ifndef __UAPI_HANTRO_V4L2_CONYTROLS_H__
>+#define __UAPI_HANTRO_V4L2_CONYTROLS_H__
>+
>+#include
>+#include
>+
>+#define V4L2_CID_HANTRO_HEVC_EXTRA_DECODE_PARAMS
>(V4L2_CID_USER_HANTRO_BASE + 0)
>+
>+/**
>+ * struct hantro_hevc_extra_decode_params - extra decode parameters for
>hantro driver
>+ * @hevc_hdr_skip_lenght: header first bits offset
>+ */
>+struct hantro_hevc_extra_decode_params {
>+ __u32 hevc_hdr_skip_lenght;
>+ __u8padding[4];
>+};
Can you clarify how hevc_hdr_skip_length differs from
v4l2_ctrl_hevc_slice_params.data_bit_offset? At first sight they would
appear to be very similar.
Regards
John Cox
>+#endif
>diff --git a/include/uapi/linux/v4l2-controls.h
>b/include/uapi/linux/v4l2-controls.h
>index 039c0d7add1b..ced7486c7f46 100644
>--- a/include/uapi/linux/v4l2-controls.h
>+++ b/include/uapi/linux/v4l2-controls.h
>@@ -209,6 +209,11 @@ enum v4l2_colorfx {
> * We reserve 128 controls for this driver.
> */
> #define V4L2_CID_USER_CCS_BASE(V4L2_CID_USER_BASE +
> 0x10f0)
>+/*
>+ * The base for HANTRO driver controls.
>+ * We reserve 32 controls for this driver.
>+ */
>+#define V4L2_CID_USER_HANTRO_BASE (V4L2_CID_USER_BASE + 0x1170)
>
> /* MPEG-class control IDs */
> /* The MPEG controls are applicable to all codec controls
