[FFmpeg-devel] [PATCH] set AVFrame decode_error_flags in case of decoding error by h264dec

[Anton Khirnov]

Quoting James Almer (2021-05-24 19:20:19)
> On 7/7/2019 5:15 PM, Michael Niedermayer wrote:
> > On Fri, Jun 21, 2019 at 07:15:17AM -0700, Amir Pauker wrote:
> >> set AVFrame decode_error_flags in case h->slice_ctx->er.error_occurred is set
> >> after the call to ff_h264_execute_decode_slices. This allows the user to detect
> >> concealed decoding errors in the call to avcodec_receive_frame
> >>
> >> Signed-off-by: Amir Pauker <amir at livelyvideo.tv>
> >> ---
> >>   libavcodec/error_resilience.c | 2 ++
> >>   libavcodec/h264dec.c          | 5 +++++
> >>   2 files changed, 7 insertions(+)
> >>
> >> diff --git a/libavcodec/error_resilience.c b/libavcodec/error_resilience.c
> >> index 35d0c60..ca22871 100644
> >> --- a/libavcodec/error_resilience.c
> >> +++ b/libavcodec/error_resilience.c
> >> @@ -1121,6 +1121,8 @@ void ff_er_frame_end(ERContext *s)
> >>       av_log(s->avctx, AV_LOG_INFO, "concealing %d DC, %d AC, %d MV errors in %c frame\n",
> >>              dc_error, ac_error, mv_error, av_get_picture_type_char(s->cur_pic.f->pict_type));
> >>   
> >> +    s->cur_pic.f->decode_error_flags |= FF_DECODE_ERROR_CONCEALMENT_ACTIVE;
> >> +
> >>       is_intra_likely = is_intra_more_likely(s);
> >>   
> >>       /* set unknown mb-type to most likely */
> >> diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
> >> index 837c3b7..8d1bd16 100644
> >> --- a/libavcodec/h264dec.c
> >> +++ b/libavcodec/h264dec.c
> >> @@ -761,6 +761,11 @@ static int decode_nal_units(H264Context *h, const uint8_t *buf, int buf_size)
> >>       if (ret < 0 && (h->avctx->err_recognition & AV_EF_EXPLODE))
> >>           goto end;
> >>   
> >> +    // set decode_error_flags to allow users to detect concealed decoding errors
> >> +    if ((ret < 0 || h->slice_ctx->er.error_occurred) && h->cur_pic_ptr) {
> >> +        h->cur_pic_ptr->f->decode_error_flags |= FF_DECODE_ERROR_DECODE_SLICES;
> >> +    }
> >> +
> >>       ret = 0;
> >>   end:
> > 
> > will split and apply
> > 
> > thx
> 
> This change seems to have produced FATE failures when run under tsan.
> http://fate.ffmpeg.org/report.cgi?time=20210524021410&slot=x86_64-archlinux-gcc-tsan
> 
> That being said, TSAN runs show a lot of issues that have remained 
> unresolved for a long while. No idea if they are false positives, or if 
> they are benign and rarely (if at all) have any effect on real world 
> usage (Like this one, apparently), but what i can say is that they are 
> masking new and potentially bad threading bugs that nobody will notice 
> in a timely manner.

Modifying frames in the DPB is a race, this flag should be applied on
the output frame rather than the DPB frame.

-- 
Anton Khirnov