Re: [PATCH v2 1/2] media: uapi: Add H264 low-level decoder API compound controls.

[ayaka]

On 1/8/19 5:52 PM, Randy 'ayaka' Li wrote:
> On Thu, Nov 15, 2018 at 03:56:49PM +0100, Maxime Ripard wrote:
> > From: Pawel Osciak <posciak@xxxxxxxxxxxx>
> >
> > Stateless video codecs will require both the H264 metadata and slices in
> > order to be able to decode frames.
> >
> > This introduces the definitions for a new pixel format for H264 slices that
> > have been parsed, as well as the structures used to pass the metadata from
> > the userspace to the kernel.
> >
> > Co-Developed-by: Maxime Ripard <maxime.ripard@xxxxxxxxxxx>
> > Signed-off-by: Pawel Osciak <posciak@xxxxxxxxxxxx>
> > Signed-off-by: Guenter Roeck <groeck@xxxxxxxxxxxx>
> > Signed-off-by: Maxime Ripard <maxime.ripard@xxxxxxxxxxx>
> > ---
> >   Documentation/media/uapi/v4l/biblio.rst       |   9 +
> >   .../media/uapi/v4l/extended-controls.rst      | 364 ++++++++++++++++++
> >   .../media/uapi/v4l/pixfmt-compressed.rst      |  20 +
> >   .../media/uapi/v4l/vidioc-queryctrl.rst       |  30 ++
> >   .../media/videodev2.h.rst.exceptions          |   5 +
> >   drivers/media/v4l2-core/v4l2-ctrls.c          |  42 ++
> >   drivers/media/v4l2-core/v4l2-ioctl.c          |   1 +
> >   include/media/v4l2-ctrls.h                    |  10 +
> >   include/uapi/linux/v4l2-controls.h            | 166 ++++++++
> >   include/uapi/linux/videodev2.h                |  11 +
> >   10 files changed, 658 insertions(+)
> > +#define V4L2_H264_DPB_ENTRY_FLAG_VALID		0x01
> > +#define V4L2_H264_DPB_ENTRY_FLAG_ACTIVE		0x02
> > +#define V4L2_H264_DPB_ENTRY_FLAG_LONG_TERM	0x04
> > +
> > +struct v4l2_h264_dpb_entry {
> > +	__u32 tag;
> > +	__u16 frame_num;
> > +	__u16 pic_num;
> Although the long term reference would use picture order count
> and short term for frame num, but only one of them is used
> for a entry of a dpb.
>
> Besides, for a frame picture frame_num = pic_num * 2,
> and frame_num = pic_num * 2 + 1 for a filed.

I mistook something before and something Herman told me is wrong, I read 
the book explaining the ITU standard.

The index of a short term reference picture would be frame_num or POC 
and LongTermPicNum for long term.

But stateless hardware decoder usually don't care about whether it is 
long term or short term, as the real dpb updating or management work are 
not done by the the driver or device and decoding job would only use the 
two list(or one list for slice P) for reference pictures. So those flag 
for long term or status can be removed as well.

Stateless decoder would care about just reference index of this picture 
and maybe some extra property for the filed coded below. Keeping a 
property here for the index of a picture is enough.

> > +	/* 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];
> I would prefer to keep only two list, list0 and list 1.
> Anyway P slice just use the list0 and B would use the both.
> > +	struct v4l2_h264_dpb_entry dpb[16];
> > +};
> > +
> >   #endif
> > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> > index 173a94d2cbef..dd028e0bf306 100644
> > --- a/include/uapi/linux/videodev2.h
> > +++ b/include/uapi/linux/videodev2.h
> > @@ -643,6 +643,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     */
> > @@ -1631,6 +1632,11 @@ struct v4l2_ext_control {
> >   		__u32 __user *p_u32;
> >   		struct v4l2_ctrl_mpeg2_slice_params __user *p_mpeg2_slice_params;
> >   		struct v4l2_ctrl_mpeg2_quantization __user *p_mpeg2_quantization;
> > +		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;
> >   		void __user *ptr;
> >   	};
> >   } __attribute__ ((packed));
> > @@ -1678,6 +1684,11 @@ enum v4l2_ctrl_type {
> >   	V4L2_CTRL_TYPE_U32	     = 0x0102,
> >   	V4L2_CTRL_TYPE_MPEG2_SLICE_PARAMS = 0x0103,
> >   	V4L2_CTRL_TYPE_MPEG2_QUANTIZATION = 0x0104,
> > +	V4L2_CTRL_TYPE_H264_SPS      = 0x0105,
> > +	V4L2_CTRL_TYPE_H264_PPS      = 0x0106,
> > +	V4L2_CTRL_TYPE_H264_SCALING_MATRIX = 0x0107,
> > +	V4L2_CTRL_TYPE_H264_SLICE_PARAMS = 0x0108,
> > +	V4L2_CTRL_TYPE_H264_DECODE_PARAMS = 0x0109,
> >   };
> >   
> >   /*  Used in the VIDIOC_QUERYCTRL ioctl for querying controls */