AviSynth / AviSynthPlus

AviSynth with improvements
http://avs-plus.net
975 stars 75 forks source link

string(Float, Format) returns localized floating point delimter #336

Closed marukuru closed 1 year ago

marukuru commented 1 year ago

Tested with latest master by using Assert(false, string(256., "%#0.0f")): string(256., "%#0.0f") returns 256, on my system, using the default localized floating point delimiter of my system. This results in problems with scripts like ExTools, which expect . as a delimiter. Locales should be ignored when using floating point formatting.

Ubuntu 22.04, ffmpeg5.

pinterf commented 1 year ago

Hi! Thanks for not forgetting your OS in the report.

Yes, As seen here:

https://github.com/AviSynth/AviSynthPlus/blob/master/avs_core/core/avisynth.cpp#L361

local invariant numeric formatting works only on Windows MSVC at the moment, Probably _vscprintf_l is MS-specific stuff and could not find a workaround for that under Linux? I'm gonna check it again.

marukuru commented 1 year ago

Hi @pinterf, thanks for your swift reply. Pointing at the specific line of code (looking through the code quickly I didn't manage to locate the function) I at least found a quick workaround for now: LC_NUMERIC="en_US.UTF-8" ffplay -i test.avs

pinterf commented 1 year ago

Yeah, good news until I have a final working solution.

pinterf commented 1 year ago

This is what I found:

There is no locale specific version of vsnprintf under Linux.

One must set LC_NUMERIC part of the locale to "C".

During program startup, the equivalent of setlocale(LC_ALL, "C"); is executed before any user code is run. But Avisynth is just a dll/shared object.

The host program can optionally call setlocale(LC_ALL, ""); - typically during startup in its main(). This sets the current locale by the existing environment variables. When you have set your LC_NUMERIC, you have probably achieved this very feature.

Aside from the custom locale-specific formatting is non-existent - as far as I know - "unfortunately setlocale is not thread safe and modifies global state which affects execution of locale-dependent functions. It is undefined behavior to call it from one thread, while another thread is executing any of many functions."

I wonder if ffmpeg code contains setlocale(LC_ALL, "");

EDIT: could not find any occurance there. Then a plugin is probably resetting it, calling setlocale(LC_ALL, "") or setlocale(LC_NUMERIC, "")

EDIT2: Yesss, KNLMeansCL is tampering with it. First it sets to the plugin's need: "C"

https://github.com/pinterf/KNLMeansCL/blob/master/KNLMeansCL/NLMAvisynth.cpp#L468

then it restores by the actual environment.

https://github.com/pinterf/KNLMeansCL/blob/master/KNLMeansCL/NLMAvisynth.cpp#L483

marukuru commented 1 year ago

KNLMeansCL/NLMAvisynth.cpp: setlocale(LC_ALL, ""); :-) Actually using KNLMeansCL in my setup.

pinterf commented 1 year ago

Ahh, you've just found the same.

marukuru commented 1 year ago

Recompiled KNLMeansCL without the setlocale and everything works as expected. :-)

marukuru commented 1 year ago

Closing this here, because it's not actually an issue with AVS+, imho.