On Sat, Oct 20, 2018 at 12:30 PM Marton Balint <c...@passwd.hu> wrote:

>
>
> On Sat, 20 Oct 2018, Hendrik Leppkes wrote:
>
> > On Sat, Oct 20, 2018 at 12:35 PM Marton Balint <c...@passwd.hu> wrote:
> >>
> >> Libx264 uses strtok which is not thread safe. Strtok is used in
> >> x264_param_default_preset in param_apply_tune in x264/common/base.c.
> >> Therefore the flag must be removed.
> >>
> >> Fixes ticket #7446.
> >>
> >> Signed-off-by: Marton Balint <c...@passwd.hu>
> >> ---
> >>  libavcodec/libx264.c | 6 ++----
> >>  1 file changed, 2 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
> >> index 54e6703d73..d6367bf557 100644
> >> --- a/libavcodec/libx264.c
> >> +++ b/libavcodec/libx264.c
> >> @@ -1063,8 +1063,7 @@ AVCodec ff_libx264_encoder = {
> >>      .priv_class       = &x264_class,
> >>      .defaults         = x264_defaults,
> >>      .init_static_data = X264_init_static,
> >> -    .caps_internal    = FF_CODEC_CAP_INIT_THREADSAFE |
> >> -                        FF_CODEC_CAP_INIT_CLEANUP,
> >> +    .caps_internal    = FF_CODEC_CAP_INIT_CLEANUP,
> >>      .wrapper_name     = "libx264",
> >>  };
> >>  #endif
> >> @@ -1115,8 +1114,7 @@ AVCodec ff_libx262_encoder = {
> >>      .priv_class       = &X262_class,
> >>      .defaults         = x264_defaults,
> >>      .pix_fmts         = pix_fmts_8bit,
> >> -    .caps_internal    = FF_CODEC_CAP_INIT_THREADSAFE |
> >> -                        FF_CODEC_CAP_INIT_CLEANUP,
> >> +    .caps_internal    = FF_CODEC_CAP_INIT_CLEANUP,
> >>      .wrapper_name     = "libx264",
> >>  };
> >>  #endif
> >> --
> >> 2.16.4
> >>
> >
> > Was this ever reported to x264?
>
> Not by me.
>
> > Theoretically some other library or another part of a calling
> > application could still use strtok and break it.
>
> I agree, this should be fixed in x264 as well, but until then it
> seems harmless to remove the flag.
>

this could potentially break a couple of use cases where it "happens to
work" (especially those that rely on the aforementioned abaa1226)
rather that dropping the flag entirely, why not fixing x264 first, bump its
version, and apply the flag conditionally on that?
-- 
Vittorio
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to