On 02/08/16 18:39, Nicolas George wrote:
> Le sextidi 16 thermidor, an CCXXIV, Carl Eugen Hoyos a écrit :
>> - { "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" },
Urgh :( I did not consider this problem at all when writing the code.
I tested the patch and it works as intended.
> 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.
I agree that this would be the best answer. However, I'm not sure about the
backward compatibility - with the current behaviour users can have scripts
containing "1,2" in any numeric argument to ffmpeg (meaning 6/5 as here) and
have it parsed correctly because of their locale settings.
Therefore, I'm inclined to go for the proposed workaround (or something
similar*) for now, including backporting it to 3.1 for distributions such as
Debian which are encountering it, and consider the underlying locale problem
separately.
Thanks,
- Mark
* The default values here don't really matter: setting them to 1, 0, 1, 0
(respectively) might be cleaner as a way to duck the problem. (I only defined
them like this so that it does something vaguely reasonable when testing with
ffmpeg with no options, the values themselves don't matter because any real use
will set them manually.)
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
