Hi,
On Mon, 2019-06-10 at 10:52 +0200, Boris Brezillon wrote:
> Some stateless decoders don't support per-slice decoding (or at least
> not in a way that would make them efficient or easy to use).
> Let's expose a menu to control and expose the supported decoding modes.
> Drivers are allowed to support only one decoding but they can support
> both too.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>
> ---
> Changes in v2:
> * Allow decoding multiple slices in per-slice decoding mode
> * Minor doc improvement/fixes
> ---
> .../media/uapi/v4l/ext-ctrls-codec.rst | 46 ++++++++++++++++++-
> drivers/media/v4l2-core/v4l2-ctrls.c | 9 ++++
> include/media/h264-ctrls.h | 13 ++++++
> 3 files changed, 67 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
> index 82547d5de250..e3b9ab73a588 100644
> --- a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
> +++ b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
> @@ -1748,6 +1748,14 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
> * - __u32
> - ``size``
> -
> + * - __u32
> + - ``start_byte_offset``
> + - Where the slice payload starts in the output buffer. Useful when
> + operating in per frame decoding mode and decoding multi-slice content.
> + In this case, the output buffer will contain more than one slice and
> + some codecs need to know where each slice starts. Note that this
> + offsets points to the beginning of the slice which is supposed to
> + contain an ANNEX B start code.
> * - __u32
> - ``header_bit_size``
> -
> @@ -1931,7 +1939,13 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
> -
> * - __u16
> - ``num_slices``
> - - Number of slices needed to decode the current frame
> + - Number of slices attached to decode the operation. When operating in
> + per-frame decoding mode (see
> + :c:type:`v4l2_mpeg_video_h264_decoding_mode`), this field should
> + be set to the number of slices needed to fully decode the frame. When
> + operating in per-slice decoding mode this field can be set to anything
> + between 1 and the remaining number of slices needed to fully decode the
> + frame.
> * - __u16
> - ``nal_ref_idc``
> - NAL reference ID value coming from the NAL Unit header
> @@ -2022,6 +2036,36 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
> - 0x00000004
> - The DPB entry is a long term reference frame
>
> +``V4L2_CID_MPEG_VIDEO_H264_DECODING_MODE (enum)``
> + Specifies the decoding mode to use. Currently exposes per slice and per
> + frame decoding but new modes might be added later on.
I would maybe formulate this as slice-based and frame-based decoding
since I feel like per-slice implies that slices have to be passed one-
by-one. This wouldn't be the case with the latest proposal for slice-
based decoding, where more than one slice can be passed at a time.
About that, I'm wondering how we could handle that in our drivers.
It will probably be something like:
device_run -+-> decode slice i -> IRQ -+-> job_finish
`----------- i++ ----------'
And I'm wondering if there could be common helpers to help implement
this, if that's needed at all.
What do you think?
Anyway if you agree with the rewording, this is:
Reviewed-by: Paul Kocialkowski <paul.kocialkowski@xxxxxxxxxxx>
Cheers,
Paul
> +
> + .. note::
> +
> + This menu control is not yet part of the public kernel API and
> + it is expected to change.
> +
> +.. c:type:: v4l2_mpeg_video_h264_decoding_mode
> +
> +.. cssclass:: longtable
> +
> +.. flat-table::
> + :header-rows: 0
> + :stub-columns: 0
> + :widths: 1 1 2
> +
> + * - ``V4L2_MPEG_VIDEO_H264_DECODING_PER_SLICE``
> + - 0
> + - The decoding is done per slice. v4l2_ctrl_h264_decode_params->num_slices
> + can be set to anything between 1 and the remaining number of slices
> + needed to fully decode a frame.
> + * - ``V4L2_MPEG_VIDEO_H264_DECODING_PER_FRAME``
> + - 1
> + - The decoding is done per frame. v4l2_ctrl_h264_decode_params->num_slices
> + can be > 1. When that happens, the output buffer should contain all
> + slices needed to decode a frame/field, each slice being prefixed by an
> + Annex B NAL header/start-code.
> +
> .. _v4l2-mpeg-mpeg2:
>
> ``V4L2_CID_MPEG_VIDEO_MPEG2_SLICE_PARAMS (struct)``
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> index 1870cecad9ae..957eb9e2ab0f 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -406,6 +406,11 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
> "Explicit",
> NULL,
> };
> + static const char * const h264_decoding_mode[] = {
> + "Per Slice",
> + "Per Frame",
> + NULL,
> + };
> static const char * const mpeg_mpeg2_level[] = {
> "Low",
> "Main",
> @@ -637,6 +642,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
> return h264_fp_arrangement_type;
> case V4L2_CID_MPEG_VIDEO_H264_FMO_MAP_TYPE:
> return h264_fmo_map_type;
> + case V4L2_CID_MPEG_VIDEO_H264_DECODING_MODE:
> + return h264_decoding_mode;
> case V4L2_CID_MPEG_VIDEO_MPEG2_LEVEL:
> return mpeg_mpeg2_level;
> case V4L2_CID_MPEG_VIDEO_MPEG2_PROFILE:
> @@ -856,6 +863,7 @@ const char *v4l2_ctrl_get_name(u32 id)
> case V4L2_CID_MPEG_VIDEO_H264_SCALING_MATRIX: return "H264 Scaling Matrix";
> case V4L2_CID_MPEG_VIDEO_H264_SLICE_PARAMS: return "H264 Slice Parameters";
> case V4L2_CID_MPEG_VIDEO_H264_DECODE_PARAMS: return "H264 Decode Parameters";
> + case V4L2_CID_MPEG_VIDEO_H264_DECODING_MODE: return "H264 Decoding Mode";
> case V4L2_CID_MPEG_VIDEO_MPEG2_LEVEL: return "MPEG2 Level";
> case V4L2_CID_MPEG_VIDEO_MPEG2_PROFILE: return "MPEG2 Profile";
> case V4L2_CID_MPEG_VIDEO_MPEG4_I_FRAME_QP: return "MPEG4 I-Frame QP Value";
> @@ -1223,6 +1231,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
> case V4L2_CID_MPEG_VIDEO_H264_VUI_SAR_IDC:
> case V4L2_CID_MPEG_VIDEO_H264_SEI_FP_ARRANGEMENT_TYPE:
> case V4L2_CID_MPEG_VIDEO_H264_FMO_MAP_TYPE:
> + case V4L2_CID_MPEG_VIDEO_H264_DECODING_MODE:
> case V4L2_CID_MPEG_VIDEO_MPEG2_LEVEL:
> case V4L2_CID_MPEG_VIDEO_MPEG2_PROFILE:
> case V4L2_CID_MPEG_VIDEO_MPEG4_LEVEL:
> diff --git a/include/media/h264-ctrls.h b/include/media/h264-ctrls.h
> index e1404d78d6ff..4572936b7c48 100644
> --- a/include/media/h264-ctrls.h
> +++ b/include/media/h264-ctrls.h
> @@ -26,6 +26,7 @@
> #define V4L2_CID_MPEG_VIDEO_H264_SCALING_MATRIX (V4L2_CID_MPEG_BASE+1002)
> #define V4L2_CID_MPEG_VIDEO_H264_SLICE_PARAMS (V4L2_CID_MPEG_BASE+1003)
> #define V4L2_CID_MPEG_VIDEO_H264_DECODE_PARAMS (V4L2_CID_MPEG_BASE+1004)
> +#define V4L2_CID_MPEG_VIDEO_H264_DECODING_MODE (V4L2_CID_MPEG_BASE+1005)
>
> /* enum v4l2_ctrl_type type values */
> #define V4L2_CTRL_TYPE_H264_SPS 0x0110
> @@ -33,6 +34,12 @@
> #define V4L2_CTRL_TYPE_H264_SCALING_MATRIX 0x0112
> #define V4L2_CTRL_TYPE_H264_SLICE_PARAMS 0x0113
> #define V4L2_CTRL_TYPE_H264_DECODE_PARAMS 0x0114
> +#define V4L2_CTRL_TYPE_H264_DECODING_MODE 0x0115
> +
> +enum v4l2_mpeg_video_h264_decoding_mode {
> + V4L2_MPEG_VIDEO_H264_DECODING_PER_SLICE,
> + V4L2_MPEG_VIDEO_H264_DECODING_PER_FRAME,
> +};
>
> #define V4L2_H264_SPS_CONSTRAINT_SET0_FLAG 0x01
> #define V4L2_H264_SPS_CONSTRAINT_SET1_FLAG 0x02
> @@ -111,6 +118,8 @@ struct v4l2_h264_pred_weight_table {
> struct v4l2_h264_weight_factors weight_factors[2];
> };
>
> +#define V4L2_H264_MAX_SLICES_PER_FRAME 16
> +
> #define V4L2_H264_SLICE_TYPE_P 0
> #define V4L2_H264_SLICE_TYPE_B 1
> #define V4L2_H264_SLICE_TYPE_I 2
> @@ -125,6 +134,10 @@ struct v4l2_h264_pred_weight_table {
> struct v4l2_ctrl_h264_slice_params {
> /* Size in bytes, including header */
> __u32 size;
> +
> + /* Offset in bytes relative to the beginning of the output buffer. */
> + __u32 start_byte_offset;
> +
> /* Offset in bits to slice_data() from the beginning of this slice. */
> __u32 header_bit_size;
>
--
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
