Re: [PATCH v3 00/24] H.264 Field Decoding Support for Frame-based Decoders

[Nicolas Dufresne <nicolas.dufresne@collabora.com>]

Hello everyone,

its not clear if that cover made it this time, though it will now. Adding a
comment below ...

Le mardi 05 avril 2022 à 16:44 -0400, Nicolas Dufresne a écrit :
> Until now, only Cedrus (a slice base decoder) supported interlaced
> decoding.  In order to support field decoding in our frame-based decoder,
> the v4l2-h264 library needed adaptation to produce the appropriate
> reference lists.
> 
> This patch extends the v4l2-h264 library to produce the larger references
> list needed to represent fields separately. Hantro, MTK-VCODEC and RKVDEC
> drivers have been adapted to accommodate the larger lists. Though, only
> Hantro and RKVDEC actually have HW support for field decoding. So only
> these two have been updated to make use of the larger lists. All this work
> has been done using the H.264 specification, LibreELEC downstream kernel
> patches, Rockchip MPP reference software and Hantro reference software.
> 
> For reviewers, the following is the map of all commit. Patches that could
> be merge independently of this serie are marked as independent. Note that
> the test results do depend on the generic fixes.
> 
> 01    : Documentation fix (independent)
> 02-03 : Improving some generic traces (independent)
> 04    : Minor v4l2-h264 fix (independent)
> 05-11 : v4l2-h264 field decoding support
> 12-16 : rkvdec h.264 generic fixes (independent)
> 17-20 : rkvdec h.264 field decoding support
> 21-24 : hantro h.264 field decoding support
> 
> All this work have been tested using GStreamer mainline implementation
> but also with FFMPEG LibreELEC fork using the testing tool fluster
> running through the ITU-T H.264 (2016-02) AVCv2 set of bitstream. Before
> this patch, the scores were:
> 
> Hantro:
>   FFMPEG:   88/135
>   GSteamer: 90/135
> RKVDEC:
>   FFMPEG:   73/135
>   GSteamer: 77/135
> 
> And after these changes:
> 
> Hantro:
>   FFMPEG:   118/135
>   GSteamer: 129/135

I have also tested on IMX8MQ now, same results. This exercise the
hantro_g1_h264.c code.

