| Message ID | AM7PR03MB66603173B9CA69CDBFD72DD38F219@AM7PR03MB6660.eurprd03.prod.outlook.com |
|---|---|
| State | Accepted |
| Commit | 218d1f76d46fc5b59a27d5ec9e88e821dc888d11 |
| Headers | show |
| Series | [FFmpeg-devel] avcodec/jpeg2000dec: Move preparations to main thread, fix race | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_ppc | success | Make finished |
| andriy/make_fate_ppc | success | Make fate finished |
| andriy/make_aarch64_jetson | success | Make finished |
| andriy/make_fate_aarch64_jetson | success | Make fate finished |
| andriy/make_armv7_RPi4 | success | Make finished |
| andriy/make_fate_armv7_RPi4 | success | Make fate finished |
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
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);
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(-)