| Message ID | 20180613140714.1686-2-maxime.ripard@bootlin.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 13/06/18 16:07, Maxime Ripard wrote: > From: Pawel Osciak <posciak@chromium.org> Obviously this needs a proper commit message. > > Signed-off-by: Pawel Osciak <posciak@chromium.org> > Reviewed-by: Wu-cheng Li <wuchengli@chromium.org> > Tested-by: Tomasz Figa <tfiga@chromium.org> > [rebase44(groeck): include linux/types.h in v4l2-controls.h] > Signed-off-by: Guenter Roeck <groeck@chromium.org> > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> > --- > drivers/media/v4l2-core/v4l2-ctrls.c | 42 +++++++ > include/media/v4l2-ctrls.h | 10 ++ > include/uapi/linux/v4l2-controls.h | 164 +++++++++++++++++++++++++++ > include/uapi/linux/videodev2.h | 11 ++ > 4 files changed, 227 insertions(+) > > diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c > index cdf860c8e3d8..1f63c725bad1 100644 > --- a/drivers/media/v4l2-core/v4l2-ctrls.c > +++ b/drivers/media/v4l2-core/v4l2-ctrls.c > @@ -807,6 +807,11 @@ const char *v4l2_ctrl_get_name(u32 id) > case V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING_LAYER:return "H264 Number of HC Layers"; > case V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING_LAYER_QP: > return "H264 Set QP Value for HC Layers"; > + case V4L2_CID_MPEG_VIDEO_H264_SPS: return "H264 SPS"; > + case V4L2_CID_MPEG_VIDEO_H264_PPS: return "H264 PPS"; > + case V4L2_CID_MPEG_VIDEO_H264_SCALING_MATRIX: return "H264 Scaling Matrix"; > + case V4L2_CID_MPEG_VIDEO_H264_SLICE_PARAM: return "H264 Slice Parameters"; > + case V4L2_CID_MPEG_VIDEO_H264_DECODE_PARAM: return "H264 Decode Parameters"; > case V4L2_CID_MPEG_VIDEO_MPEG4_I_FRAME_QP: return "MPEG4 I-Frame QP Value"; > case V4L2_CID_MPEG_VIDEO_MPEG4_P_FRAME_QP: return "MPEG4 P-Frame QP Value"; > case V4L2_CID_MPEG_VIDEO_MPEG4_B_FRAME_QP: return "MPEG4 B-Frame QP Value"; > @@ -1272,6 +1277,21 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type, > case V4L2_CID_RDS_TX_ALT_FREQS: > *type = V4L2_CTRL_TYPE_U32; > break; > + case V4L2_CID_MPEG_VIDEO_H264_SPS: > + *type = V4L2_CTRL_TYPE_H264_SPS; > + break; > + case V4L2_CID_MPEG_VIDEO_H264_PPS: > + *type = V4L2_CTRL_TYPE_H264_PPS; > + break; > + case V4L2_CID_MPEG_VIDEO_H264_SCALING_MATRIX: > + *type = V4L2_CTRL_TYPE_H264_SCALING_MATRIX; > + break; > + case V4L2_CID_MPEG_VIDEO_H264_SLICE_PARAM: > + *type = V4L2_CTRL_TYPE_H264_SLICE_PARAM; > + break; > + case V4L2_CID_MPEG_VIDEO_H264_DECODE_PARAM: > + *type = V4L2_CTRL_TYPE_H264_DECODE_PARAM; > + break; > case V4L2_CID_MPEG_VIDEO_MPEG2_FRAME_HDR: > *type = V4L2_CTRL_TYPE_MPEG2_FRAME_HDR; > break; > @@ -1598,6 +1618,13 @@ static int std_validate(const struct v4l2_ctrl *ctrl, u32 idx, > case V4L2_CTRL_TYPE_MPEG2_FRAME_HDR: > return 0; > > + case V4L2_CTRL_TYPE_H264_SPS: > + case V4L2_CTRL_TYPE_H264_PPS: > + case V4L2_CTRL_TYPE_H264_SCALING_MATRIX: > + case V4L2_CTRL_TYPE_H264_SLICE_PARAM: > + case V4L2_CTRL_TYPE_H264_DECODE_PARAM: > + return 0; > + > default: > return -EINVAL; > } > @@ -2172,6 +2199,21 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl, > case V4L2_CTRL_TYPE_U32: > elem_size = sizeof(u32); > break; > + case V4L2_CTRL_TYPE_H264_SPS: > + elem_size = sizeof(struct v4l2_ctrl_h264_sps); > + break; > + case V4L2_CTRL_TYPE_H264_PPS: > + elem_size = sizeof(struct v4l2_ctrl_h264_pps); > + break; > + case V4L2_CTRL_TYPE_H264_SCALING_MATRIX: > + elem_size = sizeof(struct v4l2_ctrl_h264_scaling_matrix); > + break; > + case V4L2_CTRL_TYPE_H264_SLICE_PARAM: > + elem_size = sizeof(struct v4l2_ctrl_h264_slice_param); > + break; > + case V4L2_CTRL_TYPE_H264_DECODE_PARAM: > + elem_size = sizeof(struct v4l2_ctrl_h264_decode_param); > + break; > case V4L2_CTRL_TYPE_MPEG2_FRAME_HDR: > elem_size = sizeof(struct v4l2_ctrl_mpeg2_frame_hdr); > break; > diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h > index 963c37b02363..9c223793a16a 100644 > --- a/include/media/v4l2-ctrls.h > +++ b/include/media/v4l2-ctrls.h > @@ -41,6 +41,11 @@ struct poll_table_struct; > * @p_u16: Pointer to a 16-bit unsigned value. > * @p_u32: Pointer to a 32-bit unsigned value. > * @p_char: Pointer to a string. > + * @p_h264_sps: Pointer to a struct v4l2_ctrl_h264_sps. > + * @p_h264_pps: Pointer to a struct v4l2_ctrl_h264_pps. > + * @p_h264_scal_mtrx: Pointer to a struct v4l2_ctrl_h264_scaling_matrix. > + * @p_h264_slice_param: Pointer to a struct v4l2_ctrl_h264_slice_param. > + * @p_h264_decode_param: Pointer to a struct v4l2_ctrl_h264_decode_param. > * @p: Pointer to a compound value. > */ > union v4l2_ctrl_ptr { > @@ -50,6 +55,11 @@ union v4l2_ctrl_ptr { > u16 *p_u16; > u32 *p_u32; > char *p_char; > + struct v4l2_ctrl_h264_sps *p_h264_sps; > + struct v4l2_ctrl_h264_pps *p_h264_pps; > + struct v4l2_ctrl_h264_scaling_matrix *p_h264_scal_mtrx; > + struct v4l2_ctrl_h264_slice_param *p_h264_slice_param; > + struct v4l2_ctrl_h264_decode_param *p_h264_decode_param; > void *p; > }; > > diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h > index 23da8bfa7e6f..ac307c59683c 100644 > --- a/include/uapi/linux/v4l2-controls.h > +++ b/include/uapi/linux/v4l2-controls.h > @@ -50,6 +50,8 @@ > #ifndef __LINUX_V4L2_CONTROLS_H > #define __LINUX_V4L2_CONTROLS_H > > +#include <linux/types.h> > + > /* Control classes */ > #define V4L2_CTRL_CLASS_USER 0x00980000 /* Old-style 'user' controls */ > #define V4L2_CTRL_CLASS_MPEG 0x00990000 /* MPEG-compression controls */ > @@ -531,6 +533,12 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type { > }; > #define V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING_LAYER (V4L2_CID_MPEG_BASE+381) > #define V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING_LAYER_QP (V4L2_CID_MPEG_BASE+382) > +#define V4L2_CID_MPEG_VIDEO_H264_SPS (V4L2_CID_MPEG_BASE+383) > +#define V4L2_CID_MPEG_VIDEO_H264_PPS (V4L2_CID_MPEG_BASE+384) > +#define V4L2_CID_MPEG_VIDEO_H264_SCALING_MATRIX (V4L2_CID_MPEG_BASE+385) > +#define V4L2_CID_MPEG_VIDEO_H264_SLICE_PARAM (V4L2_CID_MPEG_BASE+386) > +#define V4L2_CID_MPEG_VIDEO_H264_DECODE_PARAM (V4L2_CID_MPEG_BASE+387) > + > #define V4L2_CID_MPEG_VIDEO_MPEG4_I_FRAME_QP (V4L2_CID_MPEG_BASE+400) > #define V4L2_CID_MPEG_VIDEO_MPEG4_P_FRAME_QP (V4L2_CID_MPEG_BASE+401) > #define V4L2_CID_MPEG_VIDEO_MPEG4_B_FRAME_QP (V4L2_CID_MPEG_BASE+402) > @@ -1078,6 +1086,162 @@ enum v4l2_detect_md_mode { > #define V4L2_CID_DETECT_MD_THRESHOLD_GRID (V4L2_CID_DETECT_CLASS_BASE + 3) > #define V4L2_CID_DETECT_MD_REGION_GRID (V4L2_CID_DETECT_CLASS_BASE + 4) > > +/* Complex controls */ The right term is 'Compounds controls'. > + > +#define V4L2_H264_SPS_CONSTRAINT_SET0_FLAG 0x01 > +#define V4L2_H264_SPS_CONSTRAINT_SET1_FLAG 0x02 > +#define V4L2_H264_SPS_CONSTRAINT_SET2_FLAG 0x04 > +#define V4L2_H264_SPS_CONSTRAINT_SET3_FLAG 0x08 > +#define V4L2_H264_SPS_CONSTRAINT_SET4_FLAG 0x10 > +#define V4L2_H264_SPS_CONSTRAINT_SET5_FLAG 0x20 > + > +#define V4L2_H264_SPS_FLAG_SEPARATE_COLOUR_PLANE 0x01 > +#define V4L2_H264_SPS_FLAG_QPPRIME_Y_ZERO_TRANSFORM_BYPASS 0x02 > +#define V4L2_H264_SPS_FLAG_DELTA_PIC_ORDER_ALWAYS_ZERO 0x04 > +#define V4L2_H264_SPS_FLAG_GAPS_IN_FRAME_NUM_VALUE_ALLOWED 0x08 > +#define V4L2_H264_SPS_FLAG_FRAME_MBS_ONLY 0x10 > +#define V4L2_H264_SPS_FLAG_MB_ADAPTIVE_FRAME_FIELD 0x20 > +#define V4L2_H264_SPS_FLAG_DIRECT_8X8_INFERENCE 0x40 Add a newline here. Same for the other structs below. > +struct v4l2_ctrl_h264_sps { > + __u8 profile_idc; > + __u8 constraint_set_flags; > + __u8 level_idc; > + __u8 seq_parameter_set_id; > + __u8 chroma_format_idc; > + __u8 bit_depth_luma_minus8; > + __u8 bit_depth_chroma_minus8; > + __u8 log2_max_frame_num_minus4; > + __u8 pic_order_cnt_type; > + __u8 log2_max_pic_order_cnt_lsb_minus4; There is a hole in the struct here. Is that OK? Are there alignment requirements? > + __s32 offset_for_non_ref_pic; > + __s32 offset_for_top_to_bottom_field; > + __u8 num_ref_frames_in_pic_order_cnt_cycle; > + __s32 offset_for_ref_frame[255]; Perhaps use a define instead of hardcoding 255? Not sure if that makes sense. Same for other arrays below. > + __u8 max_num_ref_frames; > + __u16 pic_width_in_mbs_minus1; > + __u16 pic_height_in_map_units_minus1; > + __u8 flags; > +}; You have to test the struct layout for 32 bit and 64 bit systems (the latter for both 64 bit arm and Intel). The layout should be the same for all of them since the control framework does not support compat32 conversions for compound controls. > + > +#define V4L2_H264_PPS_FLAG_ENTROPY_CODING_MODE 0x0001 > +#define V4L2_H264_PPS_FLAG_BOTTOM_FIELD_PIC_ORDER_IN_FRAME_PRESENT 0x0002 > +#define V4L2_H264_PPS_FLAG_WEIGHTED_PRED 0x0004 > +#define V4L2_H264_PPS_FLAG_DEBLOCKING_FILTER_CONTROL_PRESENT 0x0008 > +#define V4L2_H264_PPS_FLAG_CONSTRAINED_INTRA_PRED 0x0010 > +#define V4L2_H264_PPS_FLAG_REDUNDANT_PIC_CNT_PRESENT 0x0020 > +#define V4L2_H264_PPS_FLAG_TRANSFORM_8X8_MODE 0x0040 > +#define V4L2_H264_PPS_FLAG_PIC_SCALING_MATRIX_PRESENT 0x0080 > +struct v4l2_ctrl_h264_pps { > + __u8 pic_parameter_set_id; > + __u8 seq_parameter_set_id; > + __u8 num_slice_groups_minus1; > + __u8 num_ref_idx_l0_default_active_minus1; > + __u8 num_ref_idx_l1_default_active_minus1; > + __u8 weighted_bipred_idc; > + __s8 pic_init_qp_minus26; > + __s8 pic_init_qs_minus26; > + __s8 chroma_qp_index_offset; > + __s8 second_chroma_qp_index_offset; > + __u8 flags; > +}; > + > +struct v4l2_ctrl_h264_scaling_matrix { > + __u8 scaling_list_4x4[6][16]; > + __u8 scaling_list_8x8[6][64]; > +}; > + > +struct v4l2_h264_weight_factors { > + __s8 luma_weight[32]; > + __s8 luma_offset[32]; > + __s8 chroma_weight[32][2]; > + __s8 chroma_offset[32][2]; > +}; > + > +struct v4l2_h264_pred_weight_table { > + __u8 luma_log2_weight_denom; > + __u8 chroma_log2_weight_denom; > + struct v4l2_h264_weight_factors weight_factors[2]; > +}; > + > +enum v4l2_h264_slice_type { > + V4L2_H264_SLICE_TYPE_P = 0, > + V4L2_H264_SLICE_TYPE_B = 1, > + V4L2_H264_SLICE_TYPE_I = 2, > + V4L2_H264_SLICE_TYPE_SP = 3, > + V4L2_H264_SLICE_TYPE_SI = 4, > +}; > + > +#define V4L2_SLICE_FLAG_FIELD_PIC 0x01 > +#define V4L2_SLICE_FLAG_BOTTOM_FIELD 0x02 > +#define V4L2_SLICE_FLAG_DIRECT_SPATIAL_MV_PRED 0x04 > +#define V4L2_SLICE_FLAG_SP_FOR_SWITCH 0x08 > +struct v4l2_ctrl_h264_slice_param { > + /* Size in bytes, including header */ > + __u32 size; > + /* Offset in bits to slice_data() from the beginning of this slice. */ > + __u32 header_bit_size; > + > + __u16 first_mb_in_slice; > + enum v4l2_h264_slice_type slice_type; Avoid enums in a struct. > + __u8 pic_parameter_set_id; > + __u8 colour_plane_id; > + __u16 frame_num; > + __u16 idr_pic_id; > + __u16 pic_order_cnt_lsb; > + __s32 delta_pic_order_cnt_bottom; > + __s32 delta_pic_order_cnt0; > + __s32 delta_pic_order_cnt1; > + __u8 redundant_pic_cnt; > + > + struct v4l2_h264_pred_weight_table pred_weight_table; > + /* Size in bits of dec_ref_pic_marking() syntax element. */ > + __u32 dec_ref_pic_marking_bit_size; > + /* Size in bits of pic order count syntax. */ > + __u32 pic_order_cnt_bit_size; > + > + __u8 cabac_init_idc; > + __s8 slice_qp_delta; > + __s8 slice_qs_delta; > + __u8 disable_deblocking_filter_idc; > + __s8 slice_alpha_c0_offset_div2; > + __s8 slice_beta_offset_div2; > + __u32 slice_group_change_cycle; > + > + __u8 num_ref_idx_l0_active_minus1; > + __u8 num_ref_idx_l1_active_minus1; > + /* Entries on each list are indices > + * into v4l2_ctrl_h264_decode_param.dpb[]. */ > + __u8 ref_pic_list0[32]; > + __u8 ref_pic_list1[32]; > + > + __u8 flags; > +}; > + > +/* If not set, this entry is unused for reference. */ > +#define V4L2_H264_DPB_ENTRY_FLAG_ACTIVE 0x01 > +#define V4L2_H264_DPB_ENTRY_FLAG_LONG_TERM 0x02 > +struct v4l2_h264_dpb_entry { > + __u32 buf_index; /* v4l2_buffer index */ > + __u16 frame_num; > + __u16 pic_num; > + /* Note that field is indicated by v4l2_buffer.field */ > + __s32 top_field_order_cnt; > + __s32 bottom_field_order_cnt; > + __u8 flags; /* V4L2_H264_DPB_ENTRY_FLAG_* */ > +}; > + > +struct v4l2_ctrl_h264_decode_param { > + __u32 num_slices; > + __u8 idr_pic_flag; > + __u8 nal_ref_idc; > + __s32 top_field_order_cnt; > + __s32 bottom_field_order_cnt; > + __u8 ref_pic_list_p0[32]; > + __u8 ref_pic_list_b0[32]; > + __u8 ref_pic_list_b1[32]; > + struct v4l2_h264_dpb_entry dpb[16]; > +}; > + > struct v4l2_ctrl_mpeg2_frame_hdr { > __u32 slice_len; > __u32 slice_pos; > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > index 242a6bfa1440..4b4a1b25a0db 100644 > --- a/include/uapi/linux/videodev2.h > +++ b/include/uapi/linux/videodev2.h > @@ -626,6 +626,7 @@ struct v4l2_pix_format { > #define V4L2_PIX_FMT_H264 v4l2_fourcc('H', '2', '6', '4') /* H264 with start codes */ > #define V4L2_PIX_FMT_H264_NO_SC v4l2_fourcc('A', 'V', 'C', '1') /* H264 without start codes */ > #define V4L2_PIX_FMT_H264_MVC v4l2_fourcc('M', '2', '6', '4') /* H264 MVC */ > +#define V4L2_PIX_FMT_H264_SLICE v4l2_fourcc('S', '2', '6', '4') /* H264 parsed slices */ > #define V4L2_PIX_FMT_H263 v4l2_fourcc('H', '2', '6', '3') /* H263 */ > #define V4L2_PIX_FMT_MPEG1 v4l2_fourcc('M', 'P', 'G', '1') /* MPEG-1 ES */ > #define V4L2_PIX_FMT_MPEG2 v4l2_fourcc('M', 'P', 'G', '2') /* MPEG-2 ES */ > @@ -1589,6 +1590,11 @@ struct v4l2_ext_control { > __u8 __user *p_u8; > __u16 __user *p_u16; > __u32 __user *p_u32; > + struct v4l2_ctrl_h264_sps __user *p_h264_sps; > + struct v4l2_ctrl_h264_pps __user *p_h264_pps; > + struct v4l2_ctrl_h264_scaling_matrix __user *p_h264_scal_mtrx; > + struct v4l2_ctrl_h264_slice_param __user *p_h264_slice_param; > + struct v4l2_ctrl_h264_decode_param __user *p_h264_decode_param; > struct v4l2_ctrl_mpeg2_frame_hdr __user *p_mpeg2_frame_hdr; > void __user *ptr; > }; > @@ -1635,6 +1641,11 @@ enum v4l2_ctrl_type { > V4L2_CTRL_TYPE_U8 = 0x0100, > V4L2_CTRL_TYPE_U16 = 0x0101, > V4L2_CTRL_TYPE_U32 = 0x0102, > + V4L2_CTRL_TYPE_H264_SPS = 0x0103, > + V4L2_CTRL_TYPE_H264_PPS = 0x0104, > + V4L2_CTRL_TYPE_H264_SCALING_MATRIX = 0x0105, > + V4L2_CTRL_TYPE_H264_SLICE_PARAM = 0x0106, > + V4L2_CTRL_TYPE_H264_DECODE_PARAM = 0x0107, > V4L2_CTRL_TYPE_MPEG2_FRAME_HDR = 0x0109, > }; > > Documentation is also missing, but I assume that will be added later in a separate patch. I *think* that the alignments of the structs are the same for all architectures, but I would feel much happier if you can confirm that by testing this. Regards, Hans
On Fri, Jun 15, 2018 at 5:00 AM Hans Verkuil <hverkuil@xs4all.nl> wrote: > > On 13/06/18 16:07, Maxime Ripard wrote: > > From: Pawel Osciak <posciak@chromium.org> > > Obviously this needs a proper commit message. > > > > > Signed-off-by: Pawel Osciak <posciak@chromium.org> > > Reviewed-by: Wu-cheng Li <wuchengli@chromium.org> > > Tested-by: Tomasz Figa <tfiga@chromium.org> > > [rebase44(groeck): include linux/types.h in v4l2-controls.h] That internal note should go away. Guenter
Hi, On Wed, 2018-06-13 at 16:07 +0200, Maxime Ripard wrote: > From: Pawel Osciak <posciak@chromium.org> > > Signed-off-by: Pawel Osciak <posciak@chromium.org> > Reviewed-by: Wu-cheng Li <wuchengli@chromium.org> > Tested-by: Tomasz Figa <tfiga@chromium.org> > [rebase44(groeck): include linux/types.h in v4l2-controls.h] > Signed-off-by: Guenter Roeck <groeck@chromium.org> > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> > --- > drivers/media/v4l2-core/v4l2-ctrls.c | 42 +++++++ > include/media/v4l2-ctrls.h | 10 ++ > include/uapi/linux/v4l2-controls.h | 164 +++++++++++++++++++++++++++ > include/uapi/linux/videodev2.h | 11 ++ > 4 files changed, 227 insertions(+) > > diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c > index cdf860c8e3d8..1f63c725bad1 100644 > --- a/drivers/media/v4l2-core/v4l2-ctrls.c > +++ b/drivers/media/v4l2-core/v4l2-ctrls.c > @@ -807,6 +807,11 @@ const char *v4l2_ctrl_get_name(u32 id) > case V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING_LAYER:return "H264 Number of HC Layers"; > case V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING_LAYER_QP: > return "H264 Set QP Value for HC Layers"; > + case V4L2_CID_MPEG_VIDEO_H264_SPS: return "H264 SPS"; > + case V4L2_CID_MPEG_VIDEO_H264_PPS: return "H264 PPS"; > + case V4L2_CID_MPEG_VIDEO_H264_SCALING_MATRIX: return "H264 Scaling Matrix"; > + case V4L2_CID_MPEG_VIDEO_H264_SLICE_PARAM: return "H264 Slice Parameters"; The MPEG2 fashion of this control is called V4L2_CID_MPEG_VIDEO_MPEG2_SLICE_HEADER and it would be good to have consistency between both. I think having a _SLICE_PARAMS suffix for both (instead of _SLICE_HEADER for MPEG2 and _SLICE_PARAM for H264) would make the most sense. > + case V4L2_CID_MPEG_VIDEO_H264_DECODE_PARAM: return "H264 Decode Parameters"; Same here, I think having a _DECODE_PARAMS suffix instead would be best. Cheers, Paul > case V4L2_CID_MPEG_VIDEO_MPEG4_I_FRAME_QP: return "MPEG4 I-Frame QP Value"; > case V4L2_CID_MPEG_VIDEO_MPEG4_P_FRAME_QP: return "MPEG4 P-Frame QP Value"; > case V4L2_CID_MPEG_VIDEO_MPEG4_B_FRAME_QP: return "MPEG4 B-Frame QP Value"; > @@ -1272,6 +1277,21 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type, > case V4L2_CID_RDS_TX_ALT_FREQS: > *type = V4L2_CTRL_TYPE_U32; > break; > + case V4L2_CID_MPEG_VIDEO_H264_SPS: > + *type = V4L2_CTRL_TYPE_H264_SPS; > + break; > + case V4L2_CID_MPEG_VIDEO_H264_PPS: > + *type = V4L2_CTRL_TYPE_H264_PPS; > + break; > + case V4L2_CID_MPEG_VIDEO_H264_SCALING_MATRIX: > + *type = V4L2_CTRL_TYPE_H264_SCALING_MATRIX; > + break; > + case V4L2_CID_MPEG_VIDEO_H264_SLICE_PARAM: > + *type = V4L2_CTRL_TYPE_H264_SLICE_PARAM; > + break; > + case V4L2_CID_MPEG_VIDEO_H264_DECODE_PARAM: > + *type = V4L2_CTRL_TYPE_H264_DECODE_PARAM; > + break; > case V4L2_CID_MPEG_VIDEO_MPEG2_FRAME_HDR: > *type = V4L2_CTRL_TYPE_MPEG2_FRAME_HDR; > break; > @@ -1598,6 +1618,13 @@ static int std_validate(const struct v4l2_ctrl *ctrl, u32 idx, > case V4L2_CTRL_TYPE_MPEG2_FRAME_HDR: > return 0; > > + case V4L2_CTRL_TYPE_H264_SPS: > + case V4L2_CTRL_TYPE_H264_PPS: > + case V4L2_CTRL_TYPE_H264_SCALING_MATRIX: > + case V4L2_CTRL_TYPE_H264_SLICE_PARAM: > + case V4L2_CTRL_TYPE_H264_DECODE_PARAM: > + return 0; > + > default: > return -EINVAL; > } > @@ -2172,6 +2199,21 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl, > case V4L2_CTRL_TYPE_U32: > elem_size = sizeof(u32); > break; > + case V4L2_CTRL_TYPE_H264_SPS: > + elem_size = sizeof(struct v4l2_ctrl_h264_sps); > + break; > + case V4L2_CTRL_TYPE_H264_PPS: > + elem_size = sizeof(struct v4l2_ctrl_h264_pps); > + break; > + case V4L2_CTRL_TYPE_H264_SCALING_MATRIX: > + elem_size = sizeof(struct v4l2_ctrl_h264_scaling_matrix); > + break; > + case V4L2_CTRL_TYPE_H264_SLICE_PARAM: > + elem_size = sizeof(struct v4l2_ctrl_h264_slice_param); > + break; > + case V4L2_CTRL_TYPE_H264_DECODE_PARAM: > + elem_size = sizeof(struct v4l2_ctrl_h264_decode_param); > + break; > case V4L2_CTRL_TYPE_MPEG2_FRAME_HDR: > elem_size = sizeof(struct v4l2_ctrl_mpeg2_frame_hdr); > break; > diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h > index 963c37b02363..9c223793a16a 100644 > --- a/include/media/v4l2-ctrls.h > +++ b/include/media/v4l2-ctrls.h > @@ -41,6 +41,11 @@ struct poll_table_struct; > * @p_u16: Pointer to a 16-bit unsigned value. > * @p_u32: Pointer to a 32-bit unsigned value. > * @p_char: Pointer to a string. > + * @p_h264_sps: Pointer to a struct v4l2_ctrl_h264_sps. > + * @p_h264_pps: Pointer to a struct v4l2_ctrl_h264_pps. > + * @p_h264_scal_mtrx: Pointer to a struct v4l2_ctrl_h264_scaling_matrix. > + * @p_h264_slice_param: Pointer to a struct v4l2_ctrl_h264_slice_param. > + * @p_h264_decode_param: Pointer to a struct v4l2_ctrl_h264_decode_param. > * @p: Pointer to a compound value. > */ > union v4l2_ctrl_ptr { > @@ -50,6 +55,11 @@ union v4l2_ctrl_ptr { > u16 *p_u16; > u32 *p_u32; > char *p_char; > + struct v4l2_ctrl_h264_sps *p_h264_sps; > + struct v4l2_ctrl_h264_pps *p_h264_pps; > + struct v4l2_ctrl_h264_scaling_matrix *p_h264_scal_mtrx; > + struct v4l2_ctrl_h264_slice_param *p_h264_slice_param; > + struct v4l2_ctrl_h264_decode_param *p_h264_decode_param; > void *p; > }; > > diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h > index 23da8bfa7e6f..ac307c59683c 100644 > --- a/include/uapi/linux/v4l2-controls.h > +++ b/include/uapi/linux/v4l2-controls.h > @@ -50,6 +50,8 @@ > #ifndef __LINUX_V4L2_CONTROLS_H > #define __LINUX_V4L2_CONTROLS_H > > +#include <linux/types.h> > + > /* Control classes */ > #define V4L2_CTRL_CLASS_USER 0x00980000 /* Old-style 'user' controls */ > #define V4L2_CTRL_CLASS_MPEG 0x00990000 /* MPEG-compression controls */ > @@ -531,6 +533,12 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type { > }; > #define V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING_LAYER (V4L2_CID_MPEG_BASE+381) > #define V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING_LAYER_QP (V4L2_CID_MPEG_BASE+382) > +#define V4L2_CID_MPEG_VIDEO_H264_SPS (V4L2_CID_MPEG_BASE+383) > +#define V4L2_CID_MPEG_VIDEO_H264_PPS (V4L2_CID_MPEG_BASE+384) > +#define V4L2_CID_MPEG_VIDEO_H264_SCALING_MATRIX (V4L2_CID_MPEG_BASE+385) > +#define V4L2_CID_MPEG_VIDEO_H264_SLICE_PARAM (V4L2_CID_MPEG_BASE+386) > +#define V4L2_CID_MPEG_VIDEO_H264_DECODE_PARAM (V4L2_CID_MPEG_BASE+387) > + > #define V4L2_CID_MPEG_VIDEO_MPEG4_I_FRAME_QP (V4L2_CID_MPEG_BASE+400) > #define V4L2_CID_MPEG_VIDEO_MPEG4_P_FRAME_QP (V4L2_CID_MPEG_BASE+401) > #define V4L2_CID_MPEG_VIDEO_MPEG4_B_FRAME_QP (V4L2_CID_MPEG_BASE+402) > @@ -1078,6 +1086,162 @@ enum v4l2_detect_md_mode { > #define V4L2_CID_DETECT_MD_THRESHOLD_GRID (V4L2_CID_DETECT_CLASS_BASE + 3) > #define V4L2_CID_DETECT_MD_REGION_GRID (V4L2_CID_DETECT_CLASS_BASE + 4) > > +/* Complex controls */ > + > +#define V4L2_H264_SPS_CONSTRAINT_SET0_FLAG 0x01 > +#define V4L2_H264_SPS_CONSTRAINT_SET1_FLAG 0x02 > +#define V4L2_H264_SPS_CONSTRAINT_SET2_FLAG 0x04 > +#define V4L2_H264_SPS_CONSTRAINT_SET3_FLAG 0x08 > +#define V4L2_H264_SPS_CONSTRAINT_SET4_FLAG 0x10 > +#define V4L2_H264_SPS_CONSTRAINT_SET5_FLAG 0x20 > + > +#define V4L2_H264_SPS_FLAG_SEPARATE_COLOUR_PLANE 0x01 > +#define V4L2_H264_SPS_FLAG_QPPRIME_Y_ZERO_TRANSFORM_BYPASS 0x02 > +#define V4L2_H264_SPS_FLAG_DELTA_PIC_ORDER_ALWAYS_ZERO 0x04 > +#define V4L2_H264_SPS_FLAG_GAPS_IN_FRAME_NUM_VALUE_ALLOWED 0x08 > +#define V4L2_H264_SPS_FLAG_FRAME_MBS_ONLY 0x10 > +#define V4L2_H264_SPS_FLAG_MB_ADAPTIVE_FRAME_FIELD 0x20 > +#define V4L2_H264_SPS_FLAG_DIRECT_8X8_INFERENCE 0x40 > +struct v4l2_ctrl_h264_sps { > + __u8 profile_idc; > + __u8 constraint_set_flags; > + __u8 level_idc; > + __u8 seq_parameter_set_id; > + __u8 chroma_format_idc; > + __u8 bit_depth_luma_minus8; > + __u8 bit_depth_chroma_minus8; > + __u8 log2_max_frame_num_minus4; > + __u8 pic_order_cnt_type; > + __u8 log2_max_pic_order_cnt_lsb_minus4; > + __s32 offset_for_non_ref_pic; > + __s32 offset_for_top_to_bottom_field; > + __u8 num_ref_frames_in_pic_order_cnt_cycle; > + __s32 offset_for_ref_frame[255]; > + __u8 max_num_ref_frames; > + __u16 pic_width_in_mbs_minus1; > + __u16 pic_height_in_map_units_minus1; > + __u8 flags; > +}; > + > +#define V4L2_H264_PPS_FLAG_ENTROPY_CODING_MODE 0x0001 > +#define V4L2_H264_PPS_FLAG_BOTTOM_FIELD_PIC_ORDER_IN_FRAME_PRESENT 0x0002 > +#define V4L2_H264_PPS_FLAG_WEIGHTED_PRED 0x0004 > +#define V4L2_H264_PPS_FLAG_DEBLOCKING_FILTER_CONTROL_PRESENT 0x0008 > +#define V4L2_H264_PPS_FLAG_CONSTRAINED_INTRA_PRED 0x0010 > +#define V4L2_H264_PPS_FLAG_REDUNDANT_PIC_CNT_PRESENT 0x0020 > +#define V4L2_H264_PPS_FLAG_TRANSFORM_8X8_MODE 0x0040 > +#define V4L2_H264_PPS_FLAG_PIC_SCALING_MATRIX_PRESENT 0x0080 > +struct v4l2_ctrl_h264_pps { > + __u8 pic_parameter_set_id; > + __u8 seq_parameter_set_id; > + __u8 num_slice_groups_minus1; > + __u8 num_ref_idx_l0_default_active_minus1; > + __u8 num_ref_idx_l1_default_active_minus1; > + __u8 weighted_bipred_idc; > + __s8 pic_init_qp_minus26; > + __s8 pic_init_qs_minus26; > + __s8 chroma_qp_index_offset; > + __s8 second_chroma_qp_index_offset; > + __u8 flags; > +}; > + > +struct v4l2_ctrl_h264_scaling_matrix { > + __u8 scaling_list_4x4[6][16]; > + __u8 scaling_list_8x8[6][64]; > +}; > + > +struct v4l2_h264_weight_factors { > + __s8 luma_weight[32]; > + __s8 luma_offset[32]; > + __s8 chroma_weight[32][2]; > + __s8 chroma_offset[32][2]; > +}; > + > +struct v4l2_h264_pred_weight_table { > + __u8 luma_log2_weight_denom; > + __u8 chroma_log2_weight_denom; > + struct v4l2_h264_weight_factors weight_factors[2]; > +}; > + > +enum v4l2_h264_slice_type { > + V4L2_H264_SLICE_TYPE_P = 0, > + V4L2_H264_SLICE_TYPE_B = 1, > + V4L2_H264_SLICE_TYPE_I = 2, > + V4L2_H264_SLICE_TYPE_SP = 3, > + V4L2_H264_SLICE_TYPE_SI = 4, > +}; > > +#define V4L2_SLICE_FLAG_FIELD_PIC 0x01 > +#define V4L2_SLICE_FLAG_BOTTOM_FIELD 0x02 > +#define V4L2_SLICE_FLAG_DIRECT_SPATIAL_MV_PRED 0x04 > +#define V4L2_SLICE_FLAG_SP_FOR_SWITCH 0x08 > +struct v4l2_ctrl_h264_slice_param { > + /* Size in bytes, including header */ > + __u32 size; > + /* Offset in bits to slice_data() from the beginning of this slice. */ > + __u32 header_bit_size; > + > + __u16 first_mb_in_slice; > + enum v4l2_h264_slice_type slice_type; > + __u8 pic_parameter_set_id; > + __u8 colour_plane_id; > + __u16 frame_num; > + __u16 idr_pic_id; > + __u16 pic_order_cnt_lsb; > + __s32 delta_pic_order_cnt_bottom; > + __s32 delta_pic_order_cnt0; > + __s32 delta_pic_order_cnt1; > + __u8 redundant_pic_cnt; > + > + struct v4l2_h264_pred_weight_table pred_weight_table; > + /* Size in bits of dec_ref_pic_marking() syntax element. */ > + __u32 dec_ref_pic_marking_bit_size; > + /* Size in bits of pic order count syntax. */ > + __u32 pic_order_cnt_bit_size; > + > + __u8 cabac_init_idc; > + __s8 slice_qp_delta; > + __s8 slice_qs_delta; > + __u8 disable_deblocking_filter_idc; > + __s8 slice_alpha_c0_offset_div2; > + __s8 slice_beta_offset_div2; > + __u32 slice_group_change_cycle; > + > + __u8 num_ref_idx_l0_active_minus1; > + __u8 num_ref_idx_l1_active_minus1; > + /* Entries on each list are indices > + * into v4l2_ctrl_h264_decode_param.dpb[]. */ > + __u8 ref_pic_list0[32]; > + __u8 ref_pic_list1[32]; > + > + __u8 flags; > +}; > + > +/* If not set, this entry is unused for reference. */ > +#define V4L2_H264_DPB_ENTRY_FLAG_ACTIVE 0x01 > +#define V4L2_H264_DPB_ENTRY_FLAG_LONG_TERM 0x02 > +struct v4l2_h264_dpb_entry { > + __u32 buf_index; /* v4l2_buffer index */ > + __u16 frame_num; > + __u16 pic_num; > + /* Note that field is indicated by v4l2_buffer.field */ > + __s32 top_field_order_cnt; > + __s32 bottom_field_order_cnt; > + __u8 flags; /* V4L2_H264_DPB_ENTRY_FLAG_* */ > +}; > + > +struct v4l2_ctrl_h264_decode_param { > + __u32 num_slices; > + __u8 idr_pic_flag; > + __u8 nal_ref_idc; > + __s32 top_field_order_cnt; > + __s32 bottom_field_order_cnt; > + __u8 ref_pic_list_p0[32]; > + __u8 ref_pic_list_b0[32]; > + __u8 ref_pic_list_b1[32]; > + struct v4l2_h264_dpb_entry dpb[16]; > +}; > + > struct v4l2_ctrl_mpeg2_frame_hdr { > __u32 slice_len; > __u32 slice_pos; > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > index 242a6bfa1440..4b4a1b25a0db 100644 > --- a/include/uapi/linux/videodev2.h > +++ b/include/uapi/linux/videodev2.h > @@ -626,6 +626,7 @@ struct v4l2_pix_format { > #define V4L2_PIX_FMT_H264 v4l2_fourcc('H', '2', '6', '4') /* H264 with start codes */ > #define V4L2_PIX_FMT_H264_NO_SC v4l2_fourcc('A', 'V', 'C', '1') /* H264 without start codes */ > #define V4L2_PIX_FMT_H264_MVC v4l2_fourcc('M', '2', '6', '4') /* H264 MVC */ > +#define V4L2_PIX_FMT_H264_SLICE v4l2_fourcc('S', '2', '6', '4') /* H264 parsed slices */ > #define V4L2_PIX_FMT_H263 v4l2_fourcc('H', '2', '6', '3') /* H263 */ > #define V4L2_PIX_FMT_MPEG1 v4l2_fourcc('M', 'P', 'G', '1') /* MPEG-1 ES */ > #define V4L2_PIX_FMT_MPEG2 v4l2_fourcc('M', 'P', 'G', '2') /* MPEG-2 ES */ > @@ -1589,6 +1590,11 @@ struct v4l2_ext_control { > __u8 __user *p_u8; > __u16 __user *p_u16; > __u32 __user *p_u32; > + struct v4l2_ctrl_h264_sps __user *p_h264_sps; > + struct v4l2_ctrl_h264_pps __user *p_h264_pps; > + struct v4l2_ctrl_h264_scaling_matrix __user *p_h264_scal_mtrx; > + struct v4l2_ctrl_h264_slice_param __user *p_h264_slice_param; > + struct v4l2_ctrl_h264_decode_param __user *p_h264_decode_param; > struct v4l2_ctrl_mpeg2_frame_hdr __user *p_mpeg2_frame_hdr; > void __user *ptr; > }; > @@ -1635,6 +1641,11 @@ enum v4l2_ctrl_type { > V4L2_CTRL_TYPE_U8 = 0x0100, > V4L2_CTRL_TYPE_U16 = 0x0101, > V4L2_CTRL_TYPE_U32 = 0x0102, > + V4L2_CTRL_TYPE_H264_SPS = 0x0103, > + V4L2_CTRL_TYPE_H264_PPS = 0x0104, > + V4L2_CTRL_TYPE_H264_SCALING_MATRIX = 0x0105, > + V4L2_CTRL_TYPE_H264_SLICE_PARAM = 0x0106, > + V4L2_CTRL_TYPE_H264_DECODE_PARAM = 0x0107, > V4L2_CTRL_TYPE_MPEG2_FRAME_HDR = 0x0109, > }; >
Hi Hans, Thanks for your feedback, I have a few things I'm not really sure about though. On Fri, Jun 15, 2018 at 01:59:58PM +0200, Hans Verkuil wrote: > > +struct v4l2_ctrl_h264_sps { > > + __u8 profile_idc; > > + __u8 constraint_set_flags; > > + __u8 level_idc; > > + __u8 seq_parameter_set_id; > > + __u8 chroma_format_idc; > > + __u8 bit_depth_luma_minus8; > > + __u8 bit_depth_chroma_minus8; > > + __u8 log2_max_frame_num_minus4; > > + __u8 pic_order_cnt_type; > > + __u8 log2_max_pic_order_cnt_lsb_minus4; > > There is a hole in the struct here. Is that OK? Are there alignment > requirements? This structure represents an equivalent structure in the H264 bitstream, but it's not a 1:1 mapping, so I don't think there's any alignment issues. As of the padding, is it an issue? Isn't it defined by the ABI, and therefore the userspace will have the same padding rules? > > > + __s32 offset_for_non_ref_pic; > > + __s32 offset_for_top_to_bottom_field; > > + __u8 num_ref_frames_in_pic_order_cnt_cycle; > > + __s32 offset_for_ref_frame[255]; > > Perhaps use a define instead of hardcoding 255? Not sure if that makes sense. > Same for other arrays below. > > > + __u8 max_num_ref_frames; > > + __u16 pic_width_in_mbs_minus1; > > + __u16 pic_height_in_map_units_minus1; > > + __u8 flags; > > +}; > > You have to test the struct layout for 32 bit and 64 bit systems > (the latter for both 64 bit arm and Intel). The layout should be the > same for all of them since the control framework does not support > compat32 conversions for compound controls. I'm not really sure how to test that though? Should I write a program doing a bunch of offset_of calls to make sure it matches by hand, or is there anything smarter? Thanks! Maxime
On Thu, Jul 12, 2018 at 06:38:21PM +0200, Maxime Ripard wrote: > Hi Hans, > > Thanks for your feedback, I have a few things I'm not really sure > about though. > > On Fri, Jun 15, 2018 at 01:59:58PM +0200, Hans Verkuil wrote: > > > +struct v4l2_ctrl_h264_sps { > > > + __u8 profile_idc; > > > + __u8 constraint_set_flags; > > > + __u8 level_idc; > > > + __u8 seq_parameter_set_id; > > > + __u8 chroma_format_idc; > > > + __u8 bit_depth_luma_minus8; > > > + __u8 bit_depth_chroma_minus8; > > > + __u8 log2_max_frame_num_minus4; > > > + __u8 pic_order_cnt_type; > > > + __u8 log2_max_pic_order_cnt_lsb_minus4; > > > > There is a hole in the struct here. Is that OK? Are there alignment > > requirements? > > This structure represents an equivalent structure in the H264 > bitstream, but it's not a 1:1 mapping, so I don't think there's any > alignment issues. > > As of the padding, is it an issue? Isn't it defined by the ABI, and > therefore the userspace will have the same padding rules? Hi Maxime It gets interesting when you have a 64 bit kernel and a 32 bit userspace. Andrew
On Wed, 2018-06-13 at 16:07 +0200, Maxime Ripard wrote: > From: Pawel Osciak <posciak@chromium.org> > > Signed-off-by: Pawel Osciak <posciak@chromium.org> > Reviewed-by: Wu-cheng Li <wuchengli@chromium.org> > Tested-by: Tomasz Figa <tfiga@chromium.org> > [rebase44(groeck): include linux/types.h in v4l2-controls.h] > Signed-off-by: Guenter Roeck <groeck@chromium.org> > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> > --- > [..] > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > index 242a6bfa1440..4b4a1b25a0db 100644 > --- a/include/uapi/linux/videodev2.h > +++ b/include/uapi/linux/videodev2.h > @@ -626,6 +626,7 @@ struct v4l2_pix_format { > #define V4L2_PIX_FMT_H264 v4l2_fourcc('H', '2', '6', '4') /* H264 with start codes */ > #define V4L2_PIX_FMT_H264_NO_SC v4l2_fourcc('A', 'V', 'C', '1') /* H264 without start codes */ > #define V4L2_PIX_FMT_H264_MVC v4l2_fourcc('M', '2', '6', '4') /* H264 MVC */ > +#define V4L2_PIX_FMT_H264_SLICE v4l2_fourcc('S', '2', '6', '4') /* H264 parsed slices */ As pointed out by Tomasz, the Rockchip VPU driver expects start codes [1], so the userspace should be aware of it. Perhaps we could document this pixel format better as: #define V4L2_PIX_FMT_H264_SLICE v4l2_fourcc('S', '2', '6', '4') /* H264 parsed slices with start codes */ And introduce another pixel format: #define V4L2_PIX_FMT_H264_SLICE_NO_SC v4l2_fourcc(TODO) /* H264 parsed slices without start codes */ For cedrus to use, as it seems it doesn't need start codes. How does it sound? [1] https://cs.chromium.org/chromium/src/media/gpu/v4l2/v4l2_slice_video_decode_accelerator.cc?rcl=63129434aeacf0f54bbae96814f10cf64e3e6c35&l=2438
Le mardi 21 août 2018 à 13:58 -0300, Ezequiel Garcia a écrit : > On Wed, 2018-06-13 at 16:07 +0200, Maxime Ripard wrote: > > From: Pawel Osciak <posciak@chromium.org> > > > > Signed-off-by: Pawel Osciak <posciak@chromium.org> > > Reviewed-by: Wu-cheng Li <wuchengli@chromium.org> > > Tested-by: Tomasz Figa <tfiga@chromium.org> > > [rebase44(groeck): include linux/types.h in v4l2-controls.h] > > Signed-off-by: Guenter Roeck <groeck@chromium.org> > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> > > --- > > > > [..] > > diff --git a/include/uapi/linux/videodev2.h > > b/include/uapi/linux/videodev2.h > > index 242a6bfa1440..4b4a1b25a0db 100644 > > --- a/include/uapi/linux/videodev2.h > > +++ b/include/uapi/linux/videodev2.h > > @@ -626,6 +626,7 @@ struct v4l2_pix_format { > > #define V4L2_PIX_FMT_H264 v4l2_fourcc('H', '2', '6', '4') /* > > H264 with start codes */ > > #define V4L2_PIX_FMT_H264_NO_SC v4l2_fourcc('A', 'V', 'C', '1') /* > > H264 without start codes */ > > #define V4L2_PIX_FMT_H264_MVC v4l2_fourcc('M', '2', '6', '4') /* > > H264 MVC */ > > +#define V4L2_PIX_FMT_H264_SLICE v4l2_fourcc('S', '2', '6', '4') /* > > H264 parsed slices */ > > As pointed out by Tomasz, the Rockchip VPU driver expects start codes > [1], so the userspace > should be aware of it. Perhaps we could document this pixel format > better as: > > #define V4L2_PIX_FMT_H264_SLICE v4l2_fourcc('S', '2', '6', '4') /* > H264 parsed slices with start codes */ > > And introduce another pixel format: > > #define V4L2_PIX_FMT_H264_SLICE_NO_SC v4l2_fourcc(TODO) /* H264 > parsed slices without start codes */ > > For cedrus to use, as it seems it doesn't need start codes. I must admit that this RK requirement is a bit weird for slice data. Though, userspace wise, always adding start-code would be compatible, as the driver can just offset to remove it. Another option, because I'm not fan of adding dedicated formats for this, the RK driver could use data_offset (in mplane v4l2 buffers), just write a start code there. I like this solution because I would not be surprise if some drivers requires in fact an HW specific header, that the driver can generate as needed. > > How does it sound? > > [1] > https://cs.chromium.org/chromium/src/media/gpu/v4l2/v4l2_slice_video_decode_accelerator.cc?rcl=63129434aeacf0f54bbae96814f10cf64e3e6c35&l=2438
Hi, On Tue, Aug 21, 2018 at 01:58:38PM -0300, Ezequiel Garcia wrote: > On Wed, 2018-06-13 at 16:07 +0200, Maxime Ripard wrote: > > From: Pawel Osciak <posciak@chromium.org> > > > > Signed-off-by: Pawel Osciak <posciak@chromium.org> > > Reviewed-by: Wu-cheng Li <wuchengli@chromium.org> > > Tested-by: Tomasz Figa <tfiga@chromium.org> > > [rebase44(groeck): include linux/types.h in v4l2-controls.h] > > Signed-off-by: Guenter Roeck <groeck@chromium.org> > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> > > --- > > > [..] > > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > > index 242a6bfa1440..4b4a1b25a0db 100644 > > --- a/include/uapi/linux/videodev2.h > > +++ b/include/uapi/linux/videodev2.h > > @@ -626,6 +626,7 @@ struct v4l2_pix_format { > > #define V4L2_PIX_FMT_H264 v4l2_fourcc('H', '2', '6', '4') /* H264 with start codes */ > > #define V4L2_PIX_FMT_H264_NO_SC v4l2_fourcc('A', 'V', 'C', '1') /* H264 without start codes */ > > #define V4L2_PIX_FMT_H264_MVC v4l2_fourcc('M', '2', '6', '4') /* H264 MVC */ > > +#define V4L2_PIX_FMT_H264_SLICE v4l2_fourcc('S', '2', '6', '4') /* H264 parsed slices */ > > As pointed out by Tomasz, the Rockchip VPU driver expects start codes [1], so the userspace > should be aware of it. Perhaps we could document this pixel format better as: > > #define V4L2_PIX_FMT_H264_SLICE v4l2_fourcc('S', '2', '6', '4') /* H264 parsed slices with start codes */ I'm not sure this is something we want to do at that point. libva doesn't give the start code, so this is only going to make the life of the sane controllers more difficult. And if you need to have the start code and parse it, then you're not so stateless anymore. Maxime
On Wed, Aug 22, 2018 at 6:16 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > Hi, > > On Tue, Aug 21, 2018 at 01:58:38PM -0300, Ezequiel Garcia wrote: > > On Wed, 2018-06-13 at 16:07 +0200, Maxime Ripard wrote: > > > From: Pawel Osciak <posciak@chromium.org> > > > > > > Signed-off-by: Pawel Osciak <posciak@chromium.org> > > > Reviewed-by: Wu-cheng Li <wuchengli@chromium.org> > > > Tested-by: Tomasz Figa <tfiga@chromium.org> > > > [rebase44(groeck): include linux/types.h in v4l2-controls.h] > > > Signed-off-by: Guenter Roeck <groeck@chromium.org> > > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> > > > --- > > > > > [..] > > > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > > > index 242a6bfa1440..4b4a1b25a0db 100644 > > > --- a/include/uapi/linux/videodev2.h > > > +++ b/include/uapi/linux/videodev2.h > > > @@ -626,6 +626,7 @@ struct v4l2_pix_format { > > > #define V4L2_PIX_FMT_H264 v4l2_fourcc('H', '2', '6', '4') /* H264 with start codes */ > > > #define V4L2_PIX_FMT_H264_NO_SC v4l2_fourcc('A', 'V', 'C', '1') /* H264 without start codes */ > > > #define V4L2_PIX_FMT_H264_MVC v4l2_fourcc('M', '2', '6', '4') /* H264 MVC */ > > > +#define V4L2_PIX_FMT_H264_SLICE v4l2_fourcc('S', '2', '6', '4') /* H264 parsed slices */ > > > > As pointed out by Tomasz, the Rockchip VPU driver expects start codes [1], so the userspace > > should be aware of it. Perhaps we could document this pixel format better as: > > > > #define V4L2_PIX_FMT_H264_SLICE v4l2_fourcc('S', '2', '6', '4') /* H264 parsed slices with start codes */ > > I'm not sure this is something we want to do at that point. libva > doesn't give the start code, so this is only going to make the life of > the sane controllers more difficult. And if you need to have the start > code and parse it, then you're not so stateless anymore. I might not remember correctly, but Rockchip decoder does some slice parsing on its own (despite not doing any higher level parsing). Probably that's why it needs those start codes. I wonder if libva is the best reference here. It's been designed almost entirely by Intel for Intel video hardware. We want something that could work with a wide range of devices and avoid something like a need to create a semi-stateless API few months later. In fact, hardware from another vendor, we're working with, also does parsing of slice headers internally. Moreover, we have some weird kind-of-stateful decoders, which cannot fully deal with bitstream on its own, e.g. cannot parse formats, cannot handle resolution changes, need H264 bitstream NALUs split into separate buffers, etc. As I suggested some time ago, having the full bitstream in the buffer, with offsets of particular units included in respective controls, would be the most scalable thing. If really needed, we could add flags telling the driver that particular units are present, so one's implementation of libva could put only raw slice data in the buffers. But perhaps it's libva which needs some amendment? Best regards, Tomasz
Hi, On Wed, 2018-08-22 at 18:54 +0900, Tomasz Figa wrote: > On Wed, Aug 22, 2018 at 6:16 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > > Hi, > > > > On Tue, Aug 21, 2018 at 01:58:38PM -0300, Ezequiel Garcia wrote: > > > On Wed, 2018-06-13 at 16:07 +0200, Maxime Ripard wrote: > > > > From: Pawel Osciak <posciak@chromium.org> > > > > > > > > Signed-off-by: Pawel Osciak <posciak@chromium.org> > > > > Reviewed-by: Wu-cheng Li <wuchengli@chromium.org> > > > > Tested-by: Tomasz Figa <tfiga@chromium.org> > > > > [rebase44(groeck): include linux/types.h in v4l2-controls.h] > > > > Signed-off-by: Guenter Roeck <groeck@chromium.org> > > > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> > > > > --- > > > > > > > > > > [..] > > > > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > > > > index 242a6bfa1440..4b4a1b25a0db 100644 > > > > --- a/include/uapi/linux/videodev2.h > > > > +++ b/include/uapi/linux/videodev2.h > > > > @@ -626,6 +626,7 @@ struct v4l2_pix_format { > > > > #define V4L2_PIX_FMT_H264 v4l2_fourcc('H', '2', '6', '4') /* H264 with start codes */ > > > > #define V4L2_PIX_FMT_H264_NO_SC v4l2_fourcc('A', 'V', 'C', '1') /* H264 without start codes */ > > > > #define V4L2_PIX_FMT_H264_MVC v4l2_fourcc('M', '2', '6', '4') /* H264 MVC */ > > > > +#define V4L2_PIX_FMT_H264_SLICE v4l2_fourcc('S', '2', '6', '4') /* H264 parsed slices */ > > > > > > As pointed out by Tomasz, the Rockchip VPU driver expects start codes [1], so the userspace > > > should be aware of it. Perhaps we could document this pixel format better as: > > > > > > #define V4L2_PIX_FMT_H264_SLICE v4l2_fourcc('S', '2', '6', '4') /* H264 parsed slices with start codes */ > > > > I'm not sure this is something we want to do at that point. libva > > doesn't give the start code, so this is only going to make the life of > > the sane controllers more difficult. And if you need to have the start > > code and parse it, then you're not so stateless anymore. > > I might not remember correctly, but Rockchip decoder does some slice > parsing on its own (despite not doing any higher level parsing). > Probably that's why it needs those start codes. The VPU found on Allwinner platforms also provides a mechanism to parse the bitstream data via a dedicated interface through the VPU registers. It is used in libvdpau-sunxi but not in our driver, because we don't want to be doing bitstream parsing in the kernel. It would be good to know if this is just a feature of the Rockchip VPU hardware that can be skipped (like on Allwinner) or if it's a hard requirement in its decoding pipeline. Also, maybe it only concerns the slice header? It is already part of the slice data (provided by VAAPI) for H.264/H.265 and an offset is provided to the beginning of the coded video data. > I wonder if libva is the best reference here. It's been designed > almost entirely by Intel for Intel video hardware. We want something > that could work with a wide range of devices and avoid something like > a need to create a semi-stateless API few months later. In fact, > hardware from another vendor, we're working with, also does parsing of > slice headers internally. Moreover, we have some weird > kind-of-stateful decoders, which cannot fully deal with bitstream on > its own, e.g. cannot parse formats, cannot handle resolution changes, > need H264 bitstream NALUs split into separate buffers, etc. > > As I suggested some time ago, having the full bitstream in the buffer, > with offsets of particular units included in respective controls, > would be the most scalable thing. If really needed, we could add flags > telling the driver that particular units are present, so one's > implementation of libva could put only raw slice data in the buffers. > But perhaps it's libva which needs some amendment? If the raw bitstream is needed, I think it would make more sense to use the already-existing formats for stateful VPUs along with the controls for stateless ones instead of having the full bitstream in the V4L2_PIX_FMT_*_SLICE formats. I would also be tempted to say that reconstructing the needed parts of the bitstream in-driver for these half-way VPUs would be a better approach than blurrying the line between how (and what) data should be passed for stateful and stateless VPUs at the API level. Stateless should only cover what's in the slice NAL unit RBSP, which excludes the start code detection bytes. It is no longer parsed data otherwise. Cheers, Paul
Hi, On Tue, 2018-08-21 at 13:07 -0400, Nicolas Dufresne wrote: > Le mardi 21 août 2018 à 13:58 -0300, Ezequiel Garcia a écrit : > > On Wed, 2018-06-13 at 16:07 +0200, Maxime Ripard wrote: > > > From: Pawel Osciak <posciak@chromium.org> > > > > > > Signed-off-by: Pawel Osciak <posciak@chromium.org> > > > Reviewed-by: Wu-cheng Li <wuchengli@chromium.org> > > > Tested-by: Tomasz Figa <tfiga@chromium.org> > > > [rebase44(groeck): include linux/types.h in v4l2-controls.h] > > > Signed-off-by: Guenter Roeck <groeck@chromium.org> > > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> > > > --- > > > > > > > [..] > > > diff --git a/include/uapi/linux/videodev2.h > > > b/include/uapi/linux/videodev2.h > > > index 242a6bfa1440..4b4a1b25a0db 100644 > > > --- a/include/uapi/linux/videodev2.h > > > +++ b/include/uapi/linux/videodev2.h > > > @@ -626,6 +626,7 @@ struct v4l2_pix_format { > > > #define V4L2_PIX_FMT_H264 v4l2_fourcc('H', '2', '6', '4') /* > > > H264 with start codes */ > > > #define V4L2_PIX_FMT_H264_NO_SC v4l2_fourcc('A', 'V', 'C', '1') /* > > > H264 without start codes */ > > > #define V4L2_PIX_FMT_H264_MVC v4l2_fourcc('M', '2', '6', '4') /* > > > H264 MVC */ > > > +#define V4L2_PIX_FMT_H264_SLICE v4l2_fourcc('S', '2', '6', '4') /* > > > H264 parsed slices */ > > > > As pointed out by Tomasz, the Rockchip VPU driver expects start codes > > [1], so the userspace > > should be aware of it. Perhaps we could document this pixel format > > better as: > > > > #define V4L2_PIX_FMT_H264_SLICE v4l2_fourcc('S', '2', '6', '4') /* > > H264 parsed slices with start codes */ > > > > And introduce another pixel format: > > > > #define V4L2_PIX_FMT_H264_SLICE_NO_SC v4l2_fourcc(TODO) /* H264 > > parsed slices without start codes */ > > > > For cedrus to use, as it seems it doesn't need start codes. > > I must admit that this RK requirement is a bit weird for slice data. > Though, userspace wise, always adding start-code would be compatible, > as the driver can just offset to remove it. This would mean that the stateless API no longer takes parsed bitstream data but effectively the full bitstream, which defeats the purpose of the _SLICE pixel formats. > Another option, because I'm not fan of adding dedicated formats for > this, the RK driver could use data_offset (in mplane v4l2 buffers), > just write a start code there. I like this solution because I would not > be surprise if some drivers requires in fact an HW specific header, > that the driver can generate as needed. I like this idea, because it implies that the driver should deal with the specificities of the hardware, instead of making the blurrying the lines of stateless API for covering these cases. > > > > How does it sound? > > > > [1] > > https://cs.chromium.org/chromium/src/media/gpu/v4l2/v4l2_slice_video_decode_accelerator.cc?rcl=63129434aeacf0f54bbae96814f10cf64e3e6c35&l=2438
On Wed, Aug 22, 2018 at 10:03 PM Paul Kocialkowski <paul.kocialkowski@bootlin.com> wrote: > > Hi, > > On Wed, 2018-08-22 at 18:54 +0900, Tomasz Figa wrote: > > On Wed, Aug 22, 2018 at 6:16 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > > > > Hi, > > > > > > On Tue, Aug 21, 2018 at 01:58:38PM -0300, Ezequiel Garcia wrote: > > > > On Wed, 2018-06-13 at 16:07 +0200, Maxime Ripard wrote: > > > > > From: Pawel Osciak <posciak@chromium.org> > > > > > > > > > > Signed-off-by: Pawel Osciak <posciak@chromium.org> > > > > > Reviewed-by: Wu-cheng Li <wuchengli@chromium.org> > > > > > Tested-by: Tomasz Figa <tfiga@chromium.org> > > > > > [rebase44(groeck): include linux/types.h in v4l2-controls.h] > > > > > Signed-off-by: Guenter Roeck <groeck@chromium.org> > > > > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> > > > > > --- > > > > > > > > > > > > > [..] > > > > > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > > > > > index 242a6bfa1440..4b4a1b25a0db 100644 > > > > > --- a/include/uapi/linux/videodev2.h > > > > > +++ b/include/uapi/linux/videodev2.h > > > > > @@ -626,6 +626,7 @@ struct v4l2_pix_format { > > > > > #define V4L2_PIX_FMT_H264 v4l2_fourcc('H', '2', '6', '4') /* H264 with start codes */ > > > > > #define V4L2_PIX_FMT_H264_NO_SC v4l2_fourcc('A', 'V', 'C', '1') /* H264 without start codes */ > > > > > #define V4L2_PIX_FMT_H264_MVC v4l2_fourcc('M', '2', '6', '4') /* H264 MVC */ > > > > > +#define V4L2_PIX_FMT_H264_SLICE v4l2_fourcc('S', '2', '6', '4') /* H264 parsed slices */ > > > > > > > > As pointed out by Tomasz, the Rockchip VPU driver expects start codes [1], so the userspace > > > > should be aware of it. Perhaps we could document this pixel format better as: > > > > > > > > #define V4L2_PIX_FMT_H264_SLICE v4l2_fourcc('S', '2', '6', '4') /* H264 parsed slices with start codes */ > > > > > > I'm not sure this is something we want to do at that point. libva > > > doesn't give the start code, so this is only going to make the life of > > > the sane controllers more difficult. And if you need to have the start > > > code and parse it, then you're not so stateless anymore. > > > > I might not remember correctly, but Rockchip decoder does some slice > > parsing on its own (despite not doing any higher level parsing). > > Probably that's why it needs those start codes. > > The VPU found on Allwinner platforms also provides a mechanism to parse > the bitstream data via a dedicated interface through the VPU registers. > It is used in libvdpau-sunxi but not in our driver, because we don't > want to be doing bitstream parsing in the kernel. > > It would be good to know if this is just a feature of the Rockchip VPU > hardware that can be skipped (like on Allwinner) or if it's a hard > requirement in its decoding pipeline. It's a hard requirement for its decoding pipeline, but... > Also, maybe it only concerns the > slice header? It is already part of the slice data (provided by VAAPI) > for H.264/H.265 and an offset is provided to the beginning of the coded > video data. Yes, it seems to be only the slice header. > > > I wonder if libva is the best reference here. It's been designed > > almost entirely by Intel for Intel video hardware. We want something > > that could work with a wide range of devices and avoid something like > > a need to create a semi-stateless API few months later. In fact, > > hardware from another vendor, we're working with, also does parsing of > > slice headers internally. Moreover, we have some weird > > kind-of-stateful decoders, which cannot fully deal with bitstream on > > its own, e.g. cannot parse formats, cannot handle resolution changes, > > need H264 bitstream NALUs split into separate buffers, etc. > > > > As I suggested some time ago, having the full bitstream in the buffer, > > with offsets of particular units included in respective controls, > > would be the most scalable thing. If really needed, we could add flags > > telling the driver that particular units are present, so one's > > implementation of libva could put only raw slice data in the buffers. > > But perhaps it's libva which needs some amendment? > > If the raw bitstream is needed, I think it would make more sense to use > the already-existing formats for stateful VPUs along with the controls > for stateless ones instead of having the full bitstream in the > V4L2_PIX_FMT_*_SLICE formats. It may indeed make sense to separate this based on pixel format. However, how do we tell the client that it needs to provide those controls? Current concept was based entirely on pixel format, so I guess that would mean creating something like V4L2_PIX_FMT_*_NOT_REALLY_SLICE (_PARSED, _STATELESS?). Might be okay, though... > > I would also be tempted to say that reconstructing the needed parts of > the bitstream in-driver for these half-way VPUs would be a better > approach than blurrying the line between how (and what) data should be > passed for stateful and stateless VPUs at the API level. Stateless > should only cover what's in the slice NAL unit RBSP, which excludes the > start code detection bytes. It is no longer parsed data otherwise. I'm not sure where such decision comes from. In particular, Chromium, from which this code originates, includes start codes in V4L2_PIX_FMT_H264_SLICE. As I mentioned earlier, we can't design this API based only on 1 type of hardware semantics. The stateless API should cover any kind of codec that needs user space assistance in processing the stream, which in practice would be almost everything for which stateful API doesn't work. That said, since pixel format essentially specifies the buffer contents, having such cases differentiated based on the pixel format doesn't sound insane. Best regards, Tomasz
On Wed, Aug 22, 2018 at 10:07 PM Paul Kocialkowski <paul.kocialkowski@bootlin.com> wrote: > > Hi, > > On Tue, 2018-08-21 at 13:07 -0400, Nicolas Dufresne wrote: > > Le mardi 21 août 2018 à 13:58 -0300, Ezequiel Garcia a écrit : > > > On Wed, 2018-06-13 at 16:07 +0200, Maxime Ripard wrote: > > > > From: Pawel Osciak <posciak@chromium.org> > > > > > > > > Signed-off-by: Pawel Osciak <posciak@chromium.org> > > > > Reviewed-by: Wu-cheng Li <wuchengli@chromium.org> > > > > Tested-by: Tomasz Figa <tfiga@chromium.org> > > > > [rebase44(groeck): include linux/types.h in v4l2-controls.h] > > > > Signed-off-by: Guenter Roeck <groeck@chromium.org> > > > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> > > > > --- > > > > > > > > > > [..] > > > > diff --git a/include/uapi/linux/videodev2.h > > > > b/include/uapi/linux/videodev2.h > > > > index 242a6bfa1440..4b4a1b25a0db 100644 > > > > --- a/include/uapi/linux/videodev2.h > > > > +++ b/include/uapi/linux/videodev2.h > > > > @@ -626,6 +626,7 @@ struct v4l2_pix_format { > > > > #define V4L2_PIX_FMT_H264 v4l2_fourcc('H', '2', '6', '4') /* > > > > H264 with start codes */ > > > > #define V4L2_PIX_FMT_H264_NO_SC v4l2_fourcc('A', 'V', 'C', '1') /* > > > > H264 without start codes */ > > > > #define V4L2_PIX_FMT_H264_MVC v4l2_fourcc('M', '2', '6', '4') /* > > > > H264 MVC */ > > > > +#define V4L2_PIX_FMT_H264_SLICE v4l2_fourcc('S', '2', '6', '4') /* > > > > H264 parsed slices */ > > > > > > As pointed out by Tomasz, the Rockchip VPU driver expects start codes > > > [1], so the userspace > > > should be aware of it. Perhaps we could document this pixel format > > > better as: > > > > > > #define V4L2_PIX_FMT_H264_SLICE v4l2_fourcc('S', '2', '6', '4') /* > > > H264 parsed slices with start codes */ > > > > > > And introduce another pixel format: > > > > > > #define V4L2_PIX_FMT_H264_SLICE_NO_SC v4l2_fourcc(TODO) /* H264 > > > parsed slices without start codes */ > > > > > > For cedrus to use, as it seems it doesn't need start codes. > > > > I must admit that this RK requirement is a bit weird for slice data. > > Though, userspace wise, always adding start-code would be compatible, > > as the driver can just offset to remove it. > > This would mean that the stateless API no longer takes parsed bitstream > data but effectively the full bitstream, which defeats the purpose of > the _SLICE pixel formats. > Not entirely. One of the purposes of the _SLICE pixel format was to specify it in a way that adds a requirement of providing the required controls by the client. > > Another option, because I'm not fan of adding dedicated formats for > > this, the RK driver could use data_offset (in mplane v4l2 buffers), > > just write a start code there. I like this solution because I would not > > be surprise if some drivers requires in fact an HW specific header, > > that the driver can generate as needed. > > I like this idea, because it implies that the driver should deal with > the specificities of the hardware, instead of making the blurrying the > lines of stateless API for covering these cases. The spec says "Offset in bytes to video data in the plane. Drivers must set this field when type refers to a capture stream, applications when it refers to an output stream." which would mean that user space would have to know to reserve some bytes at the beginning for the driver to add the start code there. (Or the driver memmove()ing the data forward when the buffer is queued, assuming that there is enough space in the buffer, but it should normally be the case.) Sounds like a pixel format with full bitstream data and some offsets to particular parts inside given inside a control might be the most flexible and cleanest solution. Best regards, Tomasz
Le mercredi 22 août 2018 à 22:38 +0900, Tomasz Figa a écrit : > On Wed, Aug 22, 2018 at 10:07 PM Paul Kocialkowski > <paul.kocialkowski@bootlin.com> wrote: > > > > Hi, > > > > On Tue, 2018-08-21 at 13:07 -0400, Nicolas Dufresne wrote: > > > Le mardi 21 août 2018 à 13:58 -0300, Ezequiel Garcia a écrit : > > > > On Wed, 2018-06-13 at 16:07 +0200, Maxime Ripard wrote: > > > > > From: Pawel Osciak <posciak@chromium.org> > > > > > > > > > > Signed-off-by: Pawel Osciak <posciak@chromium.org> > > > > > Reviewed-by: Wu-cheng Li <wuchengli@chromium.org> > > > > > Tested-by: Tomasz Figa <tfiga@chromium.org> > > > > > [rebase44(groeck): include linux/types.h in v4l2-controls.h] > > > > > Signed-off-by: Guenter Roeck <groeck@chromium.org> > > > > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> > > > > > --- > > > > > > > > > > > > > [..] > > > > > diff --git a/include/uapi/linux/videodev2.h > > > > > b/include/uapi/linux/videodev2.h > > > > > index 242a6bfa1440..4b4a1b25a0db 100644 > > > > > --- a/include/uapi/linux/videodev2.h > > > > > +++ b/include/uapi/linux/videodev2.h > > > > > @@ -626,6 +626,7 @@ struct v4l2_pix_format { > > > > > #define V4L2_PIX_FMT_H264 v4l2_fourcc('H', '2', '6', '4') /* > > > > > H264 with start codes */ > > > > > #define V4L2_PIX_FMT_H264_NO_SC v4l2_fourcc('A', 'V', 'C', '1') /* > > > > > H264 without start codes */ > > > > > #define V4L2_PIX_FMT_H264_MVC v4l2_fourcc('M', '2', '6', '4') /* > > > > > H264 MVC */ > > > > > +#define V4L2_PIX_FMT_H264_SLICE v4l2_fourcc('S', '2', '6', '4') /* > > > > > H264 parsed slices */ > > > > > > > > As pointed out by Tomasz, the Rockchip VPU driver expects start codes > > > > [1], so the userspace > > > > should be aware of it. Perhaps we could document this pixel format > > > > better as: > > > > > > > > #define V4L2_PIX_FMT_H264_SLICE v4l2_fourcc('S', '2', '6', '4') /* > > > > H264 parsed slices with start codes */ > > > > > > > > And introduce another pixel format: > > > > > > > > #define V4L2_PIX_FMT_H264_SLICE_NO_SC v4l2_fourcc(TODO) /* H264 > > > > parsed slices without start codes */ > > > > > > > > For cedrus to use, as it seems it doesn't need start codes. > > > > > > I must admit that this RK requirement is a bit weird for slice data. > > > Though, userspace wise, always adding start-code would be compatible, > > > as the driver can just offset to remove it. > > > > This would mean that the stateless API no longer takes parsed bitstream > > data but effectively the full bitstream, which defeats the purpose of > > the _SLICE pixel formats. > > > > Not entirely. One of the purposes of the _SLICE pixel format was to > specify it in a way that adds a requirement of providing the required > controls by the client. Slice is also a bitstream alignment requirement. In the default _H264 format we pass an complete AU, while in _SLICE we will pass single NAL (along with the tables through controls of course). Though, H264 does not only support start codes, it also supports AVC headers (as stored in ISOMP4). That makes the sentence "passing the full bitstream" quite ambiguous, as whatever we require, userspace may endup having to replace that part of the NAL. > > > > Another option, because I'm not fan of adding dedicated formats for > > > this, the RK driver could use data_offset (in mplane v4l2 buffers), > > > just write a start code there. I like this solution because I would not > > > be surprise if some drivers requires in fact an HW specific header, > > > that the driver can generate as needed. > > > > I like this idea, because it implies that the driver should deal with > > the specificities of the hardware, instead of making the blurrying the > > lines of stateless API for covering these cases. > > The spec says > > "Offset in bytes to video data in the plane. Drivers must set this > field when type refers to a capture stream, applications when it > refers to an output stream." > > which would mean that user space would have to know to reserve some > bytes at the beginning for the driver to add the start code there. (Or > the driver memmove()ing the data forward when the buffer is queued, > assuming that there is enough space in the buffer, but it should > normally be the case.) > > Sounds like a pixel format with full bitstream data and some offsets > to particular parts inside given inside a control might be the most > flexible and cleanest solution. Yeah, that comment was miss-informed. But it's simpler then that, the prefix can be added internally to the driver, there is no need to communicate this prefix to userspace. Again, if you go the pixel format way, you'll need to resolve the large ambiguity of saying "the full bitstream data". While dealing with a prefix in the driver, is just about writing couple of bytes, and then you can support HW that wants start-codes, AVC/AVC3 headers or HW specific headers. I've seen a totally custom header on Panasonic HW accelerator on some smart TV. The other important aspect, is that VAAPI won't provide any headers, just the raw NAL. So you'd be forcing userspace to handle a HW specific requirement, which is trivial to adapt. > > Best regards, > Tomasz
Le mercredi 22 août 2018 à 22:24 +0900, Tomasz Figa a écrit : > On Wed, Aug 22, 2018 at 10:03 PM Paul Kocialkowski > <paul.kocialkowski@bootlin.com> wrote: > > > > Hi, > > > > On Wed, 2018-08-22 at 18:54 +0900, Tomasz Figa wrote: > > > On Wed, Aug 22, 2018 at 6:16 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > > > > > > Hi, > > > > > > > > On Tue, Aug 21, 2018 at 01:58:38PM -0300, Ezequiel Garcia wrote: > > > > > On Wed, 2018-06-13 at 16:07 +0200, Maxime Ripard wrote: > > > > > > From: Pawel Osciak <posciak@chromium.org> > > > > > > > > > > > > Signed-off-by: Pawel Osciak <posciak@chromium.org> > > > > > > Reviewed-by: Wu-cheng Li <wuchengli@chromium.org> > > > > > > Tested-by: Tomasz Figa <tfiga@chromium.org> > > > > > > [rebase44(groeck): include linux/types.h in v4l2-controls.h] > > > > > > Signed-off-by: Guenter Roeck <groeck@chromium.org> > > > > > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> > > > > > > --- > > > > > > > > > > > > > > > > [..] > > > > > > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > > > > > > index 242a6bfa1440..4b4a1b25a0db 100644 > > > > > > --- a/include/uapi/linux/videodev2.h > > > > > > +++ b/include/uapi/linux/videodev2.h > > > > > > @@ -626,6 +626,7 @@ struct v4l2_pix_format { > > > > > > #define V4L2_PIX_FMT_H264 v4l2_fourcc('H', '2', '6', '4') /* H264 with start codes */ > > > > > > #define V4L2_PIX_FMT_H264_NO_SC v4l2_fourcc('A', 'V', 'C', '1') /* H264 without start codes */ > > > > > > #define V4L2_PIX_FMT_H264_MVC v4l2_fourcc('M', '2', '6', '4') /* H264 MVC */ > > > > > > +#define V4L2_PIX_FMT_H264_SLICE v4l2_fourcc('S', '2', '6', '4') /* H264 parsed slices */ > > > > > > > > > > As pointed out by Tomasz, the Rockchip VPU driver expects start codes [1], so the userspace > > > > > should be aware of it. Perhaps we could document this pixel format better as: > > > > > > > > > > #define V4L2_PIX_FMT_H264_SLICE v4l2_fourcc('S', '2', '6', '4') /* H264 parsed slices with start codes */ > > > > > > > > I'm not sure this is something we want to do at that point. libva > > > > doesn't give the start code, so this is only going to make the life of > > > > the sane controllers more difficult. And if you need to have the start > > > > code and parse it, then you're not so stateless anymore. > > > > > > I might not remember correctly, but Rockchip decoder does some slice > > > parsing on its own (despite not doing any higher level parsing). > > > Probably that's why it needs those start codes. > > > > The VPU found on Allwinner platforms also provides a mechanism to parse > > the bitstream data via a dedicated interface through the VPU registers. > > It is used in libvdpau-sunxi but not in our driver, because we don't > > want to be doing bitstream parsing in the kernel. > > > > It would be good to know if this is just a feature of the Rockchip VPU > > hardware that can be skipped (like on Allwinner) or if it's a hard > > requirement in its decoding pipeline. > > It's a hard requirement for its decoding pipeline, but... > > > Also, maybe it only concerns the > > slice header? It is already part of the slice data (provided by VAAPI) > > for H.264/H.265 and an offset is provided to the beginning of the coded > > video data. > > Yes, it seems to be only the slice header. > > > > > > I wonder if libva is the best reference here. It's been designed > > > almost entirely by Intel for Intel video hardware. We want something > > > that could work with a wide range of devices and avoid something like > > > a need to create a semi-stateless API few months later. In fact, > > > hardware from another vendor, we're working with, also does parsing of > > > slice headers internally. Moreover, we have some weird > > > kind-of-stateful decoders, which cannot fully deal with bitstream on > > > its own, e.g. cannot parse formats, cannot handle resolution changes, > > > need H264 bitstream NALUs split into separate buffers, etc. > > > > > > As I suggested some time ago, having the full bitstream in the buffer, > > > with offsets of particular units included in respective controls, > > > would be the most scalable thing. If really needed, we could add flags > > > telling the driver that particular units are present, so one's > > > implementation of libva could put only raw slice data in the buffers. > > > But perhaps it's libva which needs some amendment? > > > > If the raw bitstream is needed, I think it would make more sense to use > > the already-existing formats for stateful VPUs along with the controls > > for stateless ones instead of having the full bitstream in the > > V4L2_PIX_FMT_*_SLICE formats. > > It may indeed make sense to separate this based on pixel format. > However, how do we tell the client that it needs to provide those > controls? Current concept was based entirely on pixel format, so I > guess that would mean creating something like > V4L2_PIX_FMT_*_NOT_REALLY_SLICE (_PARSED, _STATELESS?). Might be okay, > though... > > > > > I would also be tempted to say that reconstructing the needed parts of > > the bitstream in-driver for these half-way VPUs would be a better > > approach than blurrying the line between how (and what) data should be > > passed for stateful and stateless VPUs at the API level. Stateless > > should only cover what's in the slice NAL unit RBSP, which excludes the > > start code detection bytes. It is no longer parsed data otherwise. > > I'm not sure where such decision comes from. In particular, Chromium, > from which this code originates, includes start codes in > V4L2_PIX_FMT_H264_SLICE. As I mentioned earlier, we can't design this > API based only on 1 type of hardware semantics. The stateless API > should cover any kind of codec that needs user space assistance in > processing the stream, which in practice would be almost everything > for which stateful API doesn't work. Well, be aware that in most of today's use case, Chromium bitstream handling is suboptimal since the startcode are crafted by Chromium. Most of today's use case uses ISOMP4 container, which does not hold start codes but AVC headers. If we step back a little, the start codes are designed for randomly accessing bytes. It's a mark made to be able to randomly scan and find the start of a NAL. In a well framed stream, they are useless. Then AVC header is designed to announce the size of the following block, if we pass only one NAL by buffer, this is not very useful either. Both seems trivial to reconstruct if the HW requires it. > > That said, since pixel format essentially specifies the buffer > contents, having such cases differentiated based on the pixel format > doesn't sound insane. As of today, that's what the pixel formats tries to do with these _NO_SC postfix (the current naming is generally ambiguous though). > > Best regards, > Tomasz
Hi, On Wed, 2018-08-22 at 22:24 +0900, Tomasz Figa wrote: > On Wed, Aug 22, 2018 at 10:03 PM Paul Kocialkowski > <paul.kocialkowski@bootlin.com> wrote: > > > > Hi, > > > > On Wed, 2018-08-22 at 18:54 +0900, Tomasz Figa wrote: > > > On Wed, Aug 22, 2018 at 6:16 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > > > > > > Hi, > > > > > > > > On Tue, Aug 21, 2018 at 01:58:38PM -0300, Ezequiel Garcia wrote: > > > > > On Wed, 2018-06-13 at 16:07 +0200, Maxime Ripard wrote: > > > > > > From: Pawel Osciak <posciak@chromium.org> > > > > > > > > > > > > Signed-off-by: Pawel Osciak <posciak@chromium.org> > > > > > > Reviewed-by: Wu-cheng Li <wuchengli@chromium.org> > > > > > > Tested-by: Tomasz Figa <tfiga@chromium.org> > > > > > > [rebase44(groeck): include linux/types.h in v4l2-controls.h] > > > > > > Signed-off-by: Guenter Roeck <groeck@chromium.org> > > > > > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> > > > > > > --- > > > > > > > > > > > > > > > > [..] > > > > > > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > > > > > > index 242a6bfa1440..4b4a1b25a0db 100644 > > > > > > --- a/include/uapi/linux/videodev2.h > > > > > > +++ b/include/uapi/linux/videodev2.h > > > > > > @@ -626,6 +626,7 @@ struct v4l2_pix_format { > > > > > > #define V4L2_PIX_FMT_H264 v4l2_fourcc('H', '2', '6', '4') /* H264 with start codes */ > > > > > > #define V4L2_PIX_FMT_H264_NO_SC v4l2_fourcc('A', 'V', 'C', '1') /* H264 without start codes */ > > > > > > #define V4L2_PIX_FMT_H264_MVC v4l2_fourcc('M', '2', '6', '4') /* H264 MVC */ > > > > > > +#define V4L2_PIX_FMT_H264_SLICE v4l2_fourcc('S', '2', '6', '4') /* H264 parsed slices */ > > > > > > > > > > As pointed out by Tomasz, the Rockchip VPU driver expects start codes [1], so the userspace > > > > > should be aware of it. Perhaps we could document this pixel format better as: > > > > > > > > > > #define V4L2_PIX_FMT_H264_SLICE v4l2_fourcc('S', '2', '6', '4') /* H264 parsed slices with start codes */ > > > > > > > > I'm not sure this is something we want to do at that point. libva > > > > doesn't give the start code, so this is only going to make the life of > > > > the sane controllers more difficult. And if you need to have the start > > > > code and parse it, then you're not so stateless anymore. > > > > > > I might not remember correctly, but Rockchip decoder does some slice > > > parsing on its own (despite not doing any higher level parsing). > > > Probably that's why it needs those start codes. > > > > The VPU found on Allwinner platforms also provides a mechanism to parse > > the bitstream data via a dedicated interface through the VPU registers. > > It is used in libvdpau-sunxi but not in our driver, because we don't > > want to be doing bitstream parsing in the kernel. > > > > It would be good to know if this is just a feature of the Rockchip VPU > > hardware that can be skipped (like on Allwinner) or if it's a hard > > requirement in its decoding pipeline. > > It's a hard requirement for its decoding pipeline, but... > > > Also, maybe it only concerns the > > slice header? It is already part of the slice data (provided by VAAPI) > > for H.264/H.265 and an offset is provided to the beginning of the coded > > video data. > > Yes, it seems to be only the slice header. Sounds good, then I don't have any problem with that. > > > > > I wonder if libva is the best reference here. It's been designed > > > almost entirely by Intel for Intel video hardware. We want something > > > that could work with a wide range of devices and avoid something like > > > a need to create a semi-stateless API few months later. In fact, > > > hardware from another vendor, we're working with, also does parsing of > > > slice headers internally. Moreover, we have some weird > > > kind-of-stateful decoders, which cannot fully deal with bitstream on > > > its own, e.g. cannot parse formats, cannot handle resolution changes, > > > need H264 bitstream NALUs split into separate buffers, etc. > > > > > > As I suggested some time ago, having the full bitstream in the buffer, > > > with offsets of particular units included in respective controls, > > > would be the most scalable thing. If really needed, we could add flags > > > telling the driver that particular units are present, so one's > > > implementation of libva could put only raw slice data in the buffers. > > > But perhaps it's libva which needs some amendment? > > > > If the raw bitstream is needed, I think it would make more sense to use > > the already-existing formats for stateful VPUs along with the controls > > for stateless ones instead of having the full bitstream in the > > V4L2_PIX_FMT_*_SLICE formats. > > It may indeed make sense to separate this based on pixel format. > However, how do we tell the client that it needs to provide those > controls? Current concept was based entirely on pixel format, so I > guess that would mean creating something like > V4L2_PIX_FMT_*_NOT_REALLY_SLICE (_PARSED, _STATELESS?). Might be okay, > though... How about declaring support for the request API (through the associated CAPs) and only having the non-_SLICE formats listed in ENUM_FMT? > > I would also be tempted to say that reconstructing the needed parts of > > the bitstream in-driver for these half-way VPUs would be a better > > approach than blurrying the line between how (and what) data should be > > passed for stateful and stateless VPUs at the API level. Stateless > > should only cover what's in the slice NAL unit RBSP, which excludes the > > start code detection bytes. It is no longer parsed data otherwise. > > I'm not sure where such decision comes from. In particular, Chromium, > from which this code originates, includes start codes in > V4L2_PIX_FMT_H264_SLICE. As I mentioned earlier, we can't design this > API based only on 1 type of hardware semantics. The stateless API > should cover any kind of codec that needs user space assistance in > processing the stream, which in practice would be almost everything > for which stateful API doesn't work. Maybe we need to formalize what the stateless API aims to support and what the formats really entail. I was under the impression that it was synonymouse with providing parsed bitstream to the kernel. The way I understand "parsed bitstream" means the slice NALU RBSP as raw data and the metadata from other NALUs as controls (with the overlap of the slice header in recent formats, that is in both). I don't see any other sane boundary that could be conceptually attached to something like "providing parsed bitstream". Covering all that might be required for != stateful seems hard to formalize conceptually and to delimit in general. > That said, since pixel format essentially specifies the buffer > contents, having such cases differentiated based on the pixel format > doesn't sound insane. Great!
Hi, On Wed, 2018-08-22 at 22:38 +0900, Tomasz Figa wrote: > On Wed, Aug 22, 2018 at 10:07 PM Paul Kocialkowski > <paul.kocialkowski@bootlin.com> wrote: > > > > Hi, > > > > On Tue, 2018-08-21 at 13:07 -0400, Nicolas Dufresne wrote: > > > Le mardi 21 août 2018 à 13:58 -0300, Ezequiel Garcia a écrit : > > > > On Wed, 2018-06-13 at 16:07 +0200, Maxime Ripard wrote: > > > > > From: Pawel Osciak <posciak@chromium.org> > > > > > > > > > > Signed-off-by: Pawel Osciak <posciak@chromium.org> > > > > > Reviewed-by: Wu-cheng Li <wuchengli@chromium.org> > > > > > Tested-by: Tomasz Figa <tfiga@chromium.org> > > > > > [rebase44(groeck): include linux/types.h in v4l2-controls.h] > > > > > Signed-off-by: Guenter Roeck <groeck@chromium.org> > > > > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> > > > > > --- > > > > > > > > > > > > > [..] > > > > > diff --git a/include/uapi/linux/videodev2.h > > > > > b/include/uapi/linux/videodev2.h > > > > > index 242a6bfa1440..4b4a1b25a0db 100644 > > > > > --- a/include/uapi/linux/videodev2.h > > > > > +++ b/include/uapi/linux/videodev2.h > > > > > @@ -626,6 +626,7 @@ struct v4l2_pix_format { > > > > > #define V4L2_PIX_FMT_H264 v4l2_fourcc('H', '2', '6', '4') /* > > > > > H264 with start codes */ > > > > > #define V4L2_PIX_FMT_H264_NO_SC v4l2_fourcc('A', 'V', 'C', '1') /* > > > > > H264 without start codes */ > > > > > #define V4L2_PIX_FMT_H264_MVC v4l2_fourcc('M', '2', '6', '4') /* > > > > > H264 MVC */ > > > > > +#define V4L2_PIX_FMT_H264_SLICE v4l2_fourcc('S', '2', '6', '4') /* > > > > > H264 parsed slices */ > > > > > > > > As pointed out by Tomasz, the Rockchip VPU driver expects start codes > > > > [1], so the userspace > > > > should be aware of it. Perhaps we could document this pixel format > > > > better as: > > > > > > > > #define V4L2_PIX_FMT_H264_SLICE v4l2_fourcc('S', '2', '6', '4') /* > > > > H264 parsed slices with start codes */ > > > > > > > > And introduce another pixel format: > > > > > > > > #define V4L2_PIX_FMT_H264_SLICE_NO_SC v4l2_fourcc(TODO) /* H264 > > > > parsed slices without start codes */ > > > > > > > > For cedrus to use, as it seems it doesn't need start codes. > > > > > > I must admit that this RK requirement is a bit weird for slice data. > > > Though, userspace wise, always adding start-code would be compatible, > > > as the driver can just offset to remove it. > > > > This would mean that the stateless API no longer takes parsed bitstream > > data but effectively the full bitstream, which defeats the purpose of > > the _SLICE pixel formats. > > > > Not entirely. One of the purposes of the _SLICE pixel format was to > specify it in a way that adds a requirement of providing the required > controls by the client. I think we need to define what we want the stateless APIs (and these formats) to precisely reflect conceptually. I've started discussing this in the Request API and V4L2 capabilities thread. > > > Another option, because I'm not fan of adding dedicated formats for > > > this, the RK driver could use data_offset (in mplane v4l2 buffers), > > > just write a start code there. I like this solution because I would not > > > be surprise if some drivers requires in fact an HW specific header, > > > that the driver can generate as needed. > > > > I like this idea, because it implies that the driver should deal with > > the specificities of the hardware, instead of making the blurrying the > > lines of stateless API for covering these cases. > > The spec says > > "Offset in bytes to video data in the plane. Drivers must set this > field when type refers to a capture stream, applications when it > refers to an output stream." > > which would mean that user space would have to know to reserve some > bytes at the beginning for the driver to add the start code there. (Or > the driver memmove()ing the data forward when the buffer is queued, > assuming that there is enough space in the buffer, but it should > normally be the case.) > > Sounds like a pixel format with full bitstream data and some offsets > to particular parts inside given inside a control might be the most > flexible and cleanest solution. I can't help but think that bringing the whole bitstream over to the kernel with a dedicated pix fmt just for the sake of having 3 start code bytes is rather overkill anyway. I believe moving the data around to be the best call for this situation. Or maybe there's a way to alloc more data *before* the bufer that is exposed to userspace, so userspace can fill it normally and the driver can bring-in the necessary heading start code bytes before the buffer? Cheers, Paul
On Wed, Aug 22, 2018 at 11:45 PM Paul Kocialkowski <paul.kocialkowski@bootlin.com> wrote: > > Hi, > > On Wed, 2018-08-22 at 22:38 +0900, Tomasz Figa wrote: > > On Wed, Aug 22, 2018 at 10:07 PM Paul Kocialkowski > > <paul.kocialkowski@bootlin.com> wrote: > > > > > > Hi, > > > > > > On Tue, 2018-08-21 at 13:07 -0400, Nicolas Dufresne wrote: > > > > Le mardi 21 août 2018 à 13:58 -0300, Ezequiel Garcia a écrit : > > > > > On Wed, 2018-06-13 at 16:07 +0200, Maxime Ripard wrote: > > > > > > From: Pawel Osciak <posciak@chromium.org> > > > > > > > > > > > > Signed-off-by: Pawel Osciak <posciak@chromium.org> > > > > > > Reviewed-by: Wu-cheng Li <wuchengli@chromium.org> > > > > > > Tested-by: Tomasz Figa <tfiga@chromium.org> > > > > > > [rebase44(groeck): include linux/types.h in v4l2-controls.h] > > > > > > Signed-off-by: Guenter Roeck <groeck@chromium.org> > > > > > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> > > > > > > --- > > > > > > > > > > > > > > > > [..] > > > > > > diff --git a/include/uapi/linux/videodev2.h > > > > > > b/include/uapi/linux/videodev2.h > > > > > > index 242a6bfa1440..4b4a1b25a0db 100644 > > > > > > --- a/include/uapi/linux/videodev2.h > > > > > > +++ b/include/uapi/linux/videodev2.h > > > > > > @@ -626,6 +626,7 @@ struct v4l2_pix_format { > > > > > > #define V4L2_PIX_FMT_H264 v4l2_fourcc('H', '2', '6', '4') /* > > > > > > H264 with start codes */ > > > > > > #define V4L2_PIX_FMT_H264_NO_SC v4l2_fourcc('A', 'V', 'C', '1') /* > > > > > > H264 without start codes */ > > > > > > #define V4L2_PIX_FMT_H264_MVC v4l2_fourcc('M', '2', '6', '4') /* > > > > > > H264 MVC */ > > > > > > +#define V4L2_PIX_FMT_H264_SLICE v4l2_fourcc('S', '2', '6', '4') /* > > > > > > H264 parsed slices */ > > > > > > > > > > As pointed out by Tomasz, the Rockchip VPU driver expects start codes > > > > > [1], so the userspace > > > > > should be aware of it. Perhaps we could document this pixel format > > > > > better as: > > > > > > > > > > #define V4L2_PIX_FMT_H264_SLICE v4l2_fourcc('S', '2', '6', '4') /* > > > > > H264 parsed slices with start codes */ > > > > > > > > > > And introduce another pixel format: > > > > > > > > > > #define V4L2_PIX_FMT_H264_SLICE_NO_SC v4l2_fourcc(TODO) /* H264 > > > > > parsed slices without start codes */ > > > > > > > > > > For cedrus to use, as it seems it doesn't need start codes. > > > > > > > > I must admit that this RK requirement is a bit weird for slice data. > > > > Though, userspace wise, always adding start-code would be compatible, > > > > as the driver can just offset to remove it. > > > > > > This would mean that the stateless API no longer takes parsed bitstream > > > data but effectively the full bitstream, which defeats the purpose of > > > the _SLICE pixel formats. > > > > > > > Not entirely. One of the purposes of the _SLICE pixel format was to > > specify it in a way that adds a requirement of providing the required > > controls by the client. > > I think we need to define what we want the stateless APIs (and these > formats) to precisely reflect conceptually. I've started discussing this > in the Request API and V4L2 capabilities thread. > > > > > Another option, because I'm not fan of adding dedicated formats for > > > > this, the RK driver could use data_offset (in mplane v4l2 buffers), > > > > just write a start code there. I like this solution because I would not > > > > be surprise if some drivers requires in fact an HW specific header, > > > > that the driver can generate as needed. > > > > > > I like this idea, because it implies that the driver should deal with > > > the specificities of the hardware, instead of making the blurrying the > > > lines of stateless API for covering these cases. > > > > The spec says > > > > "Offset in bytes to video data in the plane. Drivers must set this > > field when type refers to a capture stream, applications when it > > refers to an output stream." > > > > which would mean that user space would have to know to reserve some > > bytes at the beginning for the driver to add the start code there. (Or > > the driver memmove()ing the data forward when the buffer is queued, > > assuming that there is enough space in the buffer, but it should > > normally be the case.) > > > > Sounds like a pixel format with full bitstream data and some offsets > > to particular parts inside given inside a control might be the most > > flexible and cleanest solution. > > I can't help but think that bringing the whole bitstream over to the > kernel with a dedicated pix fmt just for the sake of having 3 start code > bytes is rather overkill anyway. > > I believe moving the data around to be the best call for this situation. > Or maybe there's a way to alloc more data *before* the bufer that is > exposed to userspace, so userspace can fill it normally and the driver > can bring-in the necessary heading start code bytes before the buffer? After thinking this over for some time, I believe it boils down to whether we can have an in-kernel library for turning H264 (and other codec) header structs back into a bitstream, if we end up with more than one driver need to do it. If that's fine, I think we're okay with having just the parsed pixel format around. Note that I didn't think about this with the Rockchip driver in mind, since it indeed only needs few bytes. Best regards, Tomasz
On Tue, Aug 28, 2018 at 5:11 PM Tomasz Figa <tfiga@chromium.org> wrote: > > On Wed, Aug 22, 2018 at 11:45 PM Paul Kocialkowski > <paul.kocialkowski@bootlin.com> wrote: > > > > Hi, > > > > On Wed, 2018-08-22 at 22:38 +0900, Tomasz Figa wrote: > > > On Wed, Aug 22, 2018 at 10:07 PM Paul Kocialkowski > > > <paul.kocialkowski@bootlin.com> wrote: > > > > > > > > Hi, > > > > > > > > On Tue, 2018-08-21 at 13:07 -0400, Nicolas Dufresne wrote: > > > > > Le mardi 21 août 2018 à 13:58 -0300, Ezequiel Garcia a écrit : > > > > > > On Wed, 2018-06-13 at 16:07 +0200, Maxime Ripard wrote: > > > > > > > From: Pawel Osciak <posciak@chromium.org> > > > > > > > > > > > > > > Signed-off-by: Pawel Osciak <posciak@chromium.org> > > > > > > > Reviewed-by: Wu-cheng Li <wuchengli@chromium.org> > > > > > > > Tested-by: Tomasz Figa <tfiga@chromium.org> > > > > > > > [rebase44(groeck): include linux/types.h in v4l2-controls.h] > > > > > > > Signed-off-by: Guenter Roeck <groeck@chromium.org> > > > > > > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> > > > > > > > --- > > > > > > > > > > > > > > > > > > > [..] > > > > > > > diff --git a/include/uapi/linux/videodev2.h > > > > > > > b/include/uapi/linux/videodev2.h > > > > > > > index 242a6bfa1440..4b4a1b25a0db 100644 > > > > > > > --- a/include/uapi/linux/videodev2.h > > > > > > > +++ b/include/uapi/linux/videodev2.h > > > > > > > @@ -626,6 +626,7 @@ struct v4l2_pix_format { > > > > > > > #define V4L2_PIX_FMT_H264 v4l2_fourcc('H', '2', '6', '4') /* > > > > > > > H264 with start codes */ > > > > > > > #define V4L2_PIX_FMT_H264_NO_SC v4l2_fourcc('A', 'V', 'C', '1') /* > > > > > > > H264 without start codes */ > > > > > > > #define V4L2_PIX_FMT_H264_MVC v4l2_fourcc('M', '2', '6', '4') /* > > > > > > > H264 MVC */ > > > > > > > +#define V4L2_PIX_FMT_H264_SLICE v4l2_fourcc('S', '2', '6', '4') /* > > > > > > > H264 parsed slices */ > > > > > > > > > > > > As pointed out by Tomasz, the Rockchip VPU driver expects start codes > > > > > > [1], so the userspace > > > > > > should be aware of it. Perhaps we could document this pixel format > > > > > > better as: > > > > > > > > > > > > #define V4L2_PIX_FMT_H264_SLICE v4l2_fourcc('S', '2', '6', '4') /* > > > > > > H264 parsed slices with start codes */ > > > > > > > > > > > > And introduce another pixel format: > > > > > > > > > > > > #define V4L2_PIX_FMT_H264_SLICE_NO_SC v4l2_fourcc(TODO) /* H264 > > > > > > parsed slices without start codes */ > > > > > > > > > > > > For cedrus to use, as it seems it doesn't need start codes. > > > > > > > > > > I must admit that this RK requirement is a bit weird for slice data. > > > > > Though, userspace wise, always adding start-code would be compatible, > > > > > as the driver can just offset to remove it. > > > > > > > > This would mean that the stateless API no longer takes parsed bitstream > > > > data but effectively the full bitstream, which defeats the purpose of > > > > the _SLICE pixel formats. > > > > > > > > > > Not entirely. One of the purposes of the _SLICE pixel format was to > > > specify it in a way that adds a requirement of providing the required > > > controls by the client. > > > > I think we need to define what we want the stateless APIs (and these > > formats) to precisely reflect conceptually. I've started discussing this > > in the Request API and V4L2 capabilities thread. > > > > > > > Another option, because I'm not fan of adding dedicated formats for > > > > > this, the RK driver could use data_offset (in mplane v4l2 buffers), > > > > > just write a start code there. I like this solution because I would not > > > > > be surprise if some drivers requires in fact an HW specific header, > > > > > that the driver can generate as needed. > > > > > > > > I like this idea, because it implies that the driver should deal with > > > > the specificities of the hardware, instead of making the blurrying the > > > > lines of stateless API for covering these cases. > > > > > > The spec says > > > > > > "Offset in bytes to video data in the plane. Drivers must set this > > > field when type refers to a capture stream, applications when it > > > refers to an output stream." > > > > > > which would mean that user space would have to know to reserve some > > > bytes at the beginning for the driver to add the start code there. (Or > > > the driver memmove()ing the data forward when the buffer is queued, > > > assuming that there is enough space in the buffer, but it should > > > normally be the case.) > > > > > > Sounds like a pixel format with full bitstream data and some offsets > > > to particular parts inside given inside a control might be the most > > > flexible and cleanest solution. > > > > I can't help but think that bringing the whole bitstream over to the > > kernel with a dedicated pix fmt just for the sake of having 3 start code > > bytes is rather overkill anyway. > > > > I believe moving the data around to be the best call for this situation. > > Or maybe there's a way to alloc more data *before* the bufer that is > > exposed to userspace, so userspace can fill it normally and the driver > > can bring-in the necessary heading start code bytes before the buffer? > > After thinking this over for some time, I believe it boils down to > whether we can have an in-kernel library for turning H264 (and other > codec) header structs back into a bitstream, if we end up with more > than one driver need to do it. If that's fine, I think we're okay with > having just the parsed pixel format around. > > Note that I didn't think about this with the Rockchip driver in mind, > since it indeed only needs few bytes. By the way, Alex posted an RFC with stateless codec interface documentation: https://patchwork.kernel.org/patch/10583233/ I think we should move any discussion there, to have everything in one place. Best regards, Tomasz
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c index cdf860c8e3d8..1f63c725bad1 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls.c +++ b/drivers/media/v4l2-core/v4l2-ctrls.c @@ -807,6 +807,11 @@ const char *v4l2_ctrl_get_name(u32 id) case V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING_LAYER:return "H264 Number of HC Layers"; case V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING_LAYER_QP: return "H264 Set QP Value for HC Layers"; + case V4L2_CID_MPEG_VIDEO_H264_SPS: return "H264 SPS"; + case V4L2_CID_MPEG_VIDEO_H264_PPS: return "H264 PPS"; + case V4L2_CID_MPEG_VIDEO_H264_SCALING_MATRIX: return "H264 Scaling Matrix"; + case V4L2_CID_MPEG_VIDEO_H264_SLICE_PARAM: return "H264 Slice Parameters"; + case V4L2_CID_MPEG_VIDEO_H264_DECODE_PARAM: return "H264 Decode Parameters"; case V4L2_CID_MPEG_VIDEO_MPEG4_I_FRAME_QP: return "MPEG4 I-Frame QP Value"; case V4L2_CID_MPEG_VIDEO_MPEG4_P_FRAME_QP: return "MPEG4 P-Frame QP Value"; case V4L2_CID_MPEG_VIDEO_MPEG4_B_FRAME_QP: return "MPEG4 B-Frame QP Value"; @@ -1272,6 +1277,21 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type, case V4L2_CID_RDS_TX_ALT_FREQS: *type = V4L2_CTRL_TYPE_U32; break; + case V4L2_CID_MPEG_VIDEO_H264_SPS: + *type = V4L2_CTRL_TYPE_H264_SPS; + break; + case V4L2_CID_MPEG_VIDEO_H264_PPS: + *type = V4L2_CTRL_TYPE_H264_PPS; + break; + case V4L2_CID_MPEG_VIDEO_H264_SCALING_MATRIX: + *type = V4L2_CTRL_TYPE_H264_SCALING_MATRIX; + break; + case V4L2_CID_MPEG_VIDEO_H264_SLICE_PARAM: + *type = V4L2_CTRL_TYPE_H264_SLICE_PARAM; + break; + case V4L2_CID_MPEG_VIDEO_H264_DECODE_PARAM: + *type = V4L2_CTRL_TYPE_H264_DECODE_PARAM; + break; case V4L2_CID_MPEG_VIDEO_MPEG2_FRAME_HDR: *type = V4L2_CTRL_TYPE_MPEG2_FRAME_HDR; break; @@ -1598,6 +1618,13 @@ static int std_validate(const struct v4l2_ctrl *ctrl, u32 idx, case V4L2_CTRL_TYPE_MPEG2_FRAME_HDR: return 0; + case V4L2_CTRL_TYPE_H264_SPS: + case V4L2_CTRL_TYPE_H264_PPS: + case V4L2_CTRL_TYPE_H264_SCALING_MATRIX: + case V4L2_CTRL_TYPE_H264_SLICE_PARAM: + case V4L2_CTRL_TYPE_H264_DECODE_PARAM: + return 0; + default: return -EINVAL; } @@ -2172,6 +2199,21 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl, case V4L2_CTRL_TYPE_U32: elem_size = sizeof(u32); break; + case V4L2_CTRL_TYPE_H264_SPS: + elem_size = sizeof(struct v4l2_ctrl_h264_sps); + break; + case V4L2_CTRL_TYPE_H264_PPS: + elem_size = sizeof(struct v4l2_ctrl_h264_pps); + break; + case V4L2_CTRL_TYPE_H264_SCALING_MATRIX: + elem_size = sizeof(struct v4l2_ctrl_h264_scaling_matrix); + break; + case V4L2_CTRL_TYPE_H264_SLICE_PARAM: + elem_size = sizeof(struct v4l2_ctrl_h264_slice_param); + break; + case V4L2_CTRL_TYPE_H264_DECODE_PARAM: + elem_size = sizeof(struct v4l2_ctrl_h264_decode_param); + break; case V4L2_CTRL_TYPE_MPEG2_FRAME_HDR: elem_size = sizeof(struct v4l2_ctrl_mpeg2_frame_hdr); break; diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h index 963c37b02363..9c223793a16a 100644 --- a/include/media/v4l2-ctrls.h +++ b/include/media/v4l2-ctrls.h @@ -41,6 +41,11 @@ struct poll_table_struct; * @p_u16: Pointer to a 16-bit unsigned value. * @p_u32: Pointer to a 32-bit unsigned value. * @p_char: Pointer to a string. + * @p_h264_sps: Pointer to a struct v4l2_ctrl_h264_sps. + * @p_h264_pps: Pointer to a struct v4l2_ctrl_h264_pps. + * @p_h264_scal_mtrx: Pointer to a struct v4l2_ctrl_h264_scaling_matrix. + * @p_h264_slice_param: Pointer to a struct v4l2_ctrl_h264_slice_param. + * @p_h264_decode_param: Pointer to a struct v4l2_ctrl_h264_decode_param. * @p: Pointer to a compound value. */ union v4l2_ctrl_ptr { @@ -50,6 +55,11 @@ union v4l2_ctrl_ptr { u16 *p_u16; u32 *p_u32; char *p_char; + struct v4l2_ctrl_h264_sps *p_h264_sps; + struct v4l2_ctrl_h264_pps *p_h264_pps; + struct v4l2_ctrl_h264_scaling_matrix *p_h264_scal_mtrx; + struct v4l2_ctrl_h264_slice_param *p_h264_slice_param; + struct v4l2_ctrl_h264_decode_param *p_h264_decode_param; void *p; }; diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h index 23da8bfa7e6f..ac307c59683c 100644 --- a/include/uapi/linux/v4l2-controls.h +++ b/include/uapi/linux/v4l2-controls.h @@ -50,6 +50,8 @@ #ifndef __LINUX_V4L2_CONTROLS_H #define __LINUX_V4L2_CONTROLS_H +#include <linux/types.h> + /* Control classes */ #define V4L2_CTRL_CLASS_USER 0x00980000 /* Old-style 'user' controls */ #define V4L2_CTRL_CLASS_MPEG 0x00990000 /* MPEG-compression controls */ @@ -531,6 +533,12 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type { }; #define V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING_LAYER (V4L2_CID_MPEG_BASE+381) #define V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING_LAYER_QP (V4L2_CID_MPEG_BASE+382) +#define V4L2_CID_MPEG_VIDEO_H264_SPS (V4L2_CID_MPEG_BASE+383) +#define V4L2_CID_MPEG_VIDEO_H264_PPS (V4L2_CID_MPEG_BASE+384) +#define V4L2_CID_MPEG_VIDEO_H264_SCALING_MATRIX (V4L2_CID_MPEG_BASE+385) +#define V4L2_CID_MPEG_VIDEO_H264_SLICE_PARAM (V4L2_CID_MPEG_BASE+386) +#define V4L2_CID_MPEG_VIDEO_H264_DECODE_PARAM (V4L2_CID_MPEG_BASE+387) + #define V4L2_CID_MPEG_VIDEO_MPEG4_I_FRAME_QP (V4L2_CID_MPEG_BASE+400) #define V4L2_CID_MPEG_VIDEO_MPEG4_P_FRAME_QP (V4L2_CID_MPEG_BASE+401) #define V4L2_CID_MPEG_VIDEO_MPEG4_B_FRAME_QP (V4L2_CID_MPEG_BASE+402) @@ -1078,6 +1086,162 @@ enum v4l2_detect_md_mode { #define V4L2_CID_DETECT_MD_THRESHOLD_GRID (V4L2_CID_DETECT_CLASS_BASE + 3) #define V4L2_CID_DETECT_MD_REGION_GRID (V4L2_CID_DETECT_CLASS_BASE + 4) +/* Complex controls */ + +#define V4L2_H264_SPS_CONSTRAINT_SET0_FLAG 0x01 +#define V4L2_H264_SPS_CONSTRAINT_SET1_FLAG 0x02 +#define V4L2_H264_SPS_CONSTRAINT_SET2_FLAG 0x04 +#define V4L2_H264_SPS_CONSTRAINT_SET3_FLAG 0x08 +#define V4L2_H264_SPS_CONSTRAINT_SET4_FLAG 0x10 +#define V4L2_H264_SPS_CONSTRAINT_SET5_FLAG 0x20 + +#define V4L2_H264_SPS_FLAG_SEPARATE_COLOUR_PLANE 0x01 +#define V4L2_H264_SPS_FLAG_QPPRIME_Y_ZERO_TRANSFORM_BYPASS 0x02 +#define V4L2_H264_SPS_FLAG_DELTA_PIC_ORDER_ALWAYS_ZERO 0x04 +#define V4L2_H264_SPS_FLAG_GAPS_IN_FRAME_NUM_VALUE_ALLOWED 0x08 +#define V4L2_H264_SPS_FLAG_FRAME_MBS_ONLY 0x10 +#define V4L2_H264_SPS_FLAG_MB_ADAPTIVE_FRAME_FIELD 0x20 +#define V4L2_H264_SPS_FLAG_DIRECT_8X8_INFERENCE 0x40 +struct v4l2_ctrl_h264_sps { + __u8 profile_idc; + __u8 constraint_set_flags; + __u8 level_idc; + __u8 seq_parameter_set_id; + __u8 chroma_format_idc; + __u8 bit_depth_luma_minus8; + __u8 bit_depth_chroma_minus8; + __u8 log2_max_frame_num_minus4; + __u8 pic_order_cnt_type; + __u8 log2_max_pic_order_cnt_lsb_minus4; + __s32 offset_for_non_ref_pic; + __s32 offset_for_top_to_bottom_field; + __u8 num_ref_frames_in_pic_order_cnt_cycle; + __s32 offset_for_ref_frame[255]; + __u8 max_num_ref_frames; + __u16 pic_width_in_mbs_minus1; + __u16 pic_height_in_map_units_minus1; + __u8 flags; +}; + +#define V4L2_H264_PPS_FLAG_ENTROPY_CODING_MODE 0x0001 +#define V4L2_H264_PPS_FLAG_BOTTOM_FIELD_PIC_ORDER_IN_FRAME_PRESENT 0x0002 +#define V4L2_H264_PPS_FLAG_WEIGHTED_PRED 0x0004 +#define V4L2_H264_PPS_FLAG_DEBLOCKING_FILTER_CONTROL_PRESENT 0x0008 +#define V4L2_H264_PPS_FLAG_CONSTRAINED_INTRA_PRED 0x0010 +#define V4L2_H264_PPS_FLAG_REDUNDANT_PIC_CNT_PRESENT 0x0020 +#define V4L2_H264_PPS_FLAG_TRANSFORM_8X8_MODE 0x0040 +#define V4L2_H264_PPS_FLAG_PIC_SCALING_MATRIX_PRESENT 0x0080 +struct v4l2_ctrl_h264_pps { + __u8 pic_parameter_set_id; + __u8 seq_parameter_set_id; + __u8 num_slice_groups_minus1; + __u8 num_ref_idx_l0_default_active_minus1; + __u8 num_ref_idx_l1_default_active_minus1; + __u8 weighted_bipred_idc; + __s8 pic_init_qp_minus26; + __s8 pic_init_qs_minus26; + __s8 chroma_qp_index_offset; + __s8 second_chroma_qp_index_offset; + __u8 flags; +}; + +struct v4l2_ctrl_h264_scaling_matrix { + __u8 scaling_list_4x4[6][16]; + __u8 scaling_list_8x8[6][64]; +}; + +struct v4l2_h264_weight_factors { + __s8 luma_weight[32]; + __s8 luma_offset[32]; + __s8 chroma_weight[32][2]; + __s8 chroma_offset[32][2]; +}; + +struct v4l2_h264_pred_weight_table { + __u8 luma_log2_weight_denom; + __u8 chroma_log2_weight_denom; + struct v4l2_h264_weight_factors weight_factors[2]; +}; + +enum v4l2_h264_slice_type { + V4L2_H264_SLICE_TYPE_P = 0, + V4L2_H264_SLICE_TYPE_B = 1, + V4L2_H264_SLICE_TYPE_I = 2, + V4L2_H264_SLICE_TYPE_SP = 3, + V4L2_H264_SLICE_TYPE_SI = 4, +}; + +#define V4L2_SLICE_FLAG_FIELD_PIC 0x01 +#define V4L2_SLICE_FLAG_BOTTOM_FIELD 0x02 +#define V4L2_SLICE_FLAG_DIRECT_SPATIAL_MV_PRED 0x04 +#define V4L2_SLICE_FLAG_SP_FOR_SWITCH 0x08 +struct v4l2_ctrl_h264_slice_param { + /* Size in bytes, including header */ + __u32 size; + /* Offset in bits to slice_data() from the beginning of this slice. */ + __u32 header_bit_size; + + __u16 first_mb_in_slice; + enum v4l2_h264_slice_type slice_type; + __u8 pic_parameter_set_id; + __u8 colour_plane_id; + __u16 frame_num; + __u16 idr_pic_id; + __u16 pic_order_cnt_lsb; + __s32 delta_pic_order_cnt_bottom; + __s32 delta_pic_order_cnt0; + __s32 delta_pic_order_cnt1; + __u8 redundant_pic_cnt; + + struct v4l2_h264_pred_weight_table pred_weight_table; + /* Size in bits of dec_ref_pic_marking() syntax element. */ + __u32 dec_ref_pic_marking_bit_size; + /* Size in bits of pic order count syntax. */ + __u32 pic_order_cnt_bit_size; + + __u8 cabac_init_idc; + __s8 slice_qp_delta; + __s8 slice_qs_delta; + __u8 disable_deblocking_filter_idc; + __s8 slice_alpha_c0_offset_div2; + __s8 slice_beta_offset_div2; + __u32 slice_group_change_cycle; + + __u8 num_ref_idx_l0_active_minus1; + __u8 num_ref_idx_l1_active_minus1; + /* Entries on each list are indices + * into v4l2_ctrl_h264_decode_param.dpb[]. */ + __u8 ref_pic_list0[32]; + __u8 ref_pic_list1[32]; + + __u8 flags; +}; + +/* If not set, this entry is unused for reference. */ +#define V4L2_H264_DPB_ENTRY_FLAG_ACTIVE 0x01 +#define V4L2_H264_DPB_ENTRY_FLAG_LONG_TERM 0x02 +struct v4l2_h264_dpb_entry { + __u32 buf_index; /* v4l2_buffer index */ + __u16 frame_num; + __u16 pic_num; + /* Note that field is indicated by v4l2_buffer.field */ + __s32 top_field_order_cnt; + __s32 bottom_field_order_cnt; + __u8 flags; /* V4L2_H264_DPB_ENTRY_FLAG_* */ +}; + +struct v4l2_ctrl_h264_decode_param { + __u32 num_slices; + __u8 idr_pic_flag; + __u8 nal_ref_idc; + __s32 top_field_order_cnt; + __s32 bottom_field_order_cnt; + __u8 ref_pic_list_p0[32]; + __u8 ref_pic_list_b0[32]; + __u8 ref_pic_list_b1[32]; + struct v4l2_h264_dpb_entry dpb[16]; +}; + struct v4l2_ctrl_mpeg2_frame_hdr { __u32 slice_len; __u32 slice_pos; diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h index 242a6bfa1440..4b4a1b25a0db 100644 --- a/include/uapi/linux/videodev2.h +++ b/include/uapi/linux/videodev2.h @@ -626,6 +626,7 @@ struct v4l2_pix_format { #define V4L2_PIX_FMT_H264 v4l2_fourcc('H', '2', '6', '4') /* H264 with start codes */ #define V4L2_PIX_FMT_H264_NO_SC v4l2_fourcc('A', 'V', 'C', '1') /* H264 without start codes */ #define V4L2_PIX_FMT_H264_MVC v4l2_fourcc('M', '2', '6', '4') /* H264 MVC */ +#define V4L2_PIX_FMT_H264_SLICE v4l2_fourcc('S', '2', '6', '4') /* H264 parsed slices */ #define V4L2_PIX_FMT_H263 v4l2_fourcc('H', '2', '6', '3') /* H263 */ #define V4L2_PIX_FMT_MPEG1 v4l2_fourcc('M', 'P', 'G', '1') /* MPEG-1 ES */ #define V4L2_PIX_FMT_MPEG2 v4l2_fourcc('M', 'P', 'G', '2') /* MPEG-2 ES */ @@ -1589,6 +1590,11 @@ struct v4l2_ext_control { __u8 __user *p_u8; __u16 __user *p_u16; __u32 __user *p_u32; + struct v4l2_ctrl_h264_sps __user *p_h264_sps; + struct v4l2_ctrl_h264_pps __user *p_h264_pps; + struct v4l2_ctrl_h264_scaling_matrix __user *p_h264_scal_mtrx; + struct v4l2_ctrl_h264_slice_param __user *p_h264_slice_param; + struct v4l2_ctrl_h264_decode_param __user *p_h264_decode_param; struct v4l2_ctrl_mpeg2_frame_hdr __user *p_mpeg2_frame_hdr; void __user *ptr; }; @@ -1635,6 +1641,11 @@ enum v4l2_ctrl_type { V4L2_CTRL_TYPE_U8 = 0x0100, V4L2_CTRL_TYPE_U16 = 0x0101, V4L2_CTRL_TYPE_U32 = 0x0102, + V4L2_CTRL_TYPE_H264_SPS = 0x0103, + V4L2_CTRL_TYPE_H264_PPS = 0x0104, + V4L2_CTRL_TYPE_H264_SCALING_MATRIX = 0x0105, + V4L2_CTRL_TYPE_H264_SLICE_PARAM = 0x0106, + V4L2_CTRL_TYPE_H264_DECODE_PARAM = 0x0107, V4L2_CTRL_TYPE_MPEG2_FRAME_HDR = 0x0109, };