diff mbox series

[FFmpeg-devel,02/45] avcodec/a64multienc: Mark encoders as init-threadsafe

Message ID 20201127010249.2724610-2-andreas.rheinhardt@gmail.com
State Accepted
Commit 6d4b9695153849ffd484a9d18519dff887d6c237
Headers show
Series [FFmpeg-devel,01/45] avcodec/a64multienc: Fix memleak upon init failure
Related show

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished

Commit Message

Andreas Rheinhardt Nov. 27, 2020, 1:02 a.m. UTC
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavcodec/a64multienc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Anton Khirnov Dec. 4, 2020, 10:44 a.m. UTC | #1
Quoting Andreas Rheinhardt (2020-11-27 02:02:06)
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>  libavcodec/a64multienc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

LGTM
Anton Khirnov Feb. 3, 2021, 3:10 p.m. UTC | #2
Quoting Andreas Rheinhardt (2020-11-27 02:02:06)
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>  libavcodec/a64multienc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Actually seems I was mistaken - render_charset() contains three static
arrays which seem to be used in a very racy manner.
Paul B Mahol Feb. 3, 2021, 3:12 p.m. UTC | #3
On Wed, Feb 3, 2021 at 4:11 PM Anton Khirnov <anton@khirnov.net> wrote:

> Quoting Andreas Rheinhardt (2020-11-27 02:02:06)
> > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> > ---
> >  libavcodec/a64multienc.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
>
> Actually seems I was mistaken - render_charset() contains three static
> arrays which seem to be used in a very racy manner.
>
>
Looks to me easily fixable. No?


> --
> Anton Khirnov
> _______________________________________________
> 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".
Anton Khirnov Feb. 3, 2021, 3:14 p.m. UTC | #4
Quoting Paul B Mahol (2021-02-03 16:12:58)
> On Wed, Feb 3, 2021 at 4:11 PM Anton Khirnov <anton@khirnov.net> wrote:
> 
> > Quoting Andreas Rheinhardt (2020-11-27 02:02:06)
> > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> > > ---
> > >  libavcodec/a64multienc.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > Actually seems I was mistaken - render_charset() contains three static
> > arrays which seem to be used in a very racy manner.
> >
> >
> Looks to me easily fixable. No?

Probably, I didn't investigate in detail.
Watches pelcome as usual.
Andreas Rheinhardt Feb. 3, 2021, 8:18 p.m. UTC | #5
Anton Khirnov:
> Quoting Andreas Rheinhardt (2020-11-27 02:02:06)
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
>> ---
>>  libavcodec/a64multienc.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> Actually seems I was mistaken - render_charset() contains three static
> arrays which seem to be used in a very racy manner.
> 
The reason I didn't spot this is because these arrays are not accessible
from the init function at all: I start reading the init function and
look at all code accessible from it; if it contains no races, I add the
flag to mark the codec as init-threadsafe. Also notice that for this bug
it is irrelevant whether the codec is marked as init-threadsafe.
But will nevertheless fix this.

- Andreas
diff mbox series

Patch

diff --git a/libavcodec/a64multienc.c b/libavcodec/a64multienc.c
index 5f4825d55d..55616c8524 100644
--- a/libavcodec/a64multienc.c
+++ b/libavcodec/a64multienc.c
@@ -407,7 +407,7 @@  AVCodec ff_a64multi_encoder = {
     .close          = a64multi_close_encoder,
     .pix_fmts       = (const enum AVPixelFormat[]) {AV_PIX_FMT_GRAY8, AV_PIX_FMT_NONE},
     .capabilities   = AV_CODEC_CAP_DELAY,
-    .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
+    .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP | FF_CODEC_CAP_INIT_THREADSAFE,
 };
 #endif
 #if CONFIG_A64MULTI5_ENCODER
@@ -422,6 +422,6 @@  AVCodec ff_a64multi5_encoder = {
     .close          = a64multi_close_encoder,
     .pix_fmts       = (const enum AVPixelFormat[]) {AV_PIX_FMT_GRAY8, AV_PIX_FMT_NONE},
     .capabilities   = AV_CODEC_CAP_DELAY,
-    .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
+    .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP | FF_CODEC_CAP_INIT_THREADSAFE,
 };
 #endif