| Message ID | 20210524204355.6312-2-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/2] avcodec/error_resilience: cleanup mpeg2 handling | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
Quoting Michael Niedermayer (2021-05-24 22:43:55) > This should fix some minor race condition > Untested (no testcase, the flag is not tested by fate) How will this fix the race? It is still modifying a shared struct.
On 5/31/2021 6:02 AM, Anton Khirnov wrote: > Quoting Michael Niedermayer (2021-05-24 22:43:55) >> This should fix some minor race condition >> Untested (no testcase, the flag is not tested by fate) > > How will this fix the race? It is still modifying a shared struct. s->cur_pic.f->decode_error_flags is already being set in ff_er_frame_end(), and valgrind hasn't complained about it, so it may be safe to touch it here.
On 5/24/2021 5:43 PM, Michael Niedermayer wrote: > This should fix some minor race condition > Untested (no testcase, the flag is not tested by fate) > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/error_resilience.c | 3 +++ > libavcodec/h264dec.c | 5 ----- > 2 files changed, 3 insertions(+), 5 deletions(-) > > diff --git a/libavcodec/error_resilience.c b/libavcodec/error_resilience.c > index 68bc10ac31..e880efcc51 100644 > --- a/libavcodec/error_resilience.c > +++ b/libavcodec/error_resilience.c > @@ -907,6 +907,9 @@ void ff_er_frame_end(ERContext *s) > int is_intra_likely; > int size = s->b8_stride * 2 * s->mb_height; > > + if (s->error_occurred && s->cur_pic.f) s->cur_pic.f is accessed unconditionally right below this chunk, so no need for that check. > + s->cur_pic.f->decode_error_flags |= FF_DECODE_ERROR_DECODE_SLICES; You may want to merge or group setting this flag with the FF_DECODE_ERROR_CONCEALMENT_ACTIVE one a couple hundred lines into the function. > + > /* We do not support ER of field pictures yet, > * though it should not crash if enabled. */ > if (!s->avctx->error_concealment || !atomic_load(&s->error_count) || > diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c > index 38f8967265..05e46cb456 100644 > --- a/libavcodec/h264dec.c > +++ b/libavcodec/h264dec.c > @@ -716,11 +716,6 @@ 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: > >
diff --git a/libavcodec/error_resilience.c b/libavcodec/error_resilience.c index 68bc10ac31..e880efcc51 100644 --- a/libavcodec/error_resilience.c +++ b/libavcodec/error_resilience.c @@ -907,6 +907,9 @@ void ff_er_frame_end(ERContext *s) int is_intra_likely; int size = s->b8_stride * 2 * s->mb_height; + if (s->error_occurred && s->cur_pic.f) + s->cur_pic.f->decode_error_flags |= FF_DECODE_ERROR_DECODE_SLICES; + /* We do not support ER of field pictures yet, * though it should not crash if enabled. */ if (!s->avctx->error_concealment || !atomic_load(&s->error_count) || diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c index 38f8967265..05e46cb456 100644 --- a/libavcodec/h264dec.c +++ b/libavcodec/h264dec.c @@ -716,11 +716,6 @@ 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:
This should fix some minor race condition Untested (no testcase, the flag is not tested by fate) Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/error_resilience.c | 3 +++ libavcodec/h264dec.c | 5 ----- 2 files changed, 3 insertions(+), 5 deletions(-)