[FFmpeg-devel] [PATCH 2/2] lavc/hevc_refs: Fix the logic of find_ref_idx()
Fu, Linjie
linjie.fu at intel.com
Thu May 21 09:38:22 EEST 2020
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Fu,
> Linjie
> Sent: Monday, May 18, 2020 15:17
> To: ffmpeg-devel at ffmpeg.org
> Cc: Xu, Guangxin <guangxin.xu at intel.com>
> Subject: Re: [FFmpeg-devel] [PATCH 2/2] lavc/hevc_refs: Fix the logic of
> find_ref_idx()
>
> > From: Fu, Linjie <linjie.fu at intel.com>
> > Sent: Tuesday, May 12, 2020 21:44
> > To: ffmpeg-devel at ffmpeg.org
> > Cc: Xu, Guangxin <guangxin.xu at intel.com>; Fu, Linjie <linjie.fu at intel.com>
> > Subject: [PATCH 2/2] lavc/hevc_refs: Fix the logic of find_ref_idx()
> >
> > From: Xu Guangxin <guangxin.xu at intel.com>
> >
> > Currently find_ref_idx() would trigger 2 scans in DPB to find the
> > requested POC:
> > 1. Firstly, ignore MSB of ref->poc and search for the requested POC;
> > 2. Secondly, compare the entire ref->poc with requested POC;
> >
> > For long term reference, we are able to only check LSB if MSB is not
> > presented(e.g. delta_poc_msb_present_flag == 0). However, for short
> > term reference, we should never ignore poc's MSB and it should be
> > kind of bit-exact. (Details in 8.3.2)
> >
> > Otherwise this leads to decoding failures like:
> > [hevc @ 0x5638f4328600] Error constructing the frame RPS.
> > [hevc @ 0x5638f4328600] Error parsing NAL unit #2.
> > [hevc @ 0x5638f4338a80] Could not find ref with POC 21
> > Error while decoding stream #0:0: Invalid data found when processing input
> >
> > Search the requested POC based on whether MSB is used, and avoid
> > the 2-times scan for DPB buffer. This benefits both native HEVC
> > decoder and integrated HW decoders.
> >
> > Signed-off-by: Linjie Fu <linjie.fu at intel.com>
> > ---
> >
> > Since it's kind of difficult to generate an identical bitstream for
> > fate or test, I'd like to elaborate more about one of the failures:
> >
> > requested POC = 5;
> > LtMask = (1 << 4) - 1 = 15;
> > ref[0]->poc = 21; // unexpected ref for poc = 5 (short term)
> > ref[1]->poc = 5; // expected ref for poc = 5 (short term)
> >
> > Hence find_ref_idx() would wrongly return a ref with poc = 21, which
> > leads to the decoding error.
> >
> > libavcodec/hevc_refs.c | 38 ++++++++++++++++++++------------------
> > 1 file changed, 20 insertions(+), 18 deletions(-)
> >
> > diff --git a/libavcodec/hevc_refs.c b/libavcodec/hevc_refs.c
> > index 7870a72..73aa6d8 100644
> > --- a/libavcodec/hevc_refs.c
> > +++ b/libavcodec/hevc_refs.c
> > @@ -358,24 +358,26 @@ int ff_hevc_slice_rpl(HEVCContext *s)
> > return 0;
> > }
> >
> > -static HEVCFrame *find_ref_idx(HEVCContext *s, int poc)
> > +static HEVCFrame *find_ref_idx(HEVCContext *s, int poc, uint8_t
> use_msb)
> > {
> > int i;
> > - int LtMask = (1 << s->ps.sps->log2_max_poc_lsb) - 1;
> >
> > - for (i = 0; i < FF_ARRAY_ELEMS(s->DPB); i++) {
> > - HEVCFrame *ref = &s->DPB[i];
> > - if (ref->frame->buf[0] && (ref->sequence == s->seq_decode)) {
> > - if ((ref->poc & LtMask) == poc)
> > - return ref;
> > + if (use_msb) {
> > + for (i = 0; i < FF_ARRAY_ELEMS(s->DPB); i++) {
> > + HEVCFrame *ref = &s->DPB[i];
> > + if (ref->frame->buf[0] && (ref->sequence == s->seq_decode)) {
> > + if (ref->poc == poc)
> > + return ref;
> > + }
> > }
> > - }
> > -
> > - for (i = 0; i < FF_ARRAY_ELEMS(s->DPB); i++) {
> > - HEVCFrame *ref = &s->DPB[i];
> > - if (ref->frame->buf[0] && ref->sequence == s->seq_decode) {
> > - if (ref->poc == poc || (ref->poc & LtMask) == poc)
> > - return ref;
> > + } else {
> > + int LtMask = (1 << s->ps.sps->log2_max_poc_lsb) - 1;
> > + for (i = 0; i < FF_ARRAY_ELEMS(s->DPB); i++) {
> > + HEVCFrame *ref = &s->DPB[i];
> > + if (ref->frame->buf[0] && ref->sequence == s->seq_decode) {
> > + if ((ref->poc & LtMask) == poc)
> > + return ref;
> > + }
> > }
> > }
> >
> > @@ -427,9 +429,9 @@ static HEVCFrame
> > *generate_missing_ref(HEVCContext *s, int poc)
> >
> > /* add a reference with the given poc to the list and mark it as used in DPB
> > */
> > static int add_candidate_ref(HEVCContext *s, RefPicList *list,
> > - int poc, int ref_flag)
> > + int poc, int ref_flag, uint8_t use_msb)
> > {
> > - HEVCFrame *ref = find_ref_idx(s, poc);
> > + HEVCFrame *ref = find_ref_idx(s, poc, use_msb);
> >
> > if (ref == s->ref || list->nb_refs >= HEVC_MAX_REFS)
> > return AVERROR_INVALIDDATA;
> > @@ -485,7 +487,7 @@ int ff_hevc_frame_rps(HEVCContext *s)
> > else
> > list = ST_CURR_AFT;
> >
> > - ret = add_candidate_ref(s, &rps[list], poc,
> > HEVC_FRAME_FLAG_SHORT_REF);
> > + ret = add_candidate_ref(s, &rps[list], poc,
> > HEVC_FRAME_FLAG_SHORT_REF, 1);
> > if (ret < 0)
> > goto fail;
> > }
> > @@ -495,7 +497,7 @@ int ff_hevc_frame_rps(HEVCContext *s)
> > int poc = long_rps->poc[i];
> > int list = long_rps->used[i] ? LT_CURR : LT_FOLL;
> >
> > - ret = add_candidate_ref(s, &rps[list], poc,
> > HEVC_FRAME_FLAG_LONG_REF);
> > + ret = add_candidate_ref(s, &rps[list], poc,
> > HEVC_FRAME_FLAG_LONG_REF, long_rps->poc_msb_present[i]);
> > if (ret < 0)
> > goto fail;
> > }
> > --
> > 2.7.4
>
> Ping for this patch set.
> Verified with 1000 + decoding conformance cases including 8/10 bit, no
> regression observed.
>
Also see descriptions in chapter 2.4.3 in <High Efficiency Video Coding>[1]:
"Since it is possible to have more than one picture with the same POC LSB value in the DPB, there is an optional
possibility to also signal POC MSB information for long-term pictures."
This patch is to fix the potential " more than one picture with the same POC LSB value in the DPB" issue.
Ping for comments. (Would like to apply the set after this weekend, if no objections)
- Linjie
[1] < http://what-when-how.com/Tutorial/topic-397pct9eq3/High-Efficiency-Video-Coding-HEVC-51.html>
More information about the ffmpeg-devel
mailing list