diff mbox series

[FFmpeg-devel,1/3] error_resilience: set the decode_error_flags outside

Message ID 20230912114015.59937-1-thomas@gllm.fr
State New
Headers show
Series [FFmpeg-devel,1/3] error_resilience: set the decode_error_flags outside | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Thomas Guillem Sept. 12, 2023, 11:40 a.m. UTC
This will allow to fix data-races when ff_er_frame_end() is called after
ff_thread_finish_setup()
---
 libavcodec/error_resilience.c | 12 ++++++------
 libavcodec/error_resilience.h |  2 +-
 libavcodec/h263dec.c          |  6 ++++--
 libavcodec/h264dec.c          |  3 ++-
 libavcodec/mpeg12dec.c        |  3 ++-
 libavcodec/mss2.c             |  8 +++++---
 libavcodec/rv10.c             | 10 ++++++++--
 libavcodec/rv34.c             | 12 +++++++++---
 libavcodec/vc1dec.c           |  6 ++++--
 9 files changed, 41 insertions(+), 21 deletions(-)

Comments

Michael Niedermayer Sept. 12, 2023, 7:05 p.m. UTC | #1
On Tue, Sep 12, 2023 at 01:40:13PM +0200, Thomas Guillem via ffmpeg-devel wrote:
> This will allow to fix data-races when ff_er_frame_end() is called after
> ff_thread_finish_setup()
> ---
>  libavcodec/error_resilience.c | 12 ++++++------
>  libavcodec/error_resilience.h |  2 +-
>  libavcodec/h263dec.c          |  6 ++++--
>  libavcodec/h264dec.c          |  3 ++-
>  libavcodec/mpeg12dec.c        |  3 ++-
>  libavcodec/mss2.c             |  8 +++++---
>  libavcodec/rv10.c             | 10 ++++++++--
>  libavcodec/rv34.c             | 12 +++++++++---
>  libavcodec/vc1dec.c           |  6 ++++--
>  9 files changed, 41 insertions(+), 21 deletions(-)
> 

[...]

> diff --git a/libavcodec/error_resilience.h b/libavcodec/error_resilience.h
> index 47cc8a4fc6..a8cf73c72e 100644
> --- a/libavcodec/error_resilience.h
> +++ b/libavcodec/error_resilience.h
> @@ -90,7 +90,7 @@ typedef struct ERContext {
>  } ERContext;
>  
>  void ff_er_frame_start(ERContext *s);
> -void ff_er_frame_end(ERContext *s);
> +int ff_er_frame_end(ERContext *s);

The return code needs to be documented


>  void ff_er_add_slice(ERContext *s, int startx, int starty, int endx, int endy,
>                       int status);
>  
> diff --git a/libavcodec/h263dec.c b/libavcodec/h263dec.c
> index 52e51dd489..3e83d90586 100644
> --- a/libavcodec/h263dec.c
> +++ b/libavcodec/h263dec.c
> @@ -621,8 +621,10 @@ retry:
>  
>      av_assert1(s->bitstream_buffer_size == 0);
>  frame_end:
> -    if (!s->studio_profile)
> -        ff_er_frame_end(&s->er);
> +    if (!s->studio_profile) {
> +        if (ff_er_frame_end(&s->er) > 0)
> +            s->current_picture.f->decode_error_flags |= FF_DECODE_ERROR_CONCEALMENT_ACTIVE;
> +    }
[...]
> +static void rv10_er_frame_end(MpegEncContext *s)
> +{
> +    if (ff_er_frame_end(&s->er) > 0)
> +        s->current_picture_ptr->f->decode_error_flags |= FF_DECODE_ERROR_CONCEALMENT_ACTIVE;
> +}
[...]
> +static void rv34_er_frame_end(MpegEncContext *s)
> +{
> +    if (ff_er_frame_end(&s->er) > 0)
> +        s->current_picture_ptr->f->decode_error_flags |= FF_DECODE_ERROR_CONCEALMENT_ACTIVE;
> +}
[...]
> +            if (ff_er_frame_end(&s->er) > 0)
> +                s->current_picture.f->decode_error_flags |= FF_DECODE_ERROR_CONCEALMENT_ACTIVE;
> +        }
>      }

This looks like duplicated code


[...]
diff mbox series

Patch

diff --git a/libavcodec/error_resilience.c b/libavcodec/error_resilience.c
index 68e20925e0..1f43b233ff 100644
--- a/libavcodec/error_resilience.c
+++ b/libavcodec/error_resilience.c
@@ -889,7 +889,7 @@  void ff_er_add_slice(ERContext *s, int startx, int starty,
     }
 }
 
-void ff_er_frame_end(ERContext *s)
+int ff_er_frame_end(ERContext *s)
 {
     int *linesize = NULL;
     int i, mb_x, mb_y, error, error_type, dc_error, mv_error, ac_error;
@@ -906,7 +906,7 @@  void ff_er_frame_end(ERContext *s)
         !er_supported(s)                                               ||
         atomic_load(&s->error_count) == 3 * s->mb_width *
                           (s->avctx->skip_top + s->avctx->skip_bottom)) {
-        return;
+        return 0;
     }
     linesize = s->cur_pic.f->linesize;
 
