[FFmpeg-devel] [Libav-user] [PATCH]Fix a crash in vaapi_encode if "." is not the decimal separator

[Nicolas George]

Le sextidi 16 thermidor, an CCXXIV, Carl Eugen Hoyos a écrit :
> To: "This list is about using libavcodec, libavformat, libavutil,
> libavdevice and libavfilter." <libav-user at ffmpeg.org>

Why did you post to this list?

> Subject: [PATCH] lavc/vaapi_encode_h26x: Fix a crash if "." is not the
>  decimal separator.

> -    { "i_qfactor",      "1.0" },
> -    { "i_qoffset",      "0.0" },
> -    { "b_qfactor",      "1.2" },
> -    { "b_qoffset",      "0.0" },
> +    { "i_qfactor",      "1"   },
> +    { "i_qoffset",      "0"   },
> +    { "b_qfactor",      "6/5" },
> +    { "b_qoffset",      "0"   },

I think this is not correct at all.

First, the code should not crash with incorrect parameters, it should either
return a proper error code or accept the values with a sane meaning.

Second, this is not fixing the issue, it is just working around it. These
options can also be set by applications or users, and the point is supposed
to work.

The correct fix for the parsing side of the issue would be either to
document that the FFmpeg libraries cannot be used with LC_NUMERIC set to
anything else than C/POSIX (and possibly add a check at init time) or change
the code to use an unlocalized version of strtod().

I am rather in favour of the first solution, as anybody who
setlocale(LC_NUMERIC) in their are idiots and will break much more than
FFmpeg libraries.

Of course, this is on top of fixing the encoders to not crash with the
offending values.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160802/f40f88cb/attachment.sig>