> RKVDEC:
>   FFMPEG:   118/135
>   GSteamer: 129/135
> 
> Note that a bug in FFMPEG / LibreELEC fork was noticed and fixed with the
> following change:
> 
> diff --git a/libavcodec/v4l2_request_h264.c b/libavcodec/v4l2_request_h264.c
> index 88da8f0a2d..394bae0550 100644
> --- a/libavcodec/v4l2_request_h264.c
> +++ b/libavcodec/v4l2_request_h264.c
> @@ -66,7 +66,7 @@ static void fill_dpb_entry(struct v4l2_h264_dpb_entry *entry, const H264Picture
>  {
>      entry->reference_ts = ff_v4l2_request_get_capture_timestamp(pic->f);
>      entry->pic_num = pic->pic_id;
> -    entry->frame_num = pic->frame_num;
> +    entry->frame_num = pic->long_ref ? pic->pic_id : pic->frame_num;
>      entry->fields = pic->reference & V4L2_H264_FRAME_REF;
>      entry->flags = V4L2_H264_DPB_ENTRY_FLAG_VALID;
>      if (entry->fields)
> 
> Some useful links:
> 
> Detailed Hantro Results:     https://gitlab.freedesktop.org/-/snippets/5189
> Detailed RKVDEC Results:     https://gitlab.freedesktop.org/-/snippets/5253
> ITU-T H.264 (2016-02) AVCv2: https://www.itu.int/net/itu-t/sigdb/spevideo/VideoForm-s.aspx?val=102002641
> Fluster:                     https://github.com/fluendo/fluster
> GStreamer:                   https://gitlab.freedesktop.org/gstreamer/gstreamer/
> FFMPEG Fork:                 https://github.com/jernejsk/FFmpeg/tree/v4l2-request-hwaccel-4.4
> Rockchip MPP:                https://github.com/rockchip-linux/mpp
> 
> Changes in v3:
> - Improved debug message on timestamp miss-match
> - Moved H264 SPS validation into rkvdec-h264
> - Added more comments around H264 SPS validation
> - Also validate at streamon (rkvdec start())
> - Applied more Review-by and Fixes tag
> - Fixed Signed-off-by chain in Jonas patch
> 
> Changes in v2:
> - Applied most of Sebastian's suggestion in comments and commit messages.
> - Use a bool for dpb_valid and dpb_bottom in rkvdec
> - Dropped one wrong typo fix (media: v4l2-mem2mem: Fix typo in trace message)
> - Dropped Alex fix (media: rkvdec-h264: Don't hardcode SPS/PPS parameters
>   + I will carry this one later, it seems cosmetic
> 
> Jonas Karlman (5):
>   media: rkvdec: h264: Fix bit depth wrap in pps packet
>   media: rkvdec: h264: Validate and use pic width and height in mbs
>   media: rkvdec: h264: Fix reference frame_num wrap for second field
>   media: rkvdec: Ensure decoded resolution fit coded resolution
>   media: hantro: h264: Make dpb entry management more robust
> 
> Nicolas Dufresne (18):
>   media: doc: Document dual use of H.264 pic_num/frame_num
>   media: v4l2-mem2mem: Trace on implicit un-hold
>   media: h264: Avoid wrapping long_term_frame_idx
>   media: h264: Use v4l2_h264_reference for reflist
>   media: h264: Increase reference lists size to 32
>   media: h264: Store current picture fields
>   media: h264: Store all fields into the unordered list
>   media: v4l2: Trace calculated p/b0/b1 initial reflist
>   media: h264: Sort p/b reflist using frame_num
>   media: v4l2: Reorder field reflist
>   media: rkvdec: Stop overclocking the decoder
>   media: rkvdec: h264: Fix dpb_valid implementation
>   media: rkvdec: Move H264 SPS validation in rkvdec-h264
>   media: rkvdec-h264: Add field decoding support
>   media: rkvdec: Enable capture buffer holding for H264
>   media: hantro: Stop using H.264 parameter pic_num
>   media: hantro: Add H.264 field decoding support
>   media: hantro: Enable HOLD_CAPTURE_BUF for H.264
> 
> Sebastian Fricke (1):
>   media: videobuf2-v4l2: Warn on holding buffers without support
> 
>  .../media/v4l/ext-ctrls-codec-stateless.rst   |  10 +-
>  .../media/common/videobuf2/videobuf2-v4l2.c   |   7 +-
>  .../mediatek/vcodec/vdec/vdec_h264_req_if.c   |  17 +-
>  drivers/media/v4l2-core/v4l2-h264.c           | 261 ++++++++++++++----
>  drivers/media/v4l2-core/v4l2-mem2mem.c        |   1 +
>  .../staging/media/hantro/hantro_g1_h264_dec.c |  38 +--
>  drivers/staging/media/hantro/hantro_h264.c    | 119 ++++++--
>  drivers/staging/media/hantro/hantro_hw.h      |   7 +-
>  drivers/staging/media/hantro/hantro_v4l2.c    |  25 ++
>  .../media/hantro/rockchip_vpu2_hw_h264_dec.c  |  98 +++----
>  drivers/staging/media/rkvdec/rkvdec-h264.c    | 154 ++++++++---
>  drivers/staging/media/rkvdec/rkvdec.c         |  35 +--
>  drivers/staging/media/rkvdec/rkvdec.h         |   2 +
>  include/media/v4l2-h264.h                     |  31 ++-
>  14 files changed, 580 insertions(+), 225 deletions(-)
> 
> -- 
> 2.34.1
> 
>