@@ -921,7 +921,7 @@  void ff_er_frame_end(ERContext *s)
 
         if (mb_x == s->mb_width) {
             av_log(s->avctx, AV_LOG_DEBUG, "ignoring last missing slice\n");
-            return;
+            return 0;
         }
     }
 
@@ -960,7 +960,7 @@  void ff_er_frame_end(ERContext *s)
                 s->cur_pic.ref_index[i]  = NULL;
                 s->cur_pic.motion_val[i] = NULL;
             }
-            return;
+            return 0;
         }
     }
 
@@ -1114,8 +1114,6 @@  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 */
@@ -1352,4 +1350,6 @@  void ff_er_frame_end(ERContext *s)
     memset(&s->cur_pic, 0, sizeof(ERPicture));
     memset(&s->last_pic, 0, sizeof(ERPicture));
     memset(&s->next_pic, 0, sizeof(ERPicture));
+
+    return 1;
 }
diff --git a/libavcodec/error_resilience.h b/libavcodec/error_resilience.h
index 47cc8a4fc6..a8cf73c72e 100644
--- a/libavcodec/error_resilience.h
+++ b/libavcodec/error_resilience.h
@@ -90,7 +90,7 @@  typedef struct ERContext {
 } ERContext;
 
 void ff_er_frame_start(ERContext *s);
-void ff_er_frame_end(ERContext *s);
+int ff_er_frame_end(ERContext *s);
 void ff_er_add_slice(ERContext *s, int startx, int starty, int endx, int endy,
                      int status);
 
diff --git a/libavcodec/h263dec.c b/libavcodec/h263dec.c
index 52e51dd489..3e83d90586 100644
--- a/libavcodec/h263dec.c
+++ b/libavcodec/h263dec.c
@@ -621,8 +621,10 @@  retry:
 
     av_assert1(s->bitstream_buffer_size == 0);
 frame_end:
-    if (!s->studio_profile)
-        ff_er_frame_end(&s->er);
+    if (!s->studio_profile) {
+        if (ff_er_frame_end(&s->er) > 0)
+            s->current_picture.f->decode_error_flags |= FF_DECODE_ERROR_CONCEALMENT_ACTIVE;
+    }
 
     if (avctx->hwaccel) {
         ret = FF_HW_SIMPLE_CALL(avctx, end_frame);
diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
index f13b1081fc..553f300c3d 100644
--- a/libavcodec/h264dec.c
+++ b/libavcodec/h264dec.c
@@ -779,7 +779,8 @@  end:
         if (sl->ref_count[1])
             ff_h264_set_erpic(&h->er.next_pic, sl->ref_list[1][0].parent);
 
-        ff_er_frame_end(&h->er);
+        if (ff_er_frame_end(&h->er) > 0)
+            h->cur_pic_ptr->f->decode_error_flags |= FF_DECODE_ERROR_CONCEALMENT_ACTIVE;
         if (use_last_pic)
             memset(&sl->ref_list[0][0], 0, sizeof(sl->ref_list[0][0]));
     }
diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
index 1accd07e9e..b2cc78c3d3 100644
--- a/libavcodec/mpeg12dec.c
+++ b/libavcodec/mpeg12dec.c
@@ -2033,7 +2033,8 @@  static int slice_end(AVCodecContext *avctx, AVFrame *pict)
     if (/* s->mb_y << field_pic == s->mb_height && */ !s->first_field && !s1->first_slice) {
         /* end of image */
 
-        ff_er_frame_end(&s->er);
+        if (ff_er_frame_end(&s->er) > 0)
+            s->current_picture_ptr->f->decode_error_flags |= FF_DECODE_ERROR_CONCEALMENT_ACTIVE;
 
         ff_mpv_frame_end(s);
 
diff --git a/libavcodec/mss2.c b/libavcodec/mss2.c
index 70aa56cb84..29cb4be614 100644
--- a/libavcodec/mss2.c
+++ b/libavcodec/mss2.c
@@ -421,8 +421,12 @@  static int decode_wmv9(AVCodecContext *avctx, const uint8_t *buf, int buf_size,
 
     ff_vc1_decode_blocks(v);
 
+    f = s->current_picture.f;
+
     if (v->end_mb_x == s->mb_width && s->end_mb_y == s->mb_height) {
-        ff_er_frame_end(&s->er);
+        if (ff_er_frame_end(&s->er) > 0)
+             f->decode_error_flags |= FF_DECODE_ERROR_CONCEALMENT_ACTIVE;
+
     } else {
         av_log(v->s.avctx, AV_LOG_WARNING,
                "disabling error correction due to block count mismatch %dx%d != %dx%d\n",
@@ -431,8 +435,6 @@  static int decode_wmv9(AVCodecContext *avctx, const uint8_t *buf, int buf_size,
 
     ff_mpv_frame_end(s);
 
-    f = s->current_picture.f;
-
     if (v->respic == 3) {
         ctx->dsp.upsample_plane(f->data[0], f->linesize[0], w,      h);
         ctx->dsp.upsample_plane(f->data[1], f->linesize[1], w+1 >> 1, h+1 >> 1);
diff --git a/libavcodec/rv10.c b/libavcodec/rv10.c
index 5edd934f82..6d7ae8f903 100644
--- a/libavcodec/rv10.c
+++ b/libavcodec/rv10.c
@@ -440,6 +440,12 @@  static av_cold int rv10_decode_end(AVCodecContext *avctx)
     return 0;
 }
 
+static void rv10_er_frame_end(MpegEncContext *s)
+{
+    if (ff_er_frame_end(&s->er) > 0)
+        s->current_picture_ptr->f->decode_error_flags |= FF_DECODE_ERROR_CONCEALMENT_ACTIVE;
+}
+
 static int rv10_decode_packet(AVCodecContext *avctx, const uint8_t *buf,
                               int buf_size, int buf_size2, int whole_size)
 {
@@ -477,7 +483,7 @@  static int rv10_decode_packet(AVCodecContext *avctx, const uint8_t *buf,
     if ((s->mb_x == 0 && s->mb_y == 0) || !s->current_picture_ptr) {
         // FIXME write parser so we always have complete frames?
         if (s->current_picture_ptr) {
-            ff_er_frame_end(&s->er);
+            rv10_er_frame_end(s);
             ff_mpv_frame_end(s);
             s->mb_x = s->mb_y = s->resync_mb_x = s->resync_mb_y = 0;
         }
@@ -649,7 +655,7 @@  static int rv10_decode_frame(AVCodecContext *avctx, AVFrame *pict,
     }
 
     if (s->current_picture_ptr && s->mb_y >= s->mb_height) {
-        ff_er_frame_end(&s->er);
+        rv10_er_frame_end(s);
         ff_mpv_frame_end(s);
 
         if (s->pict_type == AV_PICTURE_TYPE_B || s->low_delay) {
diff --git a/libavcodec/rv34.c b/libavcodec/rv34.c
index af4d6a3400..cae6e9d81b 100644
--- a/libavcodec/rv34.c
+++ b/libavcodec/rv34.c
@@ -1554,13 +1554,19 @@  static int get_slice_offset(AVCodecContext *avctx, const uint8_t *buf, int n, in
         return buf_size;
 }
 
+static void rv34_er_frame_end(MpegEncContext *s)
+{
+    if (ff_er_frame_end(&s->er) > 0)
+        s->current_picture_ptr->f->decode_error_flags |= FF_DECODE_ERROR_CONCEALMENT_ACTIVE;
+}
+
 static int finish_frame(AVCodecContext *avctx, AVFrame *pict)
 {
     RV34DecContext *r = avctx->priv_data;
     MpegEncContext *s = &r->s;
     int got_picture = 0, ret;
 
-    ff_er_frame_end(&s->er);
+    rv34_er_frame_end(s);
     ff_mpv_frame_end(s);
     s->mb_num_left = 0;
 
@@ -1655,7 +1661,7 @@  int ff_rv34_decode_frame(AVCodecContext *avctx, AVFrame *pict,
             av_log(avctx, AV_LOG_ERROR, "New frame but still %d MB left.\n",
                    s->mb_num_left);
             if (!s->context_reinit)
-                ff_er_frame_end(&s->er);
+                rv34_er_frame_end(s);
             ff_mpv_frame_end(s);
         }
 
@@ -1790,7 +1796,7 @@  int ff_rv34_decode_frame(AVCodecContext *avctx, AVFrame *pict,
             av_log(avctx, AV_LOG_INFO, "marking unfished frame as finished\n");
             /* always mark the current frame as finished, frame-mt supports
              * only complete frames */
-            ff_er_frame_end(&s->er);
+            rv34_er_frame_end(s);
             ff_mpv_frame_end(s);
             s->mb_num_left = 0;
             ff_thread_report_progress(&s->current_picture_ptr->tf, INT_MAX, 0);
diff --git a/libavcodec/vc1dec.c b/libavcodec/vc1dec.c
index b8663aaf98..7113abd0a9 100644
--- a/libavcodec/vc1dec.c
+++ b/libavcodec/vc1dec.c
@@ -1347,8 +1347,10 @@  static int vc1_decode_frame(AVCodecContext *avctx, AVFrame *pict,
         }
         if (   !v->field_mode
             && avctx->codec_id != AV_CODEC_ID_WMV3IMAGE
-            && avctx->codec_id != AV_CODEC_ID_VC1IMAGE)
-            ff_er_frame_end(&s->er);
+            && avctx->codec_id != AV_CODEC_ID_VC1IMAGE) {
+            if (ff_er_frame_end(&s->er) > 0)
+                s->current_picture.f->decode_error_flags |= FF_DECODE_ERROR_CONCEALMENT_ACTIVE;
+        }
     }
 
     ff_mpv_frame_end(s);