Re: [linux-sunxi] [PATCH v2 1/2] media: v4l: Add definitions for the HEVC slice format and controls

[Randy Li]

On 12/12/18 8:51 PM, Paul Kocialkowski wrote:
> Hi,
>
> On Wed, 2018-12-05 at 21:59 +0100, Jernej Åkrabec wrote:
>
> > > +
> > > +#define V4L2_HEVC_DPB_ENTRY_RPS_ST_CURR_BEFORE	0x01
> > > +#define V4L2_HEVC_DPB_ENTRY_RPS_ST_CURR_AFTER	0x02
> > > +#define V4L2_HEVC_DPB_ENTRY_RPS_LT_CURR		0x03
> > > +
> > > +#define V4L2_HEVC_DPB_ENTRIES_NUM_MAX		16
> > > +
> > > +struct v4l2_hevc_dpb_entry {
> > > +	__u32	buffer_tag;
> > > +	__u8	rps;
> > > +	__u8	field_pic;
> > > +	__u16	pic_order_cnt[2];
> > > +};

Please add a property for reference index, if that rps is not used for 
this, some device would request that(not the rockchip one). And 
Rockchip's VDPU1 and VDPU2 for AVC would request a similar property.

Adding another buffer_tag for referring the memory of the motion vectors 
for each frames. Or a better method is add a meta data to echo picture 
buffer, since the picture output is just the same as the original, 
display won't care whether the motion vectors are written the button of 
picture or somewhere else.


> > > +
> > > +struct v4l2_hevc_pred_weight_table {
> > > +	__u8	luma_log2_weight_denom;
> > > +	__s8	delta_chroma_log2_weight_denom;
> > > +
> > > +	__s8	delta_luma_weight_l0[V4L2_HEVC_DPB_ENTRIES_NUM_MAX];
> > > +	__s8	luma_offset_l0[V4L2_HEVC_DPB_ENTRIES_NUM_MAX];
> > > +	__s8	delta_chroma_weight_l0[V4L2_HEVC_DPB_ENTRIES_NUM_MAX][2];
> > > +	__s8	chroma_offset_l0[V4L2_HEVC_DPB_ENTRIES_NUM_MAX][2];
> > > +
> > > +	__s8	delta_luma_weight_l1[V4L2_HEVC_DPB_ENTRIES_NUM_MAX];
> > > +	__s8	luma_offset_l1[V4L2_HEVC_DPB_ENTRIES_NUM_MAX];
> > > +	__s8	delta_chroma_weight_l1[V4L2_HEVC_DPB_ENTRIES_NUM_MAX][2];
> > > +	__s8	chroma_offset_l1[V4L2_HEVC_DPB_ENTRIES_NUM_MAX][2];
> > > +};
> > > +
Those properties I think are not necessary are applying for the 
Rockchip's device, may not work for the others.
> > > +struct v4l2_ctrl_hevc_slice_params {
> > > +	__u32	bit_size;
> > > +	__u32	data_bit_offset;
> > > +
> > > +	/* ISO/IEC 23008-2, ITU-T Rec. H.265: NAL unit header */
> > > +	__u8	nal_unit_type;
> > > +	__u8	nuh_temporal_id_plus1;
> > > +
> > > +	/* ISO/IEC 23008-2, ITU-T Rec. H.265: General slice segment header */
> > > +	__u8	slice_type;
> > > +	__u8	colour_plane_id;
----------------------------------------------------------------------------
> > > +	__u16	slice_pic_order_cnt;
> > > +	__u8	slice_sao_luma_flag;
> > > +	__u8	slice_sao_chroma_flag;
> > > +	__u8	slice_temporal_mvp_enabled_flag;
> > > +	__u8	num_ref_idx_l0_active_minus1;
> > > +	__u8	num_ref_idx_l1_active_minus1;
Rockchip's decoder doesn't use this part.
> > > +	__u8	mvd_l1_zero_flag;
> > > +	__u8	cabac_init_flag;
> > > +	__u8	collocated_from_l0_flag;
> > > +	__u8	collocated_ref_idx;
> > > +	__u8	five_minus_max_num_merge_cand;
> > > +	__u8	use_integer_mv_flag;
> > > +	__s8	slice_qp_delta;
> > > +	__s8	slice_cb_qp_offset;
> > > +	__s8	slice_cr_qp_offset;
> > > +	__s8	slice_act_y_qp_offset;
> > > +	__s8	slice_act_cb_qp_offset;
> > > +	__s8	slice_act_cr_qp_offset;
> > > +	__u8	slice_deblocking_filter_disabled_flag;
> > > +	__s8	slice_beta_offset_div2;
> > > +	__s8	slice_tc_offset_div2;
> > > +	__u8	slice_loop_filter_across_slices_enabled_flag;
> > > +
> > > +	/* ISO/IEC 23008-2, ITU-T Rec. H.265: Picture timing SEI message */
> > > +	__u8	pic_struct;
I think the decoder doesn't care about this, it is used for display.
> > > +
> > > +	/* ISO/IEC 23008-2, ITU-T Rec. H.265: General slice segment header */
> > > +	struct v4l2_hevc_dpb_entry dpb[V4L2_HEVC_DPB_ENTRIES_NUM_MAX];
> > > +	__u8	num_active_dpb_entries;
> > > +	__u8	ref_idx_l0[V4L2_HEVC_DPB_ENTRIES_NUM_MAX];
> > > +	__u8	ref_idx_l1[V4L2_HEVC_DPB_ENTRIES_NUM_MAX];
> > > +
> > > +	__u8	num_rps_poc_st_curr_before;
> > > +	__u8	num_rps_poc_st_curr_after;
> > > +	__u8	num_rps_poc_lt_curr;
> > > +
> > > +	/* ISO/IEC 23008-2, ITU-T Rec. H.265: Weighted prediction parameter */
> > > +	struct v4l2_hevc_pred_weight_table pred_weight_table;
> > > +};
> > > +
> > >   #endif