| Message ID | 20210428095123.2998580-1-plundblad@google.com |
|---|---|
| State | Accepted |
| Commit | da0abbbb01cae21039e2db028467d0f489c9c9ae |
| Headers | show |
| Series | [FFmpeg-devel] libswscale: Make sws_init_context thread safe. | 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 |
Hi, Would anyone consider applying this fairly simple patch to fix a multi-threading issue in the sws initialization code? Regards, //Peter On Wed, Apr 28, 2021 at 11:51 AM Peter Lundblad <plundblad@google.com> wrote: > Call ff_sws_rgb2rgb_init via ff_thread_once instead of checking one of the > variables it updates. > --- > libswscale/utils.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/libswscale/utils.c b/libswscale/utils.c > index 6bac7b658d..d673209d95 100644 > --- a/libswscale/utils.c > +++ b/libswscale/utils.c > @@ -49,6 +49,7 @@ > #include "libavutil/mathematics.h" > #include "libavutil/opt.h" > #include "libavutil/pixdesc.h" > +#include "libavutil/thread.h" > #include "libavutil/aarch64/cpu.h" > #include "libavutil/ppc/cpu.h" > #include "libavutil/x86/asm.h" > @@ -1189,12 +1190,13 @@ av_cold int sws_init_context(SwsContext *c, > SwsFilter *srcFilter, > int ret = 0; > enum AVPixelFormat tmpFmt; > static const float float_mult = 1.0f / 255.0f; > + static AVOnce rgb2rgb_once = AV_ONCE_INIT; > > cpu_flags = av_get_cpu_flags(); > flags = c->flags; > emms_c(); > - if (!rgb15to16) > - ff_sws_rgb2rgb_init(); > + if (ff_thread_once(&rgb2rgb_once, ff_sws_rgb2rgb_init) != 0) > + return AVERROR_UNKNOWN; > > unscaled = (srcW == dstW && srcH == dstH); > > -- > 2.31.1.527.g47e6f16901-goog > >
On Wed, Apr 28, 2021 at 11:51:23AM +0200, Peter Lundblad wrote: > Call ff_sws_rgb2rgb_init via ff_thread_once instead of checking one of the > variables it updates. > --- > libswscale/utils.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/libswscale/utils.c b/libswscale/utils.c > index 6bac7b658d..d673209d95 100644 > --- a/libswscale/utils.c > +++ b/libswscale/utils.c > @@ -49,6 +49,7 @@ > #include "libavutil/mathematics.h" > #include "libavutil/opt.h" > #include "libavutil/pixdesc.h" > +#include "libavutil/thread.h" > #include "libavutil/aarch64/cpu.h" > #include "libavutil/ppc/cpu.h" > #include "libavutil/x86/asm.h" > @@ -1189,12 +1190,13 @@ av_cold int sws_init_context(SwsContext *c, SwsFilter *srcFilter, > int ret = 0; > enum AVPixelFormat tmpFmt; > static const float float_mult = 1.0f / 255.0f; > + static AVOnce rgb2rgb_once = AV_ONCE_INIT; > > cpu_flags = av_get_cpu_flags(); > flags = c->flags; > emms_c(); > - if (!rgb15to16) > - ff_sws_rgb2rgb_init(); > + if (ff_thread_once(&rgb2rgb_once, ff_sws_rgb2rgb_init) != 0) > + return AVERROR_UNKNOWN; > it would be ideal if the use of global/static variables in ff_sws_rgb2rgb_init could be avoided because as it is before and after your patch it doesnt work fully when the cpu flags are manually changed and sws_init_context() called again. That said your patch fixes a race so ill apply it thx [...]
diff --git a/libswscale/utils.c b/libswscale/utils.c index 6bac7b658d..d673209d95 100644 --- a/libswscale/utils.c +++ b/libswscale/utils.c @@ -49,6 +49,7 @@ #include "libavutil/mathematics.h" #include "libavutil/opt.h" #include "libavutil/pixdesc.h" +#include "libavutil/thread.h" #include "libavutil/aarch64/cpu.h" #include "libavutil/ppc/cpu.h" #include "libavutil/x86/asm.h" @@ -1189,12 +1190,13 @@ av_cold int sws_init_context(SwsContext *c, SwsFilter *srcFilter, int ret = 0; enum AVPixelFormat tmpFmt; static const float float_mult = 1.0f / 255.0f; + static AVOnce rgb2rgb_once = AV_ONCE_INIT; cpu_flags = av_get_cpu_flags(); flags = c->flags; emms_c(); - if (!rgb15to16) - ff_sws_rgb2rgb_init(); + if (ff_thread_once(&rgb2rgb_once, ff_sws_rgb2rgb_init) != 0) + return AVERROR_UNKNOWN; unscaled = (srcW == dstW && srcH == dstH);