media: hantro: Add support for H264 decoding
Subsystems (7)
v7 cover media: hantro: Add support for H264 decoding
Yet another small respin of H264 uAPI review and Hantro G1 H264 decoding support, addressing some last minute feedback from Hans.
It would be awesome if we can get this first part merged for v5.4. We have lots of changes and improvements in the pipeline for this driver!
Thanks,
Ezequiel
Boris Brezillon (3):
media: uapi: h264: Add the concept of decoding mode
media: uapi: h264: Get rid of the p0/b0/b1 ref-lists
media: hantro: Move copy_metadata() before doing a decode operation
Ezequiel Garcia (4):
media: uapi: h264: Rename pixel format
media: uapi: h264: Add the concept of start code
media: cedrus: Cleanup control initialization
media: cedrus: Specify H264 startcode and decoding mode
Hertz Wong (3):
media: hantro: Add core bits to support H264 decoding
media: hantro: Add support for H264 decoding on G1
media: hantro: Enable H264 decoding on rk3288
Rasmus Villemoes (1):
lib/sort.c: implement sort() variant taking context argument
.../media/uapi/v4l/ext-ctrls-codec.rst | 99 ++- .../media/uapi/v4l/pixfmt-compressed.rst | 11 +- drivers/media/v4l2-core/v4l2-ctrls.c | 18 + drivers/media/v4l2-core/v4l2-ioctl.c | 2 +- drivers/staging/media/hantro/Makefile | 2 + drivers/staging/media/hantro/hantro.h | 9 +- drivers/staging/media/hantro/hantro_drv.c | 50 +- .../staging/media/hantro/hantro_g1_h264_dec.c | 292 ++++++++ drivers/staging/media/hantro/hantro_h264.c | 646 ++++++++++++++++++ drivers/staging/media/hantro/hantro_hw.h | 56 ++ drivers/staging/media/hantro/hantro_v4l2.c | 10 + drivers/staging/media/hantro/rk3288_vpu_hw.c | 21 +- drivers/staging/media/sunxi/cedrus/cedrus.c | 63 +- drivers/staging/media/sunxi/cedrus/cedrus.h | 3 +- .../staging/media/sunxi/cedrus/cedrus_dec.c | 2 +- .../staging/media/sunxi/cedrus/cedrus_video.c | 6 +- include/linux/sort.h | 5 + include/media/h264-ctrls.h | 21 +- lib/sort.c | 34 +- 19 files changed, 1294 insertions(+), 56 deletions(-) create mode 100644 drivers/staging/media/hantro/hantro_g1_h264_dec.c create mode 100644 drivers/staging/media/hantro/hantro_h264.c
Signature · 1 line
2.22.0
PATCH v7 1/11 lib/sort.c: implement sort() variant taking context argument
From: Rasmus Villemoes <[email protected]>
Our list_sort() utility has always supported a context argument that is passed through to the comparison routine. Now there's a use case for the similar thing for sort().
This implements sort_r by simply extending the existing sort function in the obvious way. To avoid code duplication, we want to implement sort() in terms of sort_r(). The naive way to do that is
static int cmp_wrapper(const void *a, const void *b, const void *ctx) {
int (*real_cmp)(const void*, const void*) = ctx;
return real_cmp(a, b);
}
sort(..., cmp) { sort_r(..., cmp_wrapper, cmp) }
but this would do two indirect calls for each comparison. Instead, do as is done for the default swap functions - that only adds a cost of a single easily predicted branch to each comparison call.
Aside from introducing support for the context argument, this also serves as preparation for patches that will eliminate the indirect comparison calls in common cases.
--- include/linux/sort.h | 5 +++++ lib/sort.c | 34 ++++++++++++++++++++++++++++------ 2 files changed, 33 insertions(+), 6 deletions(-)
include/linux/sort.h +5 -0
@@ -4,6 +4,11 @@#include <linux/types.h> +void sort_r (void *base,size_t num,size_t size, +int (*cmp)(const void *,const void *,const void *), +void (*swap)(void *,void *,int ), +const void *priv); +void sort (void *base,size_t num,size_t size,int (*cmp)(const void *,const void *),void (*swap)(void *,void *,int ));
lib/sort.c +28 -6
@@ -144,6 +144,18 @@ static void do_swap(void *a, void *b, size_t size, swap_func_t swap_func)swap_func (a, b, (int )size); } +typedef int (*cmp_func_t )(const void *,const void *); +typedef int (*cmp_r_func_t )(const void *,const void *,const void *); +#define _CMP_WRAPPER ((cmp_r_func_t)0L) + +static int do_cmp (const void *a,const void *b, +cmp_r_func_t cmp,const void *priv) +{ +if (cmp == _CMP_WRAPPER) +return ((cmp_func_t)(priv))(a, b); +return cmp (a, b, priv); +} +/** * parent - given the offset of the child, find the offset of the parent. * @i: the offset of the heap element whose parent is sought. Non-zero.
@@ -171,12 +183,13 @@ static size_t parent(size_t i, unsigned int lsbit, size_t size) }/** -* sort - sort an array of elements +* sort_r - sort an array of elements * @base: pointer to data to sort * @num: number of elements * @size: size of each element * @cmp_func: pointer to comparison function * @swap_func: pointer to swap function or NULL +* @priv: third argument passed to comparison function * * This function does a heapsort on the given array. You may provide * a swap_func function if you need to do something more than a memory
@@ -188,9 +201,10 @@ static size_t parent(size_t i, unsigned int lsbit, size_t size)* O(n*n) worst-case behavior and extra memory requirements that make * it less suitable for kernel use. */ -void sort (void *base,size_t num,size_t size, -int (*cmp_func)(const void *,const void *), -void (*swap_func)(void *,void *,int size)) +void sort_r (void *base,size_t num,size_t size, +int (*cmp_func)(const void *,const void *,const void *), +void (*swap_func)(void *,void *,int size), +const void *priv) {/* pre-scale counters for performance */ size_t n = num * size, a = (num/2 ) * size;
@@ -238,12 +252,12 @@ void sort(void *base, size_t num, size_t size,* average, 3/4 worst-case.) */ for (b = a; c =2 *b + size, (d = c + size) < n;) - b =cmp_func (base + c, base + d) >=0 ? c : d; + b =do_cmp (base + c, base + d, cmp_func, priv) >=0 ? c : d;if (d == n)/* Special case last leaf with no sibling */ b = c;/* Now backtrack from "b" to the correct location for "a" */ -while (b != a &&cmp_func (base + a, base + b) >=0 ) +while (b != a &&do_cmp (base + a, base + b, cmp_func, priv) >=0 ) b =parent (b, lsbit, size); c = b;/* Where "a" belongs */ while (b != a) {/* Shift it into place */
@@ -252,4 +266,12 @@ void sort(void *base, size_t num, size_t size, } } } +EXPORT_SYMBOL (sort_r); + +void sort (void *base,size_t num,size_t size, +int (*cmp_func)(const void *,const void *), +void (*swap_func)(void *,void *,int size)) +{ +return sort_r (base, num, size, _CMP_WRAPPER, swap_func, cmp_func); +}EXPORT_SYMBOL (sort);
Signature · 1 line
2.22.0
PATCH v7 2/11 media: uapi: h264: Rename pixel format
In reply to Ezequiel GarciaThe V4L2_PIX_FMT_H264_SLICE_RAW name was originally suggested because the pixel format would represent H264 slices without any start code.
However, as we will now introduce a start code menu control, give the pixel format a more meaningful name, while it's still early enough to do so.
---
Changes in v7:
* None.
Changes in v6:
* None.
Changes in v5:
* None.
Changes in v4:
* New patch.
--- Documentation/media/uapi/v4l/pixfmt-compressed.rst | 4 ++-- drivers/media/v4l2-core/v4l2-ioctl.c | 2 +- drivers/staging/media/sunxi/cedrus/cedrus_dec.c | 2 +- drivers/staging/media/sunxi/cedrus/cedrus_video.c | 6 +++--- include/media/h264-ctrls.h | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-)
Documentation/media/uapi/v4l/pixfmt-compressed.rst +2 -2
@@ -52,9 +52,9 @@ Compressed Formats - ``V4L2_PIX_FMT_H264_MVC`` - 'M264' - H264 MVC video elementary stream. - * .. _V4L2-PIX-FMT-H264-SLICE-RAW: + * .. _V4L2-PIX-FMT-H264-SLICE: - - ``V4L2_PIX_FMT_H264_SLICE_RAW`` + - ``V4L2_PIX_FMT_H264_SLICE`` - 'S264' - H264 parsed slice data, without the start code and as extracted from the H264 bitstream. This format is adapted for
drivers/media/v4l2-core/v4l2-ioctl.c +1 -1
@@ -1343,7 +1343,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)case V4L2_PIX_FMT_H264 : descr ="H.264" ;break ;case V4L2_PIX_FMT_H264_NO_SC : descr ="H.264 (No Start Codes)" ;break ;case V4L2_PIX_FMT_H264_MVC : descr ="H.264 MVC" ;break ; -case V4L2_PIX_FMT_H264_SLICE_RAW : descr ="H.264 Parsed Slice Data" ;break ; +case V4L2_PIX_FMT_H264_SLICE : descr ="H.264 Parsed Slice Data" ;break ;case V4L2_PIX_FMT_H263 : descr ="H.263" ;break ;case V4L2_PIX_FMT_MPEG1 : descr ="MPEG-1 ES" ;break ;case V4L2_PIX_FMT_MPEG2 : descr ="MPEG-2 ES" ;break ;
drivers/staging/media/sunxi/cedrus/cedrus_dec.c +1 -1
@@ -46,7 +46,7 @@ void cedrus_device_run(void *priv) V4L2_CID_MPEG_VIDEO_MPEG2_QUANTIZATION);break ; -case V4L2_PIX_FMT_H264_SLICE_RAW : +case V4L2_PIX_FMT_H264_SLICE : run.h264.decode_params =cedrus_find_control_data (ctx,V4L2_CID_MPEG_VIDEO_H264_DECODE_PARAMS ); run.h264.pps = cedrus_find_control_data(ctx,
drivers/staging/media/sunxi/cedrus/cedrus_video.c +3 -3
@@ -38,7 +38,7 @@ static struct cedrus_format cedrus_formats[] = { .directions = CEDRUS_DECODE_SRC, }, { - .pixelformat = V4L2_PIX_FMT_H264_SLICE_RAW, + .pixelformat = V4L2_PIX_FMT_H264_SLICE, .directions = CEDRUS_DECODE_SRC, }, {
@@ -104,7 +104,7 @@ static void cedrus_prepare_format(struct v4l2_pix_format *pix_fmt)switch (pix_fmt->pixelformat) {case V4L2_PIX_FMT_MPEG2_SLICE : -case V4L2_PIX_FMT_H264_SLICE_RAW : +case V4L2_PIX_FMT_H264_SLICE :/* Zero bytes per line for encoded source. */ bytesperline =0 ;
@@ -449,7 +449,7 @@ static int cedrus_start_streaming(struct vb2_queue *vq, unsigned int count) ctx->current_codec =CEDRUS_CODEC_MPEG2 ;break ; -case V4L2_PIX_FMT_H264_SLICE_RAW : +case V4L2_PIX_FMT_H264_SLICE : ctx->current_codec =CEDRUS_CODEC_H264 ;break ;
include/media/h264-ctrls.h +1 -1
@@ -14,7 +14,7 @@#include <linux/videodev2.h> /* Our pixel format isn't stable at the moment */ -#define V4L2_PIX_FMT_H264_SLICE_RAW v4l2_fourcc('S', '2', '6', '4')/* H264 parsed slices */ +#define V4L2_PIX_FMT_H264_SLICE v4l2_fourcc('S', '2', '6', '4')/* H264 parsed slices */ /* * This is put insanely high to avoid conflicting with controls that
Signature · 1 line
2.22.0
Re: [PATCH v7 02/11] media: uapi: h264: Rename pixel format
Hi,
Quoted 7 lines · Ezequiel Garcia
The
V4L2_PIX_FMT_H264_SLICE_RAWname was originally suggested because the pixel format would represent H264 slices without any start code.However, as we will now introduce a start code menu control, give the pixel format a more meaningful name, while it's still early enough to do so.
I definitely agree that SLICE_RAW is not the suffix we are looking for, but I'm not sure that _SLICE is self-describing given that we can operate either per-frame or per-slice, and _SLICE sort of implies the latter. Also, VP8 uses _FRAME to clearly indicate that it operates per-frame.
In addition, the _SLICE suffix is used by MPEG-2 in the stable API. Since we certainly want MPEG-2 to allow per-slice and per-frame decoding as well as H.264 and that the _SLICE format is specified to be the broken "concatenated slices" that cedrus expects, we probably want to use another suffix. This way, we could deprecated MPEG2_SLICE and introduce a new format for MPEG-2 that would have consistent naming with the other mpeg formats.
One suggestion I had was to call it H264_PARSED (and apply this to MPEG-2 and HEVC when similar controls to H.264 are set in place for them). I think Hans had another suggestion for the name but I don't recall what it was at this point.
Either way, if this has to be some debate, we could perhaps take it off your series and stay with SLICE_RAW for now, as long as we do rename it before making the API stable.
What do you think?
Cheers,
Paul
Quoted 108 lines
Signed-off-by Ezequiel Garcia <[email protected]>Tested-by Philipp Zabel <[email protected]>---
Changes in v7:
* None.
Changes in v6:
* None.
Changes in v5:
* None.
Changes in v4:
* New patch.--- Documentation/media/uapi/v4l/pixfmt-compressed.rst | 4 ++-- drivers/media/v4l2-core/v4l2-ioctl.c | 2 +- drivers/staging/media/sunxi/cedrus/cedrus_dec.c | 2 +- drivers/staging/media/sunxi/cedrus/cedrus_video.c | 6 +++--- include/media/h264-ctrls.h | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-)Documentation/media/uapi/v4l/pixfmt-compressed.rst +2
-2@@ -52,9 +52,9 @@ Compressed Formats - ``V4L2_PIX_FMT_H264_MVC`` - 'M264' - H264 MVC video elementary stream. - * .. _V4L2-PIX-FMT-H264-SLICE-RAW: + * .. _V4L2-PIX-FMT-H264-SLICE: - - ``V4L2_PIX_FMT_H264_SLICE_RAW`` + - ``V4L2_PIX_FMT_H264_SLICE`` - 'S264' - H264 parsed slice data, without the start code and as extracted from the H264 bitstream. This format is adapted fordrivers/media/v4l2-core/v4l2-ioctl.c +1
-1@@ -1343,7 +1343,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)case V4L2_PIX_FMT_H264 : descr ="H.264" ;break ;case V4L2_PIX_FMT_H264_NO_SC : descr ="H.264 (No Start Codes)" ;break ;case V4L2_PIX_FMT_H264_MVC : descr ="H.264 MVC" ;break ; -case V4L2_PIX_FMT_H264_SLICE_RAW : descr ="H.264 Parsed Slice Data" ;break ; +case V4L2_PIX_FMT_H264_SLICE : descr ="H.264 Parsed Slice Data" ;break ;case V4L2_PIX_FMT_H263 : descr ="H.263" ;break ;case V4L2_PIX_FMT_MPEG1 : descr ="MPEG-1 ES" ;break ;case V4L2_PIX_FMT_MPEG2 : descr ="MPEG-2 ES" ;break ;drivers/staging/media/sunxi/cedrus/cedrus_dec.c +1
-1@@ -46,7 +46,7 @@ void cedrus_device_run(void *priv) V4L2_CID_MPEG_VIDEO_MPEG2_QUANTIZATION);break ; -case V4L2_PIX_FMT_H264_SLICE_RAW : +case V4L2_PIX_FMT_H264_SLICE : run.h264.decode_params =cedrus_find_control_data (ctx,V4L2_CID_MPEG_VIDEO_H264_DECODE_PARAMS ); run.h264.pps = cedrus_find_control_data(ctx,drivers/staging/media/sunxi/cedrus/cedrus_video.c +3
-3@@ -38,7 +38,7 @@ static struct cedrus_format cedrus_formats[] = { .directions = CEDRUS_DECODE_SRC, }, { - .pixelformat = V4L2_PIX_FMT_H264_SLICE_RAW, + .pixelformat = V4L2_PIX_FMT_H264_SLICE, .directions = CEDRUS_DECODE_SRC, }, {@@ -104,7 +104,7 @@ static void cedrus_prepare_format(struct v4l2_pix_format *pix_fmt)switch (pix_fmt->pixelformat) {case V4L2_PIX_FMT_MPEG2_SLICE : -case V4L2_PIX_FMT_H264_SLICE_RAW : +case V4L2_PIX_FMT_H264_SLICE :/* Zero bytes per line for encoded source. */ bytesperline =0 ;@@ -449,7 +449,7 @@ static int cedrus_start_streaming(struct vb2_queue *vq, unsigned int count) ctx->current_codec =CEDRUS_CODEC_MPEG2 ;break ; -case V4L2_PIX_FMT_H264_SLICE_RAW : +case V4L2_PIX_FMT_H264_SLICE : ctx->current_codec =CEDRUS_CODEC_H264 ;break ;include/media/h264-ctrls.h +1
-1@@ -14,7 +14,7 @@#include <linux/videodev2.h> /* Our pixel format isn't stable at the moment */ -#define V4L2_PIX_FMT_H264_SLICE_RAW v4l2_fourcc('S', '2', '6', '4')/* H264 parsed slices */ +#define V4L2_PIX_FMT_H264_SLICE v4l2_fourcc('S', '2', '6', '4')/* H264 parsed slices */ /* * This is put insanely high to avoid conflicting with controls that Signature · 1 line
2.22.0
Signature · 3 lines
Paul Kocialkowski, Bootlin Embedded Linux and kernel engineering https://bootlin.com
Re: [PATCH v7 02/11] media: uapi: h264: Rename pixel format
Quoted 7 lines · Paul Kocialkowski
Hi,
Quoted 4 lines · Ezequiel Garcia
The
V4L2_PIX_FMT_H264_SLICE_RAWname was originally suggested because the pixel format would represent H264 slices without any start code.
Quoted 7 lines
Quoted 3 lines
However, as we will now introduce a start code menu control, give the pixel format a more meaningful name, while it's still early enough to do so.
I definitely agree that
SLICE_RAWis not the suffix we are looking for, but I'm not sure that_SLICEis self-describing given that we can operate either per-frame or per-slice, and_SLICEsort of implies the latter.
This is not entirely so, per-frame or per-slice mode refer to the granularity of the stateless API, the pixel format is still composed of H264 NALU slices.
As per the documentation, the decode_mode and the start_code are modifiers of the pixel format. As long as this is properly speced, the V4L2_PIX_FMT_H264_SLICE looks pretty OK to me.
Quoted 3 lines
Also, VP8 uses
_FRAMEto clearly indicate that it operates per-frame.
This is because VP8 doesn't have any concept of slices, the encoded unit is a video frame.
Quoted 11 lines
In addition, the
_SLICEsuffix is used by MPEG-2 in the stable API. Since we certainly want MPEG-2 to allow per-slice and per-frame decoding as well as H.264 and that the_SLICEformat is specified to be the broken "concatenated slices" that cedrus expects, we probably want to use another suffix. This way, we could deprecatedMPEG2_SLICEand introduce a new format for MPEG-2 that would have consistent naming with the other mpeg formats.One suggestion I had was to call it
H264_PARSED(and apply this to MPEG-2 and HEVC when similar controls to H.264 are set in place for them). I think Hans had another suggestion for the name but I don't recall what it was at this point.
Quoted 6 lines
Either way, if this has to be some debate, we could perhaps take it off your series and stay with
SLICE_RAWfor now, as long as we do rename it before making the API stable.What do you think?
With the new pixel format modifiers (decode_mode and start_code), the _RAW suffix now has no meaning. Now, we could name it _PARSED or _SLICE. As long as this is properly documented (as it is now), that'd be fine.
Now, to be completely honest, this discussion sounds like bikeshedding to me.
Thanks,
Eze
Re: [PATCH v7 02/11] media: uapi: h264: Rename pixel format
In reply to Paul KocialkowskiQuoted 7 lines · Paul Kocialkowski
Hi,
Quoted 4 lines · Ezequiel Garcia
The
V4L2_PIX_FMT_H264_SLICE_RAWname was originally suggested because the pixel format would represent H264 slices without any start code.
Quoted 8 lines
Quoted 3 lines
However, as we will now introduce a start code menu control, give the pixel format a more meaningful name, while it's still early enough to do so.
I definitely agree that
SLICE_RAWis not the suffix we are looking for, but I'm not sure that_SLICEis self-describing given that we can operate either per-frame or per-slice, and_SLICEsort of implies the latter. Also, VP8 uses_FRAMEto clearly indicate that it operates per-frame.
Well, VP8 doesn't support slices at all.
Quoted 2 lines
In addition, the
_SLICEsuffix is used by MPEG-2 in the stable API. Since we
Regarding MPEG-2: while it has a concept of slices, it is my understanding that you never process slices separately, but only full pictures. I may be wrong here.
Quoted 5 lines
certainly want MPEG-2 to allow per-slice and per-frame decoding as well as H.264 and that the
_SLICEformat is specified to be the broken "concatenated slices" that cedrus expects, we probably want to use another suffix. This way, we could deprecatedMPEG2_SLICEand introduce a new format for MPEG-2 that would have consistent naming with the other mpeg formats.
I actually think that H264_SLICE is a decent name.
I'm less sure about MPEG2_SLICE since I am not sure if it means the same as a H264 slice.
Quoted 4 lines
One suggestion I had was to call it
H264_PARSED(and apply this to MPEG-2 and HEVC when similar controls to H.264 are set in place for them). I think Hans had another suggestion for the name but I don't recall what it was at this point.
I can't remember it either. In any case, I'm not that keen on _PARSED.
I think for H264 and HEVC the _SLICE suffix is good enough.
Regards,
Hans
Quoted 120 lines
Either way, if this has to be some debate, we could perhaps take it off your series and stay with
SLICE_RAWfor now, as long as we do rename it before making the API stable.What do you think?
Cheers,
Paul
Quoted 108 lines
Signed-off-by Ezequiel Garcia <[email protected]>Tested-by Philipp Zabel <[email protected]>---
Changes in v7:
* None.
Changes in v6:
* None.
Changes in v5:
* None.
Changes in v4:
* New patch.--- Documentation/media/uapi/v4l/pixfmt-compressed.rst | 4 ++-- drivers/media/v4l2-core/v4l2-ioctl.c | 2 +- drivers/staging/media/sunxi/cedrus/cedrus_dec.c | 2 +- drivers/staging/media/sunxi/cedrus/cedrus_video.c | 6 +++--- include/media/h264-ctrls.h | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-)Documentation/media/uapi/v4l/pixfmt-compressed.rst +2
-2@@ -52,9 +52,9 @@ Compressed Formats - ``V4L2_PIX_FMT_H264_MVC`` - 'M264' - H264 MVC video elementary stream. - * .. _V4L2-PIX-FMT-H264-SLICE-RAW: + * .. _V4L2-PIX-FMT-H264-SLICE: - - ``V4L2_PIX_FMT_H264_SLICE_RAW`` + - ``V4L2_PIX_FMT_H264_SLICE`` - 'S264' - H264 parsed slice data, without the start code and as extracted from the H264 bitstream. This format is adapted fordrivers/media/v4l2-core/v4l2-ioctl.c +1
-1@@ -1343,7 +1343,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)case V4L2_PIX_FMT_H264 : descr ="H.264" ;break ;case V4L2_PIX_FMT_H264_NO_SC : descr ="H.264 (No Start Codes)" ;break ;case V4L2_PIX_FMT_H264_MVC : descr ="H.264 MVC" ;break ; -case V4L2_PIX_FMT_H264_SLICE_RAW : descr ="H.264 Parsed Slice Data" ;break ; +case V4L2_PIX_FMT_H264_SLICE : descr ="H.264 Parsed Slice Data" ;break ;case V4L2_PIX_FMT_H263 : descr ="H.263" ;break ;case V4L2_PIX_FMT_MPEG1 : descr ="MPEG-1 ES" ;break ;case V4L2_PIX_FMT_MPEG2 : descr ="MPEG-2 ES" ;break ;drivers/staging/media/sunxi/cedrus/cedrus_dec.c +1
-1@@ -46,7 +46,7 @@ void cedrus_device_run(void *priv) V4L2_CID_MPEG_VIDEO_MPEG2_QUANTIZATION);break ; -case V4L2_PIX_FMT_H264_SLICE_RAW : +case V4L2_PIX_FMT_H264_SLICE : run.h264.decode_params =cedrus_find_control_data (ctx,V4L2_CID_MPEG_VIDEO_H264_DECODE_PARAMS ); run.h264.pps = cedrus_find_control_data(ctx,drivers/staging/media/sunxi/cedrus/cedrus_video.c +3
-3@@ -38,7 +38,7 @@ static struct cedrus_format cedrus_formats[] = { .directions = CEDRUS_DECODE_SRC, }, { - .pixelformat = V4L2_PIX_FMT_H264_SLICE_RAW, + .pixelformat = V4L2_PIX_FMT_H264_SLICE, .directions = CEDRUS_DECODE_SRC, }, {@@ -104,7 +104,7 @@ static void cedrus_prepare_format(struct v4l2_pix_format *pix_fmt)switch (pix_fmt->pixelformat) {case V4L2_PIX_FMT_MPEG2_SLICE : -case V4L2_PIX_FMT_H264_SLICE_RAW : +case V4L2_PIX_FMT_H264_SLICE :/* Zero bytes per line for encoded source. */ bytesperline =0 ;@@ -449,7 +449,7 @@ static int cedrus_start_streaming(struct vb2_queue *vq, unsigned int count) ctx->current_codec =CEDRUS_CODEC_MPEG2 ;break ; -case V4L2_PIX_FMT_H264_SLICE_RAW : +case V4L2_PIX_FMT_H264_SLICE : ctx->current_codec =CEDRUS_CODEC_H264 ;break ;include/media/h264-ctrls.h +1
-1@@ -14,7 +14,7 @@#include <linux/videodev2.h> /* Our pixel format isn't stable at the moment */ -#define V4L2_PIX_FMT_H264_SLICE_RAW v4l2_fourcc('S', '2', '6', '4')/* H264 parsed slices */ +#define V4L2_PIX_FMT_H264_SLICE v4l2_fourcc('S', '2', '6', '4')/* H264 parsed slices */ /* * This is put insanely high to avoid conflicting with controls that Signature · 1 line
2.22.0
Re: [PATCH v7 02/11] media: uapi: h264: Rename pixel format
Hi,
Quoted 17 lines · Hans Verkuil
Quoted 7 lines · Paul Kocialkowski
Hi,
Quoted 4 lines · Ezequiel Garcia
The
V4L2_PIX_FMT_H264_SLICE_RAWname was originally suggested because the pixel format would represent H264 slices without any start code.Quoted 8 lines
Quoted 3 lines
However, as we will now introduce a start code menu control, give the pixel format a more meaningful name, while it's still early enough to do so.
I definitely agree that
SLICE_RAWis not the suffix we are looking for, but I'm not sure that_SLICEis self-describing given that we can operate either per-frame or per-slice, and_SLICEsort of implies the latter. Also, VP8 uses_FRAMEto clearly indicate that it operates per-frame.
Quoted 8 lines
Well, VP8 doesn't support slices at all.
Quoted 2 lines
In addition, the
_SLICEsuffix is used by MPEG-2 in the stable API. Since weRegarding MPEG-2: while it has a concept of slices, it is my understanding that you never process slices separately, but only full pictures. I may be wrong here.
I don't think that is the case since ffmpeg clearly implements decoding on a per-slice basis (mpeg_decode_slice).
Information is also passed on a per-slice basis to VAAPI (vaapi_mpeg2_decode_slice) with a distinct data buffer and slice parameter buffer for each slice. Among other things, it contains the vertical and horizontal positions for the slice, which we can set in the hardware.
Quoted 10 lines
Quoted 5 lines
certainly want MPEG-2 to allow per-slice and per-frame decoding as well as H.264 and that the
_SLICEformat is specified to be the broken "concatenated slices" that cedrus expects, we probably want to use another suffix. This way, we could deprecatedMPEG2_SLICEand introduce a new format for MPEG-2 that would have consistent naming with the other mpeg formats.I actually think that
H264_SLICEis a decent name.I'm less sure about
MPEG2_SLICEsince I am not sure if it means the same as a H264 slice.
The main problem I see is that we have already specified MPEG2_SLICE in a way that is incompatible with the future improvments we want to bring to the API:
" The output buffer must contain the appropriate number of macroblocks to decode a full corresponding frame to the matching capture buffer."
So I only see two possibilities: either we decide to change the specification of the pixel format and we can keep using the _SLICE suffix, either we need to introduce a new pixel format with another suffix, which should also be reflected on other MPEG formats for consistency. Then we can deprecate MPEG2_SLICE and have drivers stop using it.
What do you think?
Cheers,
Paul
Quoted 133 lines
Quoted 3 lines
One suggestion I had was to call it
H264_PARSED(and apply this to MPEG-2 and HEVC when similar controls to H.264 are set in place for them). I think Hans had another suggestion for the name but I don't recall what it was at this point.I can't remember it either. In any case, I'm not that keen on
_PARSED.I think for H264 and HEVC the
_SLICEsuffix is good enough.Regards,
Hans
Quoted 120 lines
Either way, if this has to be some debate, we could perhaps take it off your series and stay with
SLICE_RAWfor now, as long as we do rename it before making the API stable.What do you think?
Cheers,
Paul
Quoted 108 lines
Signed-off-by Ezequiel Garcia <[email protected]>Tested-by Philipp Zabel <[email protected]>---
Changes in v7:
* None.
Changes in v6:
* None.
Changes in v5:
* None.
Changes in v4:
* New patch.--- Documentation/media/uapi/v4l/pixfmt-compressed.rst | 4 ++-- drivers/media/v4l2-core/v4l2-ioctl.c | 2 +- drivers/staging/media/sunxi/cedrus/cedrus_dec.c | 2 +- drivers/staging/media/sunxi/cedrus/cedrus_video.c | 6 +++--- include/media/h264-ctrls.h | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-)Documentation/media/uapi/v4l/pixfmt-compressed.rst +2
-2@@ -52,9 +52,9 @@ Compressed Formats - ``V4L2_PIX_FMT_H264_MVC`` - 'M264' - H264 MVC video elementary stream. - * .. _V4L2-PIX-FMT-H264-SLICE-RAW: + * .. _V4L2-PIX-FMT-H264-SLICE: - - ``V4L2_PIX_FMT_H264_SLICE_RAW`` + - ``V4L2_PIX_FMT_H264_SLICE`` - 'S264' - H264 parsed slice data, without the start code and as extracted from the H264 bitstream. This format is adapted fordrivers/media/v4l2-core/v4l2-ioctl.c +1
-1@@ -1343,7 +1343,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)case V4L2_PIX_FMT_H264 : descr ="H.264" ;break ;case V4L2_PIX_FMT_H264_NO_SC : descr ="H.264 (No Start Codes)" ;break ;case V4L2_PIX_FMT_H264_MVC : descr ="H.264 MVC" ;break ; -case V4L2_PIX_FMT_H264_SLICE_RAW : descr ="H.264 Parsed Slice Data" ;break ; +case V4L2_PIX_FMT_H264_SLICE : descr ="H.264 Parsed Slice Data" ;break ;case V4L2_PIX_FMT_H263 : descr ="H.263" ;break ;case V4L2_PIX_FMT_MPEG1 : descr ="MPEG-1 ES" ;break ;case V4L2_PIX_FMT_MPEG2 : descr ="MPEG-2 ES" ;break ;drivers/staging/media/sunxi/cedrus/cedrus_dec.c +1
-1@@ -46,7 +46,7 @@ void cedrus_device_run(void *priv) V4L2_CID_MPEG_VIDEO_MPEG2_QUANTIZATION);break ; -case V4L2_PIX_FMT_H264_SLICE_RAW : +case V4L2_PIX_FMT_H264_SLICE : run.h264.decode_params =cedrus_find_control_data (ctx,V4L2_CID_MPEG_VIDEO_H264_DECODE_PARAMS ); run.h264.pps = cedrus_find_control_data(ctx,drivers/staging/media/sunxi/cedrus/cedrus_video.c +3
-3@@ -38,7 +38,7 @@ static struct cedrus_format cedrus_formats[] = { .directions = CEDRUS_DECODE_SRC, }, { - .pixelformat = V4L2_PIX_FMT_H264_SLICE_RAW, + .pixelformat = V4L2_PIX_FMT_H264_SLICE, .directions = CEDRUS_DECODE_SRC, }, {@@ -104,7 +104,7 @@ static void cedrus_prepare_format(struct v4l2_pix_format *pix_fmt)switch (pix_fmt->pixelformat) {case V4L2_PIX_FMT_MPEG2_SLICE : -case V4L2_PIX_FMT_H264_SLICE_RAW : +case V4L2_PIX_FMT_H264_SLICE :/* Zero bytes per line for encoded source. */ bytesperline =0 ;@@ -449,7 +449,7 @@ static int cedrus_start_streaming(struct vb2_queue *vq, unsigned int count) ctx->current_codec =CEDRUS_CODEC_MPEG2 ;break ; -case V4L2_PIX_FMT_H264_SLICE_RAW : +case V4L2_PIX_FMT_H264_SLICE : ctx->current_codec =CEDRUS_CODEC_H264 ;break ;include/media/h264-ctrls.h +1
-1@@ -14,7 +14,7 @@#include <linux/videodev2.h> /* Our pixel format isn't stable at the moment */ -#define V4L2_PIX_FMT_H264_SLICE_RAW v4l2_fourcc('S', '2', '6', '4')/* H264 parsed slices */ +#define V4L2_PIX_FMT_H264_SLICE v4l2_fourcc('S', '2', '6', '4')/* H264 parsed slices */ /* * This is put insanely high to avoid conflicting with controls that Signature · 1 line
2.22.0
Signature · 3 lines
Paul Kocialkowski, Bootlin Embedded Linux and kernel engineering https://bootlin.com
Re: [PATCH v7 02/11] media: uapi: h264: Rename pixel format
Quoted 53 lines · Paul Kocialkowski
Hi,
Quoted 17 lines · Hans Verkuil
Quoted 7 lines · Paul Kocialkowski
Hi,
Quoted 4 lines · Ezequiel Garcia
The
V4L2_PIX_FMT_H264_SLICE_RAWname was originally suggested because the pixel format would represent H264 slices without any start code.Quoted 8 lines
Quoted 3 lines
However, as we will now introduce a start code menu control, give the pixel format a more meaningful name, while it's still early enough to do so.
I definitely agree that
SLICE_RAWis not the suffix we are looking for, but I'm not sure that_SLICEis self-describing given that we can operate either per-frame or per-slice, and_SLICEsort of implies the latter. Also, VP8 uses_FRAMEto clearly indicate that it operates per-frame.Quoted 8 lines
Well, VP8 doesn't support slices at all.
Quoted 2 lines
In addition, the
_SLICEsuffix is used by MPEG-2 in the stable API. Since weRegarding MPEG-2: while it has a concept of slices, it is my understanding that you never process slices separately, but only full pictures. I may be wrong here.
I don't think that is the case since ffmpeg clearly implements decoding on a per-slice basis (
mpeg_decode_slice).Information is also passed on a per-slice basis to VAAPI (
vaapi_mpeg2_decode_slice) with a distinct data buffer and slice parameter buffer for each slice. Among other things, it contains the vertical and horizontal positions for the slice, which we can set in the hardware.Quoted 10 lines
Quoted 5 lines
certainly want MPEG-2 to allow per-slice and per-frame decoding as well as H.264 and that the
_SLICEformat is specified to be the broken "concatenated slices" that cedrus expects, we probably want to use another suffix. This way, we could deprecatedMPEG2_SLICEand introduce a new format for MPEG-2 that would have consistent naming with the other mpeg formats.I actually think that
H264_SLICEis a decent name.I'm less sure about
MPEG2_SLICEsince I am not sure if it means the same as a H264 slice.The main problem I see is that we have already specified
MPEG2_SLICEin a way that is incompatible with the future improvments we want to bring to the API:
" The output buffer must contain the appropriate number of macroblocks to decode a full corresponding frame to the matching capture buffer."
Quoted 7 lines
So I only see two possibilities: either we decide to change the specification of the pixel format and we can keep using the
_SLICEsuffix, either we need to introduce a new pixel format with another suffix, which should also be reflected on other MPEG formats for consistency. Then we can deprecateMPEG2_SLICEand have drivers stop using it.What do you think?
I'd change the specification of the pixel format. So MPEG2_SLICE now supports multiple slices if the hardware supports it as well.
We would need an MPEG2_DECODING_MODE control as well, that currently would read FRAME based only.
Regards,
Hans
Quoted 139 lines
Cheers,
Paul
Quoted 133 lines
Quoted 3 lines
One suggestion I had was to call it
H264_PARSED(and apply this to MPEG-2 and HEVC when similar controls to H.264 are set in place for them). I think Hans had another suggestion for the name but I don't recall what it was at this point.I can't remember it either. In any case, I'm not that keen on
_PARSED.I think for H264 and HEVC the
_SLICEsuffix is good enough.Regards,
Hans
Quoted 120 lines
Either way, if this has to be some debate, we could perhaps take it off your series and stay with
SLICE_RAWfor now, as long as we do rename it before making the API stable.What do you think?
Cheers,
Paul
Quoted 108 lines
Signed-off-by Ezequiel Garcia <[email protected]>Tested-by Philipp Zabel <[email protected]>---
Changes in v7:
* None.
Changes in v6:
* None.
Changes in v5:
* None.
Changes in v4:
* New patch.--- Documentation/media/uapi/v4l/pixfmt-compressed.rst | 4 ++-- drivers/media/v4l2-core/v4l2-ioctl.c | 2 +- drivers/staging/media/sunxi/cedrus/cedrus_dec.c | 2 +- drivers/staging/media/sunxi/cedrus/cedrus_video.c | 6 +++--- include/media/h264-ctrls.h | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-)Documentation/media/uapi/v4l/pixfmt-compressed.rst +2
-2@@ -52,9 +52,9 @@ Compressed Formats - ``V4L2_PIX_FMT_H264_MVC`` - 'M264' - H264 MVC video elementary stream. - * .. _V4L2-PIX-FMT-H264-SLICE-RAW: + * .. _V4L2-PIX-FMT-H264-SLICE: - - ``V4L2_PIX_FMT_H264_SLICE_RAW`` + - ``V4L2_PIX_FMT_H264_SLICE`` - 'S264' - H264 parsed slice data, without the start code and as extracted from the H264 bitstream. This format is adapted fordrivers/media/v4l2-core/v4l2-ioctl.c +1
-1@@ -1343,7 +1343,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)case V4L2_PIX_FMT_H264 : descr ="H.264" ;break ;case V4L2_PIX_FMT_H264_NO_SC : descr ="H.264 (No Start Codes)" ;break ;case V4L2_PIX_FMT_H264_MVC : descr ="H.264 MVC" ;break ; -case V4L2_PIX_FMT_H264_SLICE_RAW : descr ="H.264 Parsed Slice Data" ;break ; +case V4L2_PIX_FMT_H264_SLICE : descr ="H.264 Parsed Slice Data" ;break ;case V4L2_PIX_FMT_H263 : descr ="H.263" ;break ;case V4L2_PIX_FMT_MPEG1 : descr ="MPEG-1 ES" ;break ;case V4L2_PIX_FMT_MPEG2 : descr ="MPEG-2 ES" ;break ;drivers/staging/media/sunxi/cedrus/cedrus_dec.c +1
-1@@ -46,7 +46,7 @@ void cedrus_device_run(void *priv) V4L2_CID_MPEG_VIDEO_MPEG2_QUANTIZATION);break ; -case V4L2_PIX_FMT_H264_SLICE_RAW : +case V4L2_PIX_FMT_H264_SLICE : run.h264.decode_params =cedrus_find_control_data (ctx,V4L2_CID_MPEG_VIDEO_H264_DECODE_PARAMS ); run.h264.pps = cedrus_find_control_data(ctx,drivers/staging/media/sunxi/cedrus/cedrus_video.c +3
-3@@ -38,7 +38,7 @@ static struct cedrus_format cedrus_formats[] = { .directions = CEDRUS_DECODE_SRC, }, { - .pixelformat = V4L2_PIX_FMT_H264_SLICE_RAW, + .pixelformat = V4L2_PIX_FMT_H264_SLICE, .directions = CEDRUS_DECODE_SRC, }, {@@ -104,7 +104,7 @@ static void cedrus_prepare_format(struct v4l2_pix_format *pix_fmt)switch (pix_fmt->pixelformat) {case V4L2_PIX_FMT_MPEG2_SLICE : -case V4L2_PIX_FMT_H264_SLICE_RAW : +case V4L2_PIX_FMT_H264_SLICE :/* Zero bytes per line for encoded source. */ bytesperline =0 ;@@ -449,7 +449,7 @@ static int cedrus_start_streaming(struct vb2_queue *vq, unsigned int count) ctx->current_codec =CEDRUS_CODEC_MPEG2 ;break ; -case V4L2_PIX_FMT_H264_SLICE_RAW : +case V4L2_PIX_FMT_H264_SLICE : ctx->current_codec =CEDRUS_CODEC_H264 ;break ;include/media/h264-ctrls.h +1
-1@@ -14,7 +14,7 @@#include <linux/videodev2.h> /* Our pixel format isn't stable at the moment */ -#define V4L2_PIX_FMT_H264_SLICE_RAW v4l2_fourcc('S', '2', '6', '4')/* H264 parsed slices */ +#define V4L2_PIX_FMT_H264_SLICE v4l2_fourcc('S', '2', '6', '4')/* H264 parsed slices */ /* * This is put insanely high to avoid conflicting with controls that Signature · 1 line
2.22.0
Re: [PATCH v7 02/11] media: uapi: h264: Rename pixel format
Quoted 59 lines · Hans Verkuil
Quoted 58 lines · Paul Kocialkowski
Hi,
Quoted 17 lines · Hans Verkuil
Quoted 7 lines · Paul Kocialkowski
Hi,
Quoted 4 lines · Ezequiel Garcia
The
V4L2_PIX_FMT_H264_SLICE_RAWname was originally suggested because the pixel format would represent H264 slices without any start code.Quoted 8 lines
Quoted 3 lines
However, as we will now introduce a start code menu control, give the pixel format a more meaningful name, while it's still early enough to do so.
I definitely agree that
SLICE_RAWis not the suffix we are looking for, but I'm not sure that_SLICEis self-describing given that we can operate either per-frame or per-slice, and_SLICEsort of implies the latter. Also, VP8 uses_FRAMEto clearly indicate that it operates per-frame.Quoted 7 lines
Well, VP8 doesn't support slices at all.
Quoted 1 line
In addition, the
_SLICEsuffix is used by MPEG-2 in the stable API. Since weRegarding MPEG-2: while it has a concept of slices, it is my understanding that you never process slices separately, but only full pictures. I may be wrong here.
I don't think that is the case since ffmpeg clearly implements decoding on a per-slice basis (
mpeg_decode_slice).Information is also passed on a per-slice basis to VAAPI (
vaapi_mpeg2_decode_slice) with a distinct data buffer and slice parameter buffer for each slice. Among other things, it contains the vertical and horizontal positions for the slice, which we can set in the hardware.Quoted 10 lines
Quoted 5 lines
certainly want MPEG-2 to allow per-slice and per-frame decoding as well as H.264 and that the
_SLICEformat is specified to be the broken "concatenated slices" that cedrus expects, we probably want to use another suffix. This way, we could deprecatedMPEG2_SLICEand introduce a new format for MPEG-2 that would have consistent naming with the other mpeg formats.I actually think that
H264_SLICEis a decent name.I'm less sure about
MPEG2_SLICEsince I am not sure if it means the same as a H264 slice.The main problem I see is that we have already specified
MPEG2_SLICEin a way that is incompatible with the future improvments we want to bring to the API:
" The output buffer must contain the appropriate number of macroblocks to decode a full corresponding frame to the matching capture buffer."So I only see two possibilities: either we decide to change the specification of the pixel format and we can keep using the
_SLICEsuffix, either we need to introduce a new pixel format with another suffix, which should also be reflected on other MPEG formats for consistency. Then we can deprecateMPEG2_SLICEand have drivers stop using it.
Quoted 8 lines
Quoted 1 line
What do you think?
I'd change the specification of the pixel format. So
MPEG2_SLICEnow supports multiple slices if the hardware supports it as well.We would need an
MPEG2_DECODING_MODEcontrol as well, that currently would read FRAME based only.
That's exactly what I was about to suggest.
Regards,
Ezequiel
Re: [PATCH v7 02/11] media: uapi: h264: Rename pixel format
Hi,
Quoted 62 lines · Ezequiel Garcia
Quoted 61 lines · Hans Verkuil
Quoted 59 lines · Paul Kocialkowski
Hi,
Quoted 17 lines · Hans Verkuil
Quoted 7 lines · Paul Kocialkowski
Hi,
Quoted 4 lines · Ezequiel Garcia
The
V4L2_PIX_FMT_H264_SLICE_RAWname was originally suggested because the pixel format would represent H264 slices without any start code.Quoted 8 lines
Quoted 3 lines
However, as we will now introduce a start code menu control, give the pixel format a more meaningful name, while it's still early enough to do so.
I definitely agree that
SLICE_RAWis not the suffix we are looking for, but I'm not sure that_SLICEis self-describing given that we can operate either per-frame or per-slice, and_SLICEsort of implies the latter. Also, VP8 uses_FRAMEto clearly indicate that it operates per-frame.Quoted 7 lines
Well, VP8 doesn't support slices at all.
Quoted 1 line
In addition, the
_SLICEsuffix is used by MPEG-2 in the stable API. Since weRegarding MPEG-2: while it has a concept of slices, it is my understanding that you never process slices separately, but only full pictures. I may be wrong here.
I don't think that is the case since ffmpeg clearly implements decoding on a per-slice basis (
mpeg_decode_slice).Information is also passed on a per-slice basis to VAAPI (
vaapi_mpeg2_decode_slice) with a distinct data buffer and slice parameter buffer for each slice. Among other things, it contains the vertical and horizontal positions for the slice, which we can set in the hardware.Quoted 10 lines
Quoted 5 lines
certainly want MPEG-2 to allow per-slice and per-frame decoding as well as H.264 and that the
_SLICEformat is specified to be the broken "concatenated slices" that cedrus expects, we probably want to use another suffix. This way, we could deprecatedMPEG2_SLICEand introduce a new format for MPEG-2 that would have consistent naming with the other mpeg formats.I actually think that
H264_SLICEis a decent name.I'm less sure about
MPEG2_SLICEsince I am not sure if it means the same as a H264 slice.The main problem I see is that we have already specified
MPEG2_SLICEin a way that is incompatible with the future improvments we want to bring to the API:
" The output buffer must contain the appropriate number of macroblocks to decode a full corresponding frame to the matching capture buffer."So I only see two possibilities: either we decide to change the specification of the pixel format and we can keep using the
_SLICEsuffix, either we need to introduce a new pixel format with another suffix, which should also be reflected on other MPEG formats for consistency. Then we can deprecateMPEG2_SLICEand have drivers stop using it.What do you think?
Quoted 8 lines
Quoted 6 lines
I'd change the specification of the pixel format. So
MPEG2_SLICEnow supports multiple slices if the hardware supports it as well.We would need an
MPEG2_DECODING_MODEcontrol as well, that currently would read FRAME based only.That's exactly what I was about to suggest.
Sounds perfect then!
I have started looking at the start-code situation to see if it shares similarities with H.264, but did not go far enough to reach any conclusion on that aspect.
Cheers,
Paul
Signature · 3 lines
Paul Kocialkowski, Bootlin Embedded Linux and kernel engineering https://bootlin.com
PATCH v7 3/11 media: uapi: h264: Add the concept of decoding mode
In reply to Ezequiel GarciaFrom: Boris Brezillon <[email protected]>
Some stateless decoders don't support per-slice decoding granularity (or at least not in a way that would make them efficient or easy to use).
Expose a menu to control the supported decoding modes. Drivers are allowed to support only one decoding but they can support both too.
To fully specify the decoding operation, we need to introduce a start_byte_offset, to indicate where slices start.
---
Changes in v7:
* Further specify start_byte_offset in slice- and frame-based decoding modes.
Changes in v6:
* Fix spec, specifiying the decode-mode pixelformat modifier needs to be set once, as suggested by Hans.
* Rename, as suggested by Paul.
Changes in v5:
* Improve specification as suggested by Hans. Changes in v4:
* Typos/rewording fixes
Changes in v3:
* s/per-{slice,frame} decoding/{slice,frame}-based decoding/
* Add Paul's R-b
Changes in v2:
* Allow decoding multiple slices in per-slice decoding mode
* Minor doc improvement/fixes
doc api ext-ctrls-codec
--- .../media/uapi/v4l/ext-ctrls-codec.rst | 57 ++++++++++++++++++- .../media/uapi/v4l/pixfmt-compressed.rst | 6 +- drivers/media/v4l2-core/v4l2-ctrls.c | 9 +++ include/media/h264-ctrls.h | 10 ++++ 4 files changed, 79 insertions(+), 3 deletions(-)
Documentation/media/uapi/v4l/ext-ctrls-codec.rst +56 -1
@@ -1747,6 +1747,14 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type - * - __u32 - ``size`` - + * - __u32 + - ``start_byte_offset`` + Offset (in bytes) from the beginning of the OUTPUT buffer to the start + of the slice. If the slice starts with a start code, then this is the + offset to such start code. When operating in slice-based decoding mode + (see :c:type:`v4l2_mpeg_video_h264_decode_mode`), this field should + be set to 0. When operating in frame-based decoding mode, this field + should be 0, for the first slice. * - __u32 - ``header_bit_size`` -
@@ -1930,7 +1938,10 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type - - * - __u16 - ``num_slices`` - - Number of slices needed to decode the current frame + - Number of slices needed to decode the current frame/field. When + operating in slice-based decoding mode (see + :c:type:`v4l2_mpeg_video_h264_decode_mode`), this field + should always be set to one. * - __u16 - ``nal_ref_idc`` - NAL reference ID value coming from the NAL Unit header
@@ -2021,6 +2032,50 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type - - 0x00000004 - The DPB entry is a long term reference frame +``V4L2_CID_MPEG_VIDEO_H264_DECODE_MODE (enum)`` + Specifies the decoding mode to use. Currently exposes slice-based and + frame-based decoding but new modes might be added later on. + This control is used as a modifier for V4L2_PIX_FMT_H264_SLICE + pixel format. Applications that support V4L2_PIX_FMT_H264_SLICE + are required to set this control in order to specify the decoding mode + that is expected for the buffer. + Drivers may expose a single or multiple decoding modes, depending + on what they can support. + + .. 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_decode_mode + +.. cssclass:: longtable + +.. flat-table:: + :header-rows: 0 + :stub-columns: 0 + :widths: 1 1 2 + + * - ``V4L2_MPEG_VIDEO_H264_DECODE_MODE_SLICE_BASED`` + - 0 + - Decoding is done at the slice granularity. + In this mode, ``num_slices`` field in struct + :c:type:`v4l2_ctrl_h264_decode_params` should be set to 1, + and ``start_byte_offset`` in struct + :c:type:`v4l2_ctrl_h264_slice_params` should be set to 0. + The OUTPUT buffer must contain a single slice. + * - ``V4L2_MPEG_VIDEO_H264_DECODE_MODE_FRAME_BASED`` + - 1 + - Decoding is done at the frame granularity. + In this mode, ``num_slices`` field in struct + :c:type:`v4l2_ctrl_h264_decode_params` should be set to the number + of slices in the frame, and ``start_byte_offset`` in struct + :c:type:`v4l2_ctrl_h264_slice_params` should be set accordingly + for each slice. For the first slice, ``start_byte_offset`` should + be zero. + The OUTPUT buffer must contain all slices needed to decode the + frame. The OUTPUT buffer must also contain both fields. + .. _v4l2-mpeg-mpeg2: ``V4L2_CID_MPEG_VIDEO_MPEG2_SLICE_PARAMS (struct)``
Documentation/media/uapi/v4l/pixfmt-compressed.rst +4 -2
@@ -60,8 +60,10 @@ Compressed Formats extracted from the H264 bitstream. This format is adapted for stateless video decoders that implement an H264 pipeline (using the :ref:`mem2mem` and :ref:`media-request-api`). - Metadata associated with the frame to decode are required to - be passed through the ``V4L2_CID_MPEG_VIDEO_H264_SPS``, + This pixelformat has a modifier that must be set at least once + through the ``V4L2_CID_MPEG_VIDEO_H264_DECODE_MODE`` control. + In addition, metadata associated with the frame to decode are + required to be passed through the ``V4L2_CID_MPEG_VIDEO_H264_SPS``, ``V4L2_CID_MPEG_VIDEO_H264_PPS``, ``V4L2_CID_MPEG_VIDEO_H264_SCALING_MATRIX``, ``V4L2_CID_MPEG_VIDEO_H264_SLICE_PARAMS`` and
drivers/media/v4l2-core/v4l2-ctrls.c +9 -0
@@ -402,6 +402,11 @@ const char * const *v4l2_ctrl_get_menu(u32 id)"Explicit" , NULL, }; +static const char *const h264_decode_mode[] = { +"Slice-Based" , +"Frame-Based" , +NULL , + };static const char *const mpeg_mpeg2_level[] = {"Low" , "Main",
@@ -633,6 +638,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_DECODE_MODE : +return h264_decode_mode;case V4L2_CID_MPEG_VIDEO_MPEG2_LEVEL :return mpeg_mpeg2_level;case V4L2_CID_MPEG_VIDEO_MPEG2_PROFILE :
@@ -852,6 +859,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_DECODE_MODE :return "H264 Decode 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" ;
@@ -1220,6 +1228,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_DECODE_MODE :case V4L2_CID_MPEG_VIDEO_MPEG2_LEVEL :case V4L2_CID_MPEG_VIDEO_MPEG2_PROFILE :case V4L2_CID_MPEG_VIDEO_MPEG4_LEVEL :
include/media/h264-ctrls.h +10 -0
@@ -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_DECODE_MODE (V4L2_CID_MPEG_BASE+1005)/* enum v4l2_ctrl_type type values */ #define V4L2_CTRL_TYPE_H264_SPS 0x0110
@@ -34,6 +35,11 @@#define V4L2_CTRL_TYPE_H264_SLICE_PARAMS 0x0113#define V4L2_CTRL_TYPE_H264_DECODE_PARAMS 0x0114 +enum v4l2_mpeg_video_h264_decode_mode { +V4L2_MPEG_VIDEO_H264_DECODE_MODE_SLICE_BASED , +V4L2_MPEG_VIDEO_H264_DECODE_MODE_FRAME_BASED , +}; +#define V4L2_H264_SPS_CONSTRAINT_SET0_FLAG 0x01#define V4L2_H264_SPS_CONSTRAINT_SET1_FLAG 0x02#define V4L2_H264_SPS_CONSTRAINT_SET2_FLAG 0x04
@@ -125,6 +131,10 @@ struct v4l2_h264_pred_weight_table {struct v4l2_ctrl_h264_slice_params {/* Size in bytes, including header */ __u32 size; + +/* Offset in bytes to the start of slice in the OUTPUT buffer. */ +__u32 start_byte_offset; +/* Offset in bits to slice_data() from the beginning of this slice. */ __u32 header_bit_size;
Signature · 1 line
2.22.0
PATCH v7 4/11 media: uapi: h264: Add the concept of start code
In reply to Ezequiel GarciaStateless decoders have different expectations about the start code that is prepended on H264 slices. Add a menu control to express the supported start code types (including no start code).
Drivers are allowed to support only one start code type, but they can support both too.
Note that this is independent of the H264 decoding mode, which specifies the granularity of the decoding operations. Either in frame-based or slice-based mode, this new control will allow to define the start code expected on H264 slices.
---
Changes in v7:
* None.
Changes in v6:
* Rename to START_CODE and enum values
as suggested by Hand and Paul.
Changes in v5:
* Improve specification as suggested by Hans.
Changes in v4:
* New patch.
--- .../media/uapi/v4l/ext-ctrls-codec.rst | 33 +++++++++++++++++++ .../media/uapi/v4l/pixfmt-compressed.rst | 5 +-- drivers/media/v4l2-core/v4l2-ctrls.c | 9 +++++ include/media/h264-ctrls.h | 6 ++++ 4 files changed, 51 insertions(+), 2 deletions(-)
Documentation/media/uapi/v4l/ext-ctrls-codec.rst +33 -0
@@ -2076,6 +2076,39 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type - The OUTPUT buffer must contain all slices needed to decode the frame. The OUTPUT buffer must also contain both fields. +``V4L2_CID_MPEG_VIDEO_H264_START_CODE (enum)`` + Specifies the H264 slice start code expected for each slice. + This control is used as a modifier for V4L2_PIX_FMT_H264_SLICE + pixel format. Applications that support V4L2_PIX_FMT_H264_SLICE + are required to set this control in order to specify the start code + that is expected for the buffer. + Drivers may expose a single or multiple start codes, depending + on what they can support. + + .. 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_start_code + +.. cssclass:: longtable + +.. flat-table:: + :header-rows: 0 + :stub-columns: 0 + :widths: 1 1 2 + + * - ``V4L2_MPEG_VIDEO_H264_START_CODE_NONE`` + - 0 + - Selecting this value specifies that H264 slices are passed + to the driver without any start code. + * - ``V4L2_MPEG_VIDEO_H264_START_CODE_ANNEX_B`` + - 1 + - Selecting this value specifies that H264 slices are expected + to be prefixed by Annex B start codes. According to :ref:`h264` + valid start codes can be 3-bytes 0x000001 or 4-bytes 0x00000001. + .. _v4l2-mpeg-mpeg2: ``V4L2_CID_MPEG_VIDEO_MPEG2_SLICE_PARAMS (struct)``
Documentation/media/uapi/v4l/pixfmt-compressed.rst +3 -2
@@ -60,8 +60,9 @@ Compressed Formats extracted from the H264 bitstream. This format is adapted for stateless video decoders that implement an H264 pipeline (using the :ref:`mem2mem` and :ref:`media-request-api`). - This pixelformat has a modifier that must be set at least once - through the ``V4L2_CID_MPEG_VIDEO_H264_DECODE_MODE`` control. + This pixelformat has two modifiers that must be set at least once + through the ``V4L2_CID_MPEG_VIDEO_H264_DECODE_MODE`` + and ``V4L2_CID_MPEG_VIDEO_H264_START_CODE`` controls. In addition, metadata associated with the frame to decode are required to be passed through the ``V4L2_CID_MPEG_VIDEO_H264_SPS``, ``V4L2_CID_MPEG_VIDEO_H264_PPS``,
drivers/media/v4l2-core/v4l2-ctrls.c +9 -0
@@ -407,6 +407,11 @@ const char * const *v4l2_ctrl_get_menu(u32 id)"Frame-Based" , NULL, }; +static const char *const h264_start_code[] = { +"No Start Code" , +"Annex B Start Code" , +NULL , + };static const char *const mpeg_mpeg2_level[] = {"Low" , "Main",
@@ -640,6 +645,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)return h264_fmo_map_type;case V4L2_CID_MPEG_VIDEO_H264_DECODE_MODE :return h264_decode_mode; +case V4L2_CID_MPEG_VIDEO_H264_START_CODE : +return h264_start_code;case V4L2_CID_MPEG_VIDEO_MPEG2_LEVEL :return mpeg_mpeg2_level;case V4L2_CID_MPEG_VIDEO_MPEG2_PROFILE :
@@ -860,6 +867,7 @@ const char *v4l2_ctrl_get_name(u32 id)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_DECODE_MODE :return "H264 Decode Mode" ; +case V4L2_CID_MPEG_VIDEO_H264_START_CODE :return "H264 Start Code" ;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" ;
@@ -1229,6 +1237,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,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_DECODE_MODE : +case V4L2_CID_MPEG_VIDEO_H264_START_CODE :case V4L2_CID_MPEG_VIDEO_MPEG2_LEVEL :case V4L2_CID_MPEG_VIDEO_MPEG2_PROFILE :case V4L2_CID_MPEG_VIDEO_MPEG4_LEVEL :
include/media/h264-ctrls.h +6 -0
@@ -27,6 +27,7 @@#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_DECODE_MODE (V4L2_CID_MPEG_BASE+1005) +#define V4L2_CID_MPEG_VIDEO_H264_START_CODE (V4L2_CID_MPEG_BASE+1006)/* enum v4l2_ctrl_type type values */ #define V4L2_CTRL_TYPE_H264_SPS 0x0110
@@ -40,6 +41,11 @@ enum v4l2_mpeg_video_h264_decode_mode { V4L2_MPEG_VIDEO_H264_DECODE_MODE_FRAME_BASED, }; +enum v4l2_mpeg_video_h264_start_code { +V4L2_MPEG_VIDEO_H264_START_CODE_NONE , +V4L2_MPEG_VIDEO_H264_START_CODE_ANNEX_B , +}; +#define V4L2_H264_SPS_CONSTRAINT_SET0_FLAG 0x01#define V4L2_H264_SPS_CONSTRAINT_SET1_FLAG 0x02#define V4L2_H264_SPS_CONSTRAINT_SET2_FLAG 0x04
Signature · 1 line
2.22.0
Re: [PATCH v7 04/11] media: uapi: h264: Add the concept of start code
Hi,
Quoted 5 lines · Ezequiel Garcia
Stateless decoders have different expectations about the start code that is prepended on H264 slices. Add a menu control to express the supported start code types (including no start code).
Quoted 7 lines
Drivers are allowed to support only one start code type, but they can support both too.
Note that this is independent of the H264 decoding mode, which specifies the granularity of the decoding operations. Either in frame-based or slice-based mode, this new control will allow to define the start code expected on H264 slices.
Looks good to me at this point:
Thanks!
Paul
Quoted 147 lines
Signed-off-by Ezequiel Garcia <[email protected]>Tested-by Philipp Zabel <[email protected]>---
Changes in v7:
* None.
Changes in v6:
* Rename toSTART_CODEand enum values
as suggested by Hand and Paul.
Changes in v5:
* Improve specification as suggested by Hans.
Changes in v4:
* New patch.--- .../media/uapi/v4l/ext-ctrls-codec.rst | 33 +++++++++++++++++++ .../media/uapi/v4l/pixfmt-compressed.rst | 5 +-- drivers/media/v4l2-core/v4l2-ctrls.c | 9 +++++ include/media/h264-ctrls.h | 6 ++++ 4 files changed, 51 insertions(+), 2 deletions(-)Documentation/media/uapi/v4l/ext-ctrls-codec.rst +33
-0@@ -2076,6 +2076,39 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type - The OUTPUT buffer must contain all slices needed to decode the frame. The OUTPUT buffer must also contain both fields. +``V4L2_CID_MPEG_VIDEO_H264_START_CODE (enum)`` + Specifies the H264 slice start code expected for each slice. + This control is used as a modifier for V4L2_PIX_FMT_H264_SLICE + pixel format. Applications that support V4L2_PIX_FMT_H264_SLICE + are required to set this control in order to specify the start code + that is expected for the buffer. + Drivers may expose a single or multiple start codes, depending + on what they can support. + + .. 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_start_code + +.. cssclass:: longtable + +.. flat-table:: + :header-rows: 0 + :stub-columns: 0 + :widths: 1 1 2 + + * - ``V4L2_MPEG_VIDEO_H264_START_CODE_NONE`` + - 0 + - Selecting this value specifies that H264 slices are passed + to the driver without any start code. + * - ``V4L2_MPEG_VIDEO_H264_START_CODE_ANNEX_B`` + - 1 + - Selecting this value specifies that H264 slices are expected + to be prefixed by Annex B start codes. According to :ref:`h264` + valid start codes can be 3-bytes 0x000001 or 4-bytes 0x00000001. + .. _v4l2-mpeg-mpeg2: ``V4L2_CID_MPEG_VIDEO_MPEG2_SLICE_PARAMS (struct)``Documentation/media/uapi/v4l/pixfmt-compressed.rst +3
-2@@ -60,8 +60,9 @@ Compressed Formats extracted from the H264 bitstream. This format is adapted for stateless video decoders that implement an H264 pipeline (using the :ref:`mem2mem` and :ref:`media-request-api`). - This pixelformat has a modifier that must be set at least once - through the ``V4L2_CID_MPEG_VIDEO_H264_DECODE_MODE`` control. + This pixelformat has two modifiers that must be set at least once + through the ``V4L2_CID_MPEG_VIDEO_H264_DECODE_MODE`` + and ``V4L2_CID_MPEG_VIDEO_H264_START_CODE`` controls. In addition, metadata associated with the frame to decode are required to be passed through the ``V4L2_CID_MPEG_VIDEO_H264_SPS``, ``V4L2_CID_MPEG_VIDEO_H264_PPS``,drivers/media/v4l2-core/v4l2-ctrls.c +9
-0@@ -407,6 +407,11 @@ const char * const *v4l2_ctrl_get_menu(u32 id)"Frame-Based" , NULL, }; +static const char *const h264_start_code[] = { +"No Start Code" , +"Annex B Start Code" , +NULL , + };static const char *const mpeg_mpeg2_level[] = {"Low" , "Main",@@ -640,6 +645,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)return h264_fmo_map_type;case V4L2_CID_MPEG_VIDEO_H264_DECODE_MODE :return h264_decode_mode; +case V4L2_CID_MPEG_VIDEO_H264_START_CODE : +return h264_start_code;case V4L2_CID_MPEG_VIDEO_MPEG2_LEVEL :return mpeg_mpeg2_level;case V4L2_CID_MPEG_VIDEO_MPEG2_PROFILE :@@ -860,6 +867,7 @@ const char *v4l2_ctrl_get_name(u32 id)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_DECODE_MODE :return "H264 Decode Mode" ; +case V4L2_CID_MPEG_VIDEO_H264_START_CODE :return "H264 Start Code" ;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" ;@@ -1229,6 +1237,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,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_DECODE_MODE : +case V4L2_CID_MPEG_VIDEO_H264_START_CODE :case V4L2_CID_MPEG_VIDEO_MPEG2_LEVEL :case V4L2_CID_MPEG_VIDEO_MPEG2_PROFILE :case V4L2_CID_MPEG_VIDEO_MPEG4_LEVEL :include/media/h264-ctrls.h +6
-0@@ -27,6 +27,7 @@#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_DECODE_MODE (V4L2_CID_MPEG_BASE+1005) +#define V4L2_CID_MPEG_VIDEO_H264_START_CODE (V4L2_CID_MPEG_BASE+1006)/* enum v4l2_ctrl_type type values */ #define V4L2_CTRL_TYPE_H264_SPS 0x0110@@ -40,6 +41,11 @@ enum v4l2_mpeg_video_h264_decode_mode { V4L2_MPEG_VIDEO_H264_DECODE_MODE_FRAME_BASED, }; +enum v4l2_mpeg_video_h264_start_code { +V4L2_MPEG_VIDEO_H264_START_CODE_NONE , +V4L2_MPEG_VIDEO_H264_START_CODE_ANNEX_B , +}; +#define V4L2_H264_SPS_CONSTRAINT_SET0_FLAG 0x01#define V4L2_H264_SPS_CONSTRAINT_SET1_FLAG 0x02#define V4L2_H264_SPS_CONSTRAINT_SET2_FLAG 0x04Signature · 1 line
2.22.0
Signature · 3 lines
Paul Kocialkowski, Bootlin Embedded Linux and kernel engineering https://bootlin.com
Re: [PATCH v7 04/11] media: uapi: h264: Add the concept of start code
In reply to Ezequiel GarciaQuoted 37 lines · Ezequiel Garcia
Stateless decoders have different expectations about the start code that is prepended on H264 slices. Add a menu control to express the supported start code types (including no start code).
Drivers are allowed to support only one start code type, but they can support both too.
Note that this is independent of the H264 decoding mode, which specifies the granularity of the decoding operations. Either in frame-based or slice-based mode, this new control will allow to define the start code expected on H264 slices.
Signed-off-by Ezequiel Garcia <[email protected]>Tested-by Philipp Zabel <[email protected]>---
Changes in v7:
* None.
Changes in v6:
* Rename toSTART_CODEand enum values
as suggested by Hand and Paul.
Changes in v5:
* Improve specification as suggested by Hans.
Changes in v4:
* New patch.--- .../media/uapi/v4l/ext-ctrls-codec.rst | 33 +++++++++++++++++++ .../media/uapi/v4l/pixfmt-compressed.rst | 5 +-- drivers/media/v4l2-core/v4l2-ctrls.c | 9 +++++ include/media/h264-ctrls.h | 6 ++++ 4 files changed, 51 insertions(+), 2 deletions(-)Documentation/media/uapi/v4l/ext-ctrls-codec.rst +0
-0
Documentation/media/uapi/v4l/ext-ctrls-codec.rst enum v4l2_mpeg_video_h264_hierarchical_coding_type - · line 2079
Quoted 4 lines
Documentation/media/uapi/v4l/ext-ctrls-codec.rst +1
-0@@ -2076,6 +2076,39 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type - The OUTPUT buffer must contain all slices needed to decode the frame. The OUTPUT buffer must also contain both fields. +``V4L2_CID_MPEG_VIDEO_H264_START_CODE (enum)``
I believe we need to revisit the naming here. In most documents I've read, start code refers to a header that allow parsing a bit stream from a random location. In H264, this is used as synonyme of ANNEX B NAL Headers. In this context, the obvious future value to this enum is AVCc headers, which is not a start code.
I would suggest to revisite and rename to:
V4L2_CID_MPEG_VIDEO_H264_NAL_HDR_TYPE
Quoted 116 lines
Documentation/media/uapi/v4l/ext-ctrls-codec.rst +32
-0+ Specifies the H264 slice start code expected for each slice. + This control is used as a modifier for V4L2_PIX_FMT_H264_SLICE + pixel format. Applications that support V4L2_PIX_FMT_H264_SLICE + are required to set this control in order to specify the start code + that is expected for the buffer. + Drivers may expose a single or multiple start codes, depending + on what they can support. + + .. 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_start_code + +.. cssclass:: longtable + +.. flat-table:: + :header-rows: 0 + :stub-columns: 0 + :widths: 1 1 2 + + * - ``V4L2_MPEG_VIDEO_H264_START_CODE_NONE`` + - 0 + - Selecting this value specifies that H264 slices are passed + to the driver without any start code. + * - ``V4L2_MPEG_VIDEO_H264_START_CODE_ANNEX_B`` + - 1 + - Selecting this value specifies that H264 slices are expected + to be prefixed by Annex B start codes. According to :ref:`h264` + valid start codes can be 3-bytes 0x000001 or 4-bytes 0x00000001. + .. _v4l2-mpeg-mpeg2: ``V4L2_CID_MPEG_VIDEO_MPEG2_SLICE_PARAMS (struct)``Documentation/media/uapi/v4l/pixfmt-compressed.rst +3
-2@@ -60,8 +60,9 @@ Compressed Formats extracted from the H264 bitstream. This format is adapted for stateless video decoders that implement an H264 pipeline (using the :ref:`mem2mem` and :ref:`media-request-api`). - This pixelformat has a modifier that must be set at least once - through the ``V4L2_CID_MPEG_VIDEO_H264_DECODE_MODE`` control. + This pixelformat has two modifiers that must be set at least once + through the ``V4L2_CID_MPEG_VIDEO_H264_DECODE_MODE`` + and ``V4L2_CID_MPEG_VIDEO_H264_START_CODE`` controls. In addition, metadata associated with the frame to decode are required to be passed through the ``V4L2_CID_MPEG_VIDEO_H264_SPS``, ``V4L2_CID_MPEG_VIDEO_H264_PPS``,drivers/media/v4l2-core/v4l2-ctrls.c +9
-0@@ -407,6 +407,11 @@ const char * const *v4l2_ctrl_get_menu(u32 id)"Frame-Based" , NULL, }; +static const char *const h264_start_code[] = { +"No Start Code" , +"Annex B Start Code" , +NULL , + };static const char *const mpeg_mpeg2_level[] = {"Low" , "Main",@@ -640,6 +645,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)return h264_fmo_map_type;case V4L2_CID_MPEG_VIDEO_H264_DECODE_MODE :return h264_decode_mode; +case V4L2_CID_MPEG_VIDEO_H264_START_CODE : +return h264_start_code;case V4L2_CID_MPEG_VIDEO_MPEG2_LEVEL :return mpeg_mpeg2_level;case V4L2_CID_MPEG_VIDEO_MPEG2_PROFILE :@@ -860,6 +867,7 @@ const char *v4l2_ctrl_get_name(u32 id)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_DECODE_MODE :return "H264 Decode Mode" ; +case V4L2_CID_MPEG_VIDEO_H264_START_CODE :return "H264 Start Code" ;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" ;@@ -1229,6 +1237,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,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_DECODE_MODE : +case V4L2_CID_MPEG_VIDEO_H264_START_CODE :case V4L2_CID_MPEG_VIDEO_MPEG2_LEVEL :case V4L2_CID_MPEG_VIDEO_MPEG2_PROFILE :case V4L2_CID_MPEG_VIDEO_MPEG4_LEVEL :include/media/h264-ctrls.h +6
-0@@ -27,6 +27,7 @@#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_DECODE_MODE (V4L2_CID_MPEG_BASE+1005) +#define V4L2_CID_MPEG_VIDEO_H264_START_CODE (V4L2_CID_MPEG_BASE+1006)/* enum v4l2_ctrl_type type values */ #define V4L2_CTRL_TYPE_H264_SPS 0x0110@@ -40,6 +41,11 @@ enum v4l2_mpeg_video_h264_decode_mode { V4L2_MPEG_VIDEO_H264_DECODE_MODE_FRAME_BASED, }; +enum v4l2_mpeg_video_h264_start_code { +V4L2_MPEG_VIDEO_H264_START_CODE_NONE , +V4L2_MPEG_VIDEO_H264_START_CODE_ANNEX_B , +}; +#define V4L2_H264_SPS_CONSTRAINT_SET0_FLAG 0x01#define V4L2_H264_SPS_CONSTRAINT_SET1_FLAG 0x02#define V4L2_H264_SPS_CONSTRAINT_SET2_FLAG 0x04
PATCH v7 5/11 media: uapi: h264: Get rid of the p0/b0/b1 ref-lists
In reply to Ezequiel GarciaFrom: Boris Brezillon <[email protected]>
Those lists can be extracted from the dpb, let's simplify userspace life and build that list kernel-side (generic helpers will be provided for drivers that need this list).
---
Changes in v7:
* None.
Changes in v6:
* None.
Changes in v5:
* None.
Changes in v4:
* Add R-b
* Remove the reflist from the struct
Changes in v3:
* None
Changes in v2:
* None
--- Documentation/media/uapi/v4l/ext-ctrls-codec.rst | 9 --------- include/media/h264-ctrls.h | 3 --- 2 files changed, 12 deletions(-)
Documentation/media/uapi/v4l/ext-ctrls-codec.rst +0 -9
@@ -1945,15 +1945,6 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type - * - __u16 - ``nal_ref_idc`` - NAL reference ID value coming from the NAL Unit header - * - __u8 - - ``ref_pic_list_p0[32]`` - - Backward reference list used by P-frames in the original bitstream order - * - __u8 - - ``ref_pic_list_b0[32]`` - - Backward reference list used by B-frames in the original bitstream order - * - __u8 - - ``ref_pic_list_b1[32]`` - - Forward reference list used by B-frames in the original bitstream order * - __s32 - ``top_field_order_cnt`` - Picture Order Count for the coded top field
include/media/h264-ctrls.h +0 -3
@@ -202,9 +202,6 @@ struct v4l2_ctrl_h264_decode_params {struct v4l2_h264_dpb_entry dpb[16 ];__u16 num_slices;__u16 nal_ref_idc; -__u8 ref_pic_list_p0[32 ]; -__u8 ref_pic_list_b0[32 ]; -__u8 ref_pic_list_b1[32 ];__s32 top_field_order_cnt;__s32 bottom_field_order_cnt;__u32 flags;/* V4L2_H264_DECODE_PARAM_FLAG_* */
Signature · 1 line
2.22.0
PATCH v7 6/11 media: cedrus: Cleanup control initialization
In reply to Ezequiel GarciaIn order to introduce other controls, the control initialization needs to support an initial struct v4l2_ctrl_control.
While here, let's cleanup the control initialization, removing unneeded fields.
---
Changes in v7:
* None.
Changes in v6:
* None.
Changes in v5:
* None.
Changes in v4:
* New patch.
--- drivers/staging/media/sunxi/cedrus/cedrus.c | 45 +++++++++++---------- drivers/staging/media/sunxi/cedrus/cedrus.h | 3 +- 2 files changed, 25 insertions(+), 23 deletions(-)
drivers/staging/media/sunxi/cedrus/cedrus.c +24 -21
@@ -29,44 +29,51 @@static const struct cedrus_control cedrus_controls[] = { { - .id =V4L2_CID_MPEG_VIDEO_MPEG2_SLICE_PARAMS , - .elem_size =sizeof (struct v4l2_ctrl_mpeg2_slice_params ), + .cfg = { + .id =V4L2_CID_MPEG_VIDEO_MPEG2_SLICE_PARAMS , + }, .codec =CEDRUS_CODEC_MPEG2 , .required = true, }, { - .id =V4L2_CID_MPEG_VIDEO_MPEG2_QUANTIZATION , - .elem_size =sizeof (struct v4l2_ctrl_mpeg2_quantization ), + .cfg = { + .id =V4L2_CID_MPEG_VIDEO_MPEG2_QUANTIZATION , + }, .codec =CEDRUS_CODEC_MPEG2 , .required = false, }, { - .id =V4L2_CID_MPEG_VIDEO_H264_DECODE_PARAMS , - .elem_size =sizeof (struct v4l2_ctrl_h264_decode_params ), + .cfg = { + .id =V4L2_CID_MPEG_VIDEO_H264_DECODE_PARAMS , + }, .codec =CEDRUS_CODEC_H264 , .required = true, }, { - .id =V4L2_CID_MPEG_VIDEO_H264_SLICE_PARAMS , - .elem_size =sizeof (struct v4l2_ctrl_h264_slice_params ), + .cfg = { + .id =V4L2_CID_MPEG_VIDEO_H264_SLICE_PARAMS , + }, .codec =CEDRUS_CODEC_H264 , .required = true, }, { - .id =V4L2_CID_MPEG_VIDEO_H264_SPS , - .elem_size =sizeof (struct v4l2_ctrl_h264_sps ), + .cfg = { + .id =V4L2_CID_MPEG_VIDEO_H264_SPS , + }, .codec =CEDRUS_CODEC_H264 , .required = true, }, { - .id =V4L2_CID_MPEG_VIDEO_H264_PPS , - .elem_size =sizeof (struct v4l2_ctrl_h264_pps ), + .cfg = { + .id =V4L2_CID_MPEG_VIDEO_H264_PPS , + }, .codec =CEDRUS_CODEC_H264 , .required = true, }, { - .id =V4L2_CID_MPEG_VIDEO_H264_SCALING_MATRIX , - .elem_size =sizeof (struct v4l2_ctrl_h264_scaling_matrix ), + .cfg = { + .id =V4L2_CID_MPEG_VIDEO_H264_SCALING_MATRIX , + }, .codec =CEDRUS_CODEC_H264 , .required = true, },
@@ -106,12 +113,8 @@ static int cedrus_init_ctrls(struct cedrus_dev *dev, struct cedrus_ctx *ctx)return -ENOMEM ;for (i =0 ; i <CEDRUS_CONTROLS_COUNT ; i++) { -struct v4l2_ctrl_config cfg = {}; - - cfg.elem_size = cedrus_controls[i].elem_size; - cfg.id = cedrus_controls[i].id; - - ctrl =v4l2_ctrl_new_custom (hdl, &cfg,NULL ); + ctrl =v4l2_ctrl_new_custom (hdl, &cedrus_controls[i].cfg, +NULL );if (hdl->error) {v4l2_err (&dev->v4l2_dev,"Failed to create new custom control\n" );
@@ -178,7 +181,7 @@ static int cedrus_request_validate(struct media_request *req)continue ; ctrl_test =v4l2_ctrl_request_hdl_ctrl_find (hdl, - cedrus_controls[i].id); + cedrus_controls[i].cfg.id);if (!ctrl_test) {v4l2_info (&ctx->dev->v4l2_dev,"Missing required codec control\n" );
drivers/staging/media/sunxi/cedrus/cedrus.h +1 -2
@@ -49,8 +49,7 @@ enum cedrus_h264_pic_type { };struct cedrus_control { -u32 id; -u32 elem_size; +struct v4l2_ctrl_config cfg;enum cedrus_codec codec;unsigned char required:1 ; };
Signature · 1 line
2.22.0
Re: [PATCH v7 06/11] media: cedrus: Cleanup control initialization
Hi,
Quoted 5 lines · Ezequiel Garcia
In order to introduce other controls, the control initialization needs to support an initial struct
v4l2_ctrl_control.While here, let's cleanup the control initialization, removing unneeded fields.
Thanks for the change!
Cheers,
Paul
Quoted 126 lines
Signed-off-by Ezequiel Garcia <[email protected]>---
Changes in v7:
* None.
Changes in v6:
* None.
Changes in v5:
* None.
Changes in v4:
* New patch.--- drivers/staging/media/sunxi/cedrus/cedrus.c | 45 +++++++++++---------- drivers/staging/media/sunxi/cedrus/cedrus.h | 3 +- 2 files changed, 25 insertions(+), 23 deletions(-)drivers/staging/media/sunxi/cedrus/cedrus.c +24
-21@@ -29,44 +29,51 @@static const struct cedrus_control cedrus_controls[] = { { - .id =V4L2_CID_MPEG_VIDEO_MPEG2_SLICE_PARAMS , - .elem_size =sizeof (struct v4l2_ctrl_mpeg2_slice_params ), + .cfg = { + .id =V4L2_CID_MPEG_VIDEO_MPEG2_SLICE_PARAMS , + }, .codec =CEDRUS_CODEC_MPEG2 , .required = true, }, { - .id =V4L2_CID_MPEG_VIDEO_MPEG2_QUANTIZATION , - .elem_size =sizeof (struct v4l2_ctrl_mpeg2_quantization ), + .cfg = { + .id =V4L2_CID_MPEG_VIDEO_MPEG2_QUANTIZATION , + }, .codec =CEDRUS_CODEC_MPEG2 , .required = false, }, { - .id =V4L2_CID_MPEG_VIDEO_H264_DECODE_PARAMS , - .elem_size =sizeof (struct v4l2_ctrl_h264_decode_params ), + .cfg = { + .id =V4L2_CID_MPEG_VIDEO_H264_DECODE_PARAMS , + }, .codec =CEDRUS_CODEC_H264 , .required = true, }, { - .id =V4L2_CID_MPEG_VIDEO_H264_SLICE_PARAMS , - .elem_size =sizeof (struct v4l2_ctrl_h264_slice_params ), + .cfg = { + .id =V4L2_CID_MPEG_VIDEO_H264_SLICE_PARAMS , + }, .codec =CEDRUS_CODEC_H264 , .required = true, }, { - .id =V4L2_CID_MPEG_VIDEO_H264_SPS , - .elem_size =sizeof (struct v4l2_ctrl_h264_sps ), + .cfg = { + .id =V4L2_CID_MPEG_VIDEO_H264_SPS , + }, .codec =CEDRUS_CODEC_H264 , .required = true, }, { - .id =V4L2_CID_MPEG_VIDEO_H264_PPS , - .elem_size =sizeof (struct v4l2_ctrl_h264_pps ), + .cfg = { + .id =V4L2_CID_MPEG_VIDEO_H264_PPS , + }, .codec =CEDRUS_CODEC_H264 , .required = true, }, { - .id =V4L2_CID_MPEG_VIDEO_H264_SCALING_MATRIX , - .elem_size =sizeof (struct v4l2_ctrl_h264_scaling_matrix ), + .cfg = { + .id =V4L2_CID_MPEG_VIDEO_H264_SCALING_MATRIX , + }, .codec =CEDRUS_CODEC_H264 , .required = true, },@@ -106,12 +113,8 @@ static int cedrus_init_ctrls(struct cedrus_dev *dev, struct cedrus_ctx *ctx)return -ENOMEM ;for (i =0 ; i <CEDRUS_CONTROLS_COUNT ; i++) { -struct v4l2_ctrl_config cfg = {}; - - cfg.elem_size = cedrus_controls[i].elem_size; - cfg.id = cedrus_controls[i].id; - - ctrl =v4l2_ctrl_new_custom (hdl, &cfg,NULL ); + ctrl =v4l2_ctrl_new_custom (hdl, &cedrus_controls[i].cfg, +NULL );if (hdl->error) {v4l2_err (&dev->v4l2_dev,"Failed to create new custom control\n" );@@ -178,7 +181,7 @@ static int cedrus_request_validate(struct media_request *req)continue ; ctrl_test =v4l2_ctrl_request_hdl_ctrl_find (hdl, - cedrus_controls[i].id); + cedrus_controls[i].cfg.id);if (!ctrl_test) {v4l2_info (&ctx->dev->v4l2_dev,"Missing required codec control\n" );drivers/staging/media/sunxi/cedrus/cedrus.h +1
-2@@ -49,8 +49,7 @@ enum cedrus_h264_pic_type { };struct cedrus_control { -u32 id; -u32 elem_size; +struct v4l2_ctrl_config cfg;enum cedrus_codec codec;unsigned char required:1 ; };Signature · 1 line
2.22.0
Signature · 3 lines
Paul Kocialkowski, Bootlin Embedded Linux and kernel engineering https://bootlin.com
PATCH v7 7/11 media: cedrus: Specify H264 startcode and decoding mode
In reply to Ezequiel GarciaThe cedrus VPU is slice-based and expects V4L2_PIX_FMT_H264_SLICE buffers to contain H264 slices with no start code.
Expose this to userspace with the newly added menu control.
These two controls are specified as mandatory for applications, but we mark them as non-required on the driver side for backwards compatibility.
---
Changes in v7:
* None.
Changes in v6:
* Remove incorrect menu_skip_mask.
Changes in v6:
* Adjust to control renames.
Changes in v5:
* Clarify commit log.
Changes in v4:
* New patch.
--- drivers/staging/media/sunxi/cedrus/cedrus.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
drivers/staging/media/sunxi/cedrus/cedrus.c +18 -0
@@ -77,6 +77,24 @@ static const struct cedrus_control cedrus_controls[] = { .codec = CEDRUS_CODEC_H264, .required = true, }, + { + .cfg = { + .id = V4L2_CID_MPEG_VIDEO_H264_DECODE_MODE, + .max = V4L2_MPEG_VIDEO_H264_DECODE_MODE_SLICE_BASED, + .def = V4L2_MPEG_VIDEO_H264_DECODE_MODE_SLICE_BASED, + }, + .codec = CEDRUS_CODEC_H264, + .required = false, + }, + { + .cfg = { + .id = V4L2_CID_MPEG_VIDEO_H264_START_CODE, + .max = V4L2_MPEG_VIDEO_H264_START_CODE_NONE, + .def = V4L2_MPEG_VIDEO_H264_START_CODE_NONE, + }, + .codec = CEDRUS_CODEC_H264, + .required = false, + }, };#define CEDRUS_CONTROLS_COUNT ARRAY_SIZE(cedrus_controls)
Signature · 1 line
2.22.0
Re: [PATCH v7 07/11] media: cedrus: Specify H264 startcode and decoding mode
Hi,
Quoted 2 lines · Ezequiel Garcia
The cedrus VPU is slice-based and expects
V4L2_PIX_FMT_H264_SLICEbuffers to contain H264 slices with no start code.
For clarification, cedrus does not expect any of the two options we have specified but something in-between that is rather broken (a single frame composed of a single slice only, in the format that our userspace currently fills).
So at this point, the uAPI is not yet harmonized across cedrus and hantro although the new controls from this series are exposed on both.
This situation makes me realize that perhaps we should have a formal discussion on the roadmap we want to take for stabilizing the API. I see many points that need significant adjustments and the new controls that allow Rockchip support are just one aspect.
Having both drivers abide by the API seems like the next logical step (as far as I understood, this is what Hans is expecting now), but I want to stress the fact that it is really not enough to make a proper API and things are still pretty broken all around.
Cheers,
Paul
Quoted 54 lines
Expose this to userspace with the newly added menu control.
These two controls are specified as mandatory for applications, but we mark them as non-required on the driver side for backwards compatibility.
Signed-off-by Ezequiel Garcia <[email protected]>---
Changes in v7:
* None.
Changes in v6:
* Remove incorrectmenu_skip_mask.
Changes in v6:
* Adjust to control renames.
Changes in v5:
* Clarify commit log.
Changes in v4:
* New patch.--- drivers/staging/media/sunxi/cedrus/cedrus.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)drivers/staging/media/sunxi/cedrus/cedrus.c +18
-0@@ -77,6 +77,24 @@ static const struct cedrus_control cedrus_controls[] = { .codec = CEDRUS_CODEC_H264, .required = true, }, + { + .cfg = { + .id = V4L2_CID_MPEG_VIDEO_H264_DECODE_MODE, + .max = V4L2_MPEG_VIDEO_H264_DECODE_MODE_SLICE_BASED, + .def = V4L2_MPEG_VIDEO_H264_DECODE_MODE_SLICE_BASED, + }, + .codec = CEDRUS_CODEC_H264, + .required = false, + }, + { + .cfg = { + .id = V4L2_CID_MPEG_VIDEO_H264_START_CODE, + .max = V4L2_MPEG_VIDEO_H264_START_CODE_NONE, + .def = V4L2_MPEG_VIDEO_H264_START_CODE_NONE, + }, + .codec = CEDRUS_CODEC_H264, + .required = false, + }, };#define CEDRUS_CONTROLS_COUNT ARRAY_SIZE(cedrus_controls)Signature · 1 line
2.22.0
Signature · 3 lines
Paul Kocialkowski, Bootlin Embedded Linux and kernel engineering https://bootlin.com
PATCH v7 8/11 media: hantro: Move copy_metadata() before doing a decode operation
In reply to Ezequiel GarciaFrom: Boris Brezillon <[email protected]>
Some decoders use intra slice/frame references. The capture buffer pointed by these references might be new and thus have invalid timestamp which prevents the decoder logic from retrieving the vb2_buffer object based on the output buf timestamp.
Copy all metadata (including the timestamp) before starting the decode operation.
---
Changes in v7:
* None.
Changes in v6:
* None.
Changes in v5:
* None.
Changes in v4:
* None.
--- drivers/staging/media/hantro/hantro_drv.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
drivers/staging/media/hantro/hantro_drv.c +6 -2
@@ -111,8 +111,6 @@ static void hantro_job_finish(struct hantro_dev *vpu, src->sequence = ctx->sequence_out++; dst->sequence = ctx->sequence_cap++; -v4l2_m2m_buf_copy_metadata (src, dst, true); - ret = ctx->buf_finish (ctx, &dst->vb2_buf, bytesused);if (ret) result =VB2_BUF_STATE_ERROR ;
@@ -178,8 +176,12 @@ void hantro_finish_run(struct hantro_ctx *ctx)static void device_run (void *priv) {struct hantro_ctx *ctx = priv; +struct vb2_v4l2_buffer *src, *dst;int ret; + src =hantro_get_src_buf (ctx); + dst =hantro_get_dst_buf (ctx); + ret =clk_bulk_enable (ctx->dev->variant->num_clocks, ctx->dev->clocks);if (ret) goto err_cancel_job;
@@ -187,6 +189,8 @@ static void device_run(void *priv)if (ret <0 ) goto err_cancel_job; +v4l2_m2m_buf_copy_metadata (src, dst, true); + ctx->codec_ops->run (ctx);return ;
Signature · 1 line
2.22.0
PATCH v7 9/11 media: hantro: Add core bits to support H264 decoding
In reply to Ezequiel GarciaFrom: Hertz Wong <[email protected]>
Add helpers and patch hantro_{drv,v4l2}.c to prepare addition of H264 decoding support.
---
Changes in v7:
* Comment on the source of the CABAC constant table.
* Remove unneeded menu_skip_mask.
Changes in v6:
* Fixed duplicated CABAC table memcpy.
* Adjust to renamed controls.
Changes in v5:
* None.
Changes in v4:
* Rework extra_size0, exposing the size via TRY_FMT/S_FMT
to allow buffer importation, as suggested by Tomasz.
* Drop max slice limit.
* Use a ternary operator instead of substracting POCs,
to avoid an overflow as pointed out by Rasmus.
* Specify annex B slice start code.
* Add missing extra_size0 to sizeimage
* Swap the first 2 entries of list B1 when B0 and B1 match (mandated by the spec)
* Move the update_dpb() call before the prepare_table() one to make the POCs stored in the private table match the content of the new DPB
--- drivers/staging/media/hantro/Makefile | 1 + drivers/staging/media/hantro/hantro.h | 9 +- drivers/staging/media/hantro/hantro_drv.c | 42 ++ drivers/staging/media/hantro/hantro_h264.c | 646 +++++++++++++++++++++ drivers/staging/media/hantro/hantro_hw.h | 55 ++ drivers/staging/media/hantro/hantro_v4l2.c | 10 + 6 files changed, 762 insertions(+), 1 deletion(-) create mode 100644 drivers/staging/media/hantro/hantro_h264.c
drivers/staging/media/hantro/Makefile +1 -0
@@ -10,6 +10,7 @@ hantro-vpu-y += \ rk3399_vpu_hw_mpeg2_dec.o \ rk3399_vpu_hw_vp8_dec.o \ hantro_jpeg.o \ + hantro_h264.o \ hantro_mpeg2.o \ hantro_vp8.o
drivers/staging/media/hantro/hantro.h +8 -1
@@ -30,6 +30,10 @@#define VP8_MB_WIDTH (w) DIV_ROUND_UP(w, VP8_MB_DIM)#define VP8_MB_HEIGHT (h) DIV_ROUND_UP(h, VP8_MB_DIM) +#define H264_MB_DIM 16 +#define H264_MB_WIDTH (w) DIV_ROUND_UP(w, H264_MB_DIM) +#define H264_MB_HEIGHT (h) DIV_ROUND_UP(h, H264_MB_DIM) +#define MPEG2_MB_DIM 16#define MPEG2_MB_WIDTH (w) DIV_ROUND_UP(w, MPEG2_MB_DIM)#define MPEG2_MB_HEIGHT (h) DIV_ROUND_UP(h, MPEG2_MB_DIM)
@@ -43,9 +47,9 @@ struct hantro_codec_ops;#define HANTRO_JPEG_ENCODER BIT(0)#define HANTRO_ENCODERS 0x0000ffff -#define HANTRO_MPEG2_DECODER BIT(16)#define HANTRO_VP8_DECODER BIT(17) +#define HANTRO_H264_DECODER BIT(18)#define HANTRO_DECODERS 0xffff0000/**
@@ -102,12 +106,14 @@ struct hantro_variant {* enum hantro_codec_mode - codec operating mode. * @HANTRO_MODE_NONE: No operating mode. Used for RAW video formats. * @HANTRO_MODE_JPEG_ENC: JPEG encoder. +* @HANTRO_MODE_H264_DEC: H264 decoder. * @HANTRO_MODE_MPEG2_DEC: MPEG-2 decoder. * @HANTRO_MODE_VP8_DEC: VP8 decoder. */ enum hantro_codec_mode {HANTRO_MODE_NONE =-1 ,HANTRO_MODE_JPEG_ENC , +HANTRO_MODE_H264_DEC ,HANTRO_MODE_MPEG2_DEC ,HANTRO_MODE_VP8_DEC , };
@@ -246,6 +252,7 @@ struct hantro_ctx {/* Specific for particular codec modes. */ union { +struct hantro_h264_dec_hw_ctx h264_dec;struct hantro_jpeg_enc_hw_ctx jpeg_enc;struct hantro_mpeg2_dec_hw_ctx mpeg2_dec;struct hantro_vp8_dec_hw_ctx vp8_dec;
drivers/staging/media/hantro/hantro_drv.c +42 -0
48 lines not on this page · raw message
drivers/staging/media/hantro/hantro_h264.c new file +646 -0
646 lines not on this page · raw message
drivers/staging/media/hantro/hantro_hw.h +55 -0
73 lines not on this page · raw message
drivers/staging/media/hantro/hantro_v4l2.c +10 -0
@@ -239,6 +239,15 @@ static int vidioc_try_fmt(struct file *file, void *priv, struct v4l2_format *f,/* Fill remaining fields */ v4l2_fill_pixfmt_mp (pix_mp, fmt->fourcc, pix_mp->width, pix_mp->height); +/* +* The H264 decoder needs extra space on the output buffers +* to store motion vectors. This is needed for reference +* frames. +*/ +if (ctx->vpu_src_fmt->fourcc ==V4L2_PIX_FMT_H264_SLICE ) + pix_mp->plane_fmt[0 ].sizeimage += +128 *DIV_ROUND_UP (pix_mp->width,16 ) * +DIV_ROUND_UP (pix_mp->height,16 ); }else if (!pix_mp->plane_fmt[0 ].sizeimage) {/* * For coded formats the application can specify
@@ -345,6 +354,7 @@ hantro_update_requires_request(struct hantro_ctx *ctx, u32 fourcc)break ;case V4L2_PIX_FMT_MPEG2_SLICE :case V4L2_PIX_FMT_VP8_FRAME : +case V4L2_PIX_FMT_H264_SLICE : ctx->fh.m2m_ctx->out_q_ctx.q.requires_requests = true;break ;default :
Signature · 1 line
2.22.0
PATCH v7 10/11 media: hantro: Add support for H264 decoding on G1
In reply to Ezequiel GarciaFrom: Hertz Wong <[email protected]>
Add the G1 specific bits to support H264 decoding.
---
Changes from v7:
* None.
Changes from v6:
* None.
Changes from v5:
* None.
Changes from v4:
* Update to use pic_size from the H264 private context struct.
* Remove unused variable.
--- drivers/staging/media/hantro/Makefile | 1 + .../staging/media/hantro/hantro_g1_h264_dec.c | 292 ++++++++++++++++++ drivers/staging/media/hantro/hantro_hw.h | 1 + 3 files changed, 294 insertions(+) create mode 100644 drivers/staging/media/hantro/hantro_g1_h264_dec.c
drivers/staging/media/hantro/Makefile +1 -0
@@ -4,6 +4,7 @@ hantro-vpu-y += \ hantro_drv.o \ hantro_v4l2.o \ hantro_h1_jpeg_enc.o \ + hantro_g1_h264_dec.o \ hantro_g1_mpeg2_dec.o \ hantro_g1_vp8_dec.o \ rk3399_vpu_hw_jpeg_enc.o \
drivers/staging/media/hantro/hantro_g1_h264_dec.c new file +292 -0
292 lines not on this page · raw message
drivers/staging/media/hantro/hantro_hw.h +1 -0
@@ -161,6 +161,7 @@ void hantro_jpeg_enc_exit(struct hantro_ctx *ctx);struct vb2_buffer *hantro_h264_get_ref_buf (struct hantro_ctx *ctx,unsigned int dpb_idx);int hantro_h264_dec_prepare_run (struct hantro_ctx *ctx); +void hantro_g1_h264_dec_run (struct hantro_ctx *ctx);int hantro_h264_dec_init (struct hantro_ctx *ctx);void hantro_h264_dec_exit (struct hantro_ctx *ctx);
Signature · 1 line
2.22.0
PATCH v7 11/11 media: hantro: Enable H264 decoding on rk3288
In reply to Ezequiel GarciaFrom: Hertz Wong <[email protected]>
Now that the generic bits have been added, we can activate H264 decoding on rk3288.
---
Changes in v7:
* None.
Changes in v6:
* None.
Changes in v5:
* None.
Changes in v4:
* None.
--- drivers/staging/media/hantro/rk3288_vpu_hw.c | 21 +++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-)
drivers/staging/media/hantro/rk3288_vpu_hw.c +20 -1
@@ -61,6 +61,19 @@ static const struct hantro_fmt rk3288_vpu_dec_fmts[] = { .fourcc = V4L2_PIX_FMT_NV12, .codec_mode = HANTRO_MODE_NONE, }, + { + .fourcc = V4L2_PIX_FMT_H264_SLICE, + .codec_mode = HANTRO_MODE_H264_DEC, + .max_depth = 2, + .frmsize = { + .min_width = 48, + .max_width = 3840, + .step_width = H264_MB_DIM, + .min_height = 48, + .max_height = 2160, + .step_height = H264_MB_DIM, + }, + }, { .fourcc = V4L2_PIX_FMT_MPEG2_SLICE, .codec_mode = HANTRO_MODE_MPEG2_DEC,
@@ -162,6 +175,12 @@ static const struct hantro_codec_ops rk3288_vpu_codec_ops[] = { .init = hantro_jpeg_enc_init, .exit = hantro_jpeg_enc_exit, }, + [HANTRO_MODE_H264_DEC] = { + .run = hantro_g1_h264_dec_run, + .reset = rk3288_vpu_dec_reset, + .init = hantro_h264_dec_init, + .exit = hantro_h264_dec_exit, + }, [HANTRO_MODE_MPEG2_DEC] = { .run = hantro_g1_mpeg2_dec_run, .reset = rk3288_vpu_dec_reset,
@@ -197,7 +216,7 @@ const struct hantro_variant rk3288_vpu_variant = { .dec_fmts = rk3288_vpu_dec_fmts, .num_dec_fmts = ARRAY_SIZE(rk3288_vpu_dec_fmts), .codec =HANTRO_JPEG_ENCODER |HANTRO_MPEG2_DECODER | - HANTRO_VP8_DECODER, + HANTRO_VP8_DECODER | HANTRO_H264_DECODER, .codec_ops = rk3288_vpu_codec_ops, .irqs = rk3288_irqs, .num_irqs = ARRAY_SIZE(rk3288_irqs),
Signature · 1 line
2.22.0