[FFmpeg-devel] [PATCH] avcodec/jpeg2000dec: Move preparations to main thread, fix race

[FFmpeg-devel] [PATCH] avcodec/jpeg2000dec: Move preparations to main thread, fix race

[Andreas Rheinhardt]

jpeg2000_decode_tile() (which is run concurrently by several threads
when using slice threading) currently modifies some joint values
before doing its actual work. This is a data race that happens to work
because all threads set the same values; but it is nevertheless
undefined behaviour.

Fix this by performing said preparatory work in the main thread instead.
This fixes the vsynth(1|2|_lena)-jpeg2000(-97)? FATE-tests when using
TSAN and slice threading.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/jpeg2000dec.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c
index 80b2e2d627..a7eb809f30 100644
--- a/libavcodec/jpeg2000dec.c
+++ b/libavcodec/jpeg2000dec.c
@@ -2080,7 +2080,6 @@ static int jpeg2000_decode_tile(AVCodecContext *avctx, void *td,
     Jpeg2000DecoderContext *s = avctx->priv_data;
     AVFrame *picture = td;
     Jpeg2000Tile *tile = s->tile + jobnr;
-    int x;
 
     tile_codeblocks(s, tile);
 
@@ -2088,17 +2087,6 @@ static int jpeg2000_decode_tile(AVCodecContext *avctx, void *td,
     if (tile->codsty[0].mct)
         mct_decode(s, tile);
 
-    for (x = 0; x < s->ncomponents; x++) {
-        if (s->cdef[x] < 0) {
-            for (x = 0; x < s->ncomponents; x++) {
-                s->cdef[x] = x + 1;
-            }
-            if ((s->ncomponents & 1) == 0)
-                s->cdef[s->ncomponents-1] = 0;
-            break;
-        }
-    }
-
     if (s->precision <= 8) {
         write_frame_8(s, tile, picture, 8);
     } else {
@@ -2537,6 +2525,17 @@ static int jpeg2000_decode_frame(AVCodecContext *avctx, void *data,
     if (ret = jpeg2000_read_bitstream_packets(s))
         goto end;
 
+    for (int x = 0; x < s->ncomponents; x++) {
+        if (s->cdef[x] < 0) {
+            for (x = 0; x < s->ncomponents; x++) {
+                s->cdef[x] = x + 1;
+            }
+            if ((s->ncomponents & 1) == 0)
+                s->cdef[s->ncomponents-1] = 0;
+            break;
+        }
+    }
+
     avctx->execute2(avctx, jpeg2000_decode_tile, picture, NULL, s->numXtiles * s->numYtiles);
 
     jpeg2000_dec_cleanup(s);
-- 
2.32.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avcodec/jpeg2000dec: Move preparations to main thread, fix race

[Andreas Rheinhardt]

Andreas Rheinhardt:
> jpeg2000_decode_tile() (which is run concurrently by several threads
> when using slice threading) currently modifies some joint values
> before doing its actual work. This is a data race that happens to work
> because all threads set the same values; but it is nevertheless
> undefined behaviour.
> 
> Fix this by performing said preparatory work in the main thread instead.
> This fixes the vsynth(1|2|_lena)-jpeg2000(-97)? FATE-tests when using
> TSAN and slice threading.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>  libavcodec/jpeg2000dec.c | 23 +++++++++++------------
>  1 file changed, 11 insertions(+), 12 deletions(-)
> 
> diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c
> index 80b2e2d627..a7eb809f30 100644
> --- a/libavcodec/jpeg2000dec.c
> +++ b/libavcodec/jpeg2000dec.c
> @@ -2080,7 +2080,6 @@ static int jpeg2000_decode_tile(AVCodecContext *avctx, void *td,
>      Jpeg2000DecoderContext *s = avctx->priv_data;
>      AVFrame *picture = td;
>      Jpeg2000Tile *tile = s->tile + jobnr;
> -    int x;
>  
>      tile_codeblocks(s, tile);
>  
> @@ -2088,17 +2087,6 @@ static int jpeg2000_decode_tile(AVCodecContext *avctx, void *td,
>      if (tile->codsty[0].mct)
>          mct_decode(s, tile);
>  
> -    for (x = 0; x < s->ncomponents; x++) {
> -        if (s->cdef[x] < 0) {
> -            for (x = 0; x < s->ncomponents; x++) {
> -                s->cdef[x] = x + 1;
> -            }
> -            if ((s->ncomponents & 1) == 0)
> -                s->cdef[s->ncomponents-1] = 0;
> -            break;
> -        }
> -    }
> -
>      if (s->precision <= 8) {
>          write_frame_8(s, tile, picture, 8);
>      } else {
> @@ -2537,6 +2525,17 @@ static int jpeg2000_decode_frame(AVCodecContext *avctx, void *data,
>      if (ret = jpeg2000_read_bitstream_packets(s))
>          goto end;
>  
> +    for (int x = 0; x < s->ncomponents; x++) {
> +        if (s->cdef[x] < 0) {
> +            for (x = 0; x < s->ncomponents; x++) {
> +                s->cdef[x] = x + 1;
> +            }
> +            if ((s->ncomponents & 1) == 0)
> +                s->cdef[s->ncomponents-1] = 0;
> +            break;
> +        }
> +    }
> +
>      avctx->execute2(avctx, jpeg2000_decode_tile, picture, NULL, s->numXtiles * s->numYtiles);
>  
>      jpeg2000_dec_cleanup(s);

Will apply this patch tomorrow unless there are objections.

- Andreas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".