Re: [PATCH v8 1/8] media: v4l: Add definitions for MPEG2 slice format and metadata
Hi Hans, Le lundi 03 septembre 2018 à 10:32 +0200, Hans Verkuil a écrit : > This looks very nice. I have two more comments, but they can be added > using a follow-up patch (unless you need a v9 anyway): I suppose I'll send a v9 to keep things in order here. And thanks for the review! > On 08/28/2018 09:34 AM, Paul Kocialkowski wrote: > > Stateless video decoding engines require both the MPEG slices and > > associated metadata from the video stream in order to decode > > frames. > > > > This introduces definitions for a new pixel format, describing > > buffers > > with MPEG2 slice data, as well as a control structure for passing > > the > > frame metadata to drivers. > > > > This is based on work from both Florent Revest and Hugues Fruchet. > > > > Signed-off-by: Paul Kocialkowski > > --- > > .../media/uapi/v4l/extended-controls.rst | 177 > > ++ > > .../media/uapi/v4l/pixfmt-compressed.rst | 14 ++ > > .../media/uapi/v4l/vidioc-queryctrl.rst | 14 +- > > .../media/videodev2.h.rst.exceptions | 2 + > > drivers/media/v4l2-core/v4l2-ctrls.c | 63 +++ > > drivers/media/v4l2-core/v4l2-ioctl.c | 1 + > > include/media/v4l2-ctrls.h| 18 +- > > include/uapi/linux/v4l2-controls.h| 65 +++ > > include/uapi/linux/videodev2.h| 5 + > > 9 files changed, 350 insertions(+), 9 deletions(-) > > > > diff --git a/Documentation/media/uapi/v4l/extended-controls.rst > > b/Documentation/media/uapi/v4l/extended-controls.rst > > index 9f7312bf3365..a9252225b63e 100644 > > --- a/Documentation/media/uapi/v4l/extended-controls.rst > > +++ b/Documentation/media/uapi/v4l/extended-controls.rst > > @@ -1497,6 +1497,183 @@ enum > > v4l2_mpeg_video_h264_hierarchical_coding_type - > > > > > > > > +.. _v4l2-mpeg-mpeg2: > > + > > +``V4L2_CID_MPEG_VIDEO_MPEG2_SLICE_PARAMS (struct)`` > > +Specifies the slice parameters (as extracted from the > > bitstream) for the > > +associated MPEG-2 slice data. This includes the necessary > > parameters for > > +configuring a stateless hardware decoding pipeline for MPEG-2. > > +The bitstream parameters are defined according to the ISO/IEC > > 13818-2 and > > +ITU-T Rec. H.262 specifications. > > You need to use references to the specs in the biblio.rst file. > ISO/IEC 13818-2 > is there already. As far as I can see ISO/IEC 13818-2 == ITU-T Rec. > H.262, so > it's only one spec, not two. Alright, I had missed that. Will do! > > + > > +.. c:type:: v4l2_ctrl_mpeg2_slice_params > > + > > +.. cssclass:: longtable > > + > > +.. flat-table:: struct v4l2_ctrl_mpeg2_slice_params > > +:header-rows: 0 > > +:stub-columns: 0 > > +:widths: 1 1 2 > > + > > +* - __u32 > > + - ``bit_size`` > > + - Size (in bits) of the current slice data. > > +* - __u32 > > + - ``data_bit_offset`` > > + - Offset (in bits) to the video data in the current slice > > data. > > +* - struct :c:type:`v4l2_mpeg2_sequence` > > + - ``sequence`` > > + - Structure with MPEG-2 sequence metadata, merging relevant > > fields from > > + the sequence header and sequence extension parts of the > > bitstream. > > +* - struct :c:type:`v4l2_mpeg2_picture` > > + - ``picture`` > > + - Structure with MPEG-2 picture metadata, merging relevant > > fields from > > + the picture header and picture coding extension parts of the > > bitstream. > > +* - __u8 > > + - ``quantiser_scale_code`` > > + - Code used to determine the quantization scale to use for > > the IDCT. > > +* - __u8 > > + - ``backward_ref_index`` > > + - Index for the V4L2 buffer to use as backward reference, > > used with > > + B-coded and P-coded frames. > > +* - __u8 > > + - ``forward_ref_index`` > > + - Index for the V4L2 buffer to use as forward reference, > > used with > > + P-coded frames. > > + > > +.. c:type:: v4l2_mpeg2_sequence > > + > > +.. cssclass:: longtable > > + > > +.. flat-table:: struct v4l2_mpeg2_sequence > > +:header-rows: 0 > > +:stub-columns: 0 > > +:widths: 1 1 2 > > + > > +* - __u16 > > + - ``horizontal_size`` > > + - The width of the displayable part of the frame's luminance > > component. > > +* - __u16 > > + - ``vertical_size`` > > + - The height of the displayable part of the frame's > > luminance component. > > +* - __u32 > > + - ``vbv_buffer_size`` > > + - Used to calculate the required size of the video buffering > > verifier, > > + defined (in bits) as: 16 * 1024 * vbv_buffer_size. > > +* - __u8 > > + - ``profile_and_level_indication`` > > + - The current profile and level indication as extracted from > > the > > + bitstream. > > +* - __u8 > > + - ``progressive_sequence`` > > + - Indication that all the frames for the sequence are > > progressive instead > > + of interlaced. > > +* -
Re: [PATCH v8 1/8] media: v4l: Add definitions for MPEG2 slice format and metadata
This looks very nice. I have two more comments, but they can be added using a follow-up patch (unless you need a v9 anyway): On 08/28/2018 09:34 AM, Paul Kocialkowski wrote: > Stateless video decoding engines require both the MPEG slices and > associated metadata from the video stream in order to decode frames. > > This introduces definitions for a new pixel format, describing buffers > with MPEG2 slice data, as well as a control structure for passing the > frame metadata to drivers. > > This is based on work from both Florent Revest and Hugues Fruchet. > > Signed-off-by: Paul Kocialkowski > --- > .../media/uapi/v4l/extended-controls.rst | 177 ++ > .../media/uapi/v4l/pixfmt-compressed.rst | 14 ++ > .../media/uapi/v4l/vidioc-queryctrl.rst | 14 +- > .../media/videodev2.h.rst.exceptions | 2 + > drivers/media/v4l2-core/v4l2-ctrls.c | 63 +++ > drivers/media/v4l2-core/v4l2-ioctl.c | 1 + > include/media/v4l2-ctrls.h| 18 +- > include/uapi/linux/v4l2-controls.h| 65 +++ > include/uapi/linux/videodev2.h| 5 + > 9 files changed, 350 insertions(+), 9 deletions(-) > > diff --git a/Documentation/media/uapi/v4l/extended-controls.rst > b/Documentation/media/uapi/v4l/extended-controls.rst > index 9f7312bf3365..a9252225b63e 100644 > --- a/Documentation/media/uapi/v4l/extended-controls.rst > +++ b/Documentation/media/uapi/v4l/extended-controls.rst > @@ -1497,6 +1497,183 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type - > > > > +.. _v4l2-mpeg-mpeg2: > + > +``V4L2_CID_MPEG_VIDEO_MPEG2_SLICE_PARAMS (struct)`` > +Specifies the slice parameters (as extracted from the bitstream) for the > +associated MPEG-2 slice data. This includes the necessary parameters for > +configuring a stateless hardware decoding pipeline for MPEG-2. > +The bitstream parameters are defined according to the ISO/IEC 13818-2 and > +ITU-T Rec. H.262 specifications. You need to use references to the specs in the biblio.rst file. ISO/IEC 13818-2 is there already. As far as I can see ISO/IEC 13818-2 == ITU-T Rec. H.262, so it's only one spec, not two. > + > +.. c:type:: v4l2_ctrl_mpeg2_slice_params > + > +.. cssclass:: longtable > + > +.. flat-table:: struct v4l2_ctrl_mpeg2_slice_params > +:header-rows: 0 > +:stub-columns: 0 > +:widths: 1 1 2 > + > +* - __u32 > + - ``bit_size`` > + - Size (in bits) of the current slice data. > +* - __u32 > + - ``data_bit_offset`` > + - Offset (in bits) to the video data in the current slice data. > +* - struct :c:type:`v4l2_mpeg2_sequence` > + - ``sequence`` > + - Structure with MPEG-2 sequence metadata, merging relevant fields from > + the sequence header and sequence extension parts of the bitstream. > +* - struct :c:type:`v4l2_mpeg2_picture` > + - ``picture`` > + - Structure with MPEG-2 picture metadata, merging relevant fields from > + the picture header and picture coding extension parts of the bitstream. > +* - __u8 > + - ``quantiser_scale_code`` > + - Code used to determine the quantization scale to use for the IDCT. > +* - __u8 > + - ``backward_ref_index`` > + - Index for the V4L2 buffer to use as backward reference, used with > + B-coded and P-coded frames. > +* - __u8 > + - ``forward_ref_index`` > + - Index for the V4L2 buffer to use as forward reference, used with > + P-coded frames. > + > +.. c:type:: v4l2_mpeg2_sequence > + > +.. cssclass:: longtable > + > +.. flat-table:: struct v4l2_mpeg2_sequence > +:header-rows: 0 > +:stub-columns: 0 > +:widths: 1 1 2 > + > +* - __u16 > + - ``horizontal_size`` > + - The width of the displayable part of the frame's luminance component. > +* - __u16 > + - ``vertical_size`` > + - The height of the displayable part of the frame's luminance > component. > +* - __u32 > + - ``vbv_buffer_size`` > + - Used to calculate the required size of the video buffering verifier, > + defined (in bits) as: 16 * 1024 * vbv_buffer_size. > +* - __u8 > + - ``profile_and_level_indication`` > + - The current profile and level indication as extracted from the > + bitstream. > +* - __u8 > + - ``progressive_sequence`` > + - Indication that all the frames for the sequence are progressive > instead > + of interlaced. > +* - __u8 > + - ``chroma_format`` > + - The chrominance sub-sampling format (1: 4:2:0, 2: 4:2:2, 3: 4:4:4). > + > +.. c:type:: v4l2_mpeg2_picture > + > +.. cssclass:: longtable > + > +.. flat-table:: struct v4l2_mpeg2_picture > +:header-rows: 0 > +:stub-columns: 0 > +:widths: 1 1 2 > + > +* - __u8 > + - ``picture_coding_type`` > + - Picture coding type for the frame covered by the current slice > + (V4L2_MPEG2_PICTURE_CODING_TYPE_I,
