Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OCIO fails parsing configs with non-C locale #297

Closed
jeremyselan opened this issue Dec 12, 2012 · 8 comments · Fixed by #1496
Closed

OCIO fails parsing configs with non-C locale #297

jeremyselan opened this issue Dec 12, 2012 · 8 comments · Fixed by #1496

Comments

@jeremyselan
Copy link
Contributor

When OCIO is run on a Ubuntu 12.04 machine with a German locale, parsing of srgb.spi1d fails and we get an "Invalid 'From' Tag" exception. The issue is sscanf() there is attempting to parse the string using the German locale.

This should be fixed inside OCIO, the external user should not have to mess with locales before using.

See this thread on ocio-dev for additional information:
http://groups.google.com/forum/?fromgroups=#!topic/ocio-dev/xH47bLSIgwE

@devernay
Copy link
Contributor

Also observed on OS X, with a French locale (fr_FR.UTF-8).
setting LANG=C fixes parsing, but IMHO OCIO should always parse the files using the "C" locale

devernay added a commit to MrKepzie/Natron that referenced this issue Jul 3, 2014
- setlocale(LC_ALL,"C") doesn’t seem to affect LC_NUMERIC if the LC_NUMERIC environment variable is detained (as in Ubuntu 14.04LTS)
- issue AcademySoftwareFoundation/OpenColorIO#297 requires proper numeric locale setting
- fixes #67 (hopefully)
@dictoon
Copy link

dictoon commented Apr 25, 2019

This problem has also been reported several times in appleseed:
appleseedhq/appleseed#2567

@JenusL
Copy link

JenusL commented Jun 25, 2019

I just got bit by this when writing a plugin for 3ds Max that uses OCIO.
I can't believe this has been open since 2012.
Will this ever get fixed?

@lgritz
Copy link
Collaborator

lgritz commented Jun 25, 2019

I went through this a year or two ago with OpenImageIO. I had to ruthlessly find all printf, scanf, and iostream stuff and replace it all with a new set of calls that I guarantee to be locale-independent.
It's not super hard and you are welcome to steal what you need from OIIO (I can point out what is necessary), but it can be an extensive undertaking.

@Dithermaster
Copy link

An alternative to changing the calls is to fix the locale around the call. We fixed this specific OCIO issue on the host side by saving the thread locale, setting it to "C" locale, calling the OCIO function, and then restoring the locale. It's all wrapped in an auto-style C++ class, so it's one line of code in the places we needed it. Yes, I reported it at the time, but we needed a fix ASAP so that's how we did it.

I recall it was a beautiful looking bug -- if you were running French language OS, one of our color managed looks got all posterized. It turned out to be because the LUT read only got integers instead of floating-point numbers (because decimal separator in French is not "."), so the curve because stair-steps.

@KelSolaar
Copy link
Contributor

KelSolaar commented Jun 25, 2019 via email

@JenusL
Copy link

JenusL commented Jun 25, 2019

What bugs me is that I spent a long time trying to figure out what was wrong. But after I found out it was locale, the fix was easy in the host (3ds Max in this case).
Took me about 10 minutes to code it. But in case someone doesn't want to reinvent the wheel, here's what I did.

// OCIO has a , vs . bug depending on system locale
// https://github.com/imageworks/OpenColorIO/issues/297
const char* curlocale = strdup(setlocale(LC_NUMERIC, NULL));  // save current numeric locale
setlocale(LC_NUMERIC, "C");  // set numeric locale to C

// Put your affected OCIO code here
processor = config->getProcessor(transform);

setlocale(LC_NUMERIC, curlocale);  // restore numeric locale

It's a very manual approach but it was quick. So I would love to see how @Dithermaster solved it :)
Keep in mind I only override LC_NUMERIC. Maybe some other languages/locales need to override more.

@Dithermaster
Copy link

Our solution was based on the same APIs, but wrapped in C++ and with Windows support:

class AutoSetAndRestoreThreadLocale
{
public:
    AutoSetAndRestoreThreadLocale();
    ~AutoSetAndRestoreThreadLocale();

private:
#ifndef _WIN32
    locale_t oldLocale;
    locale_t currentLocale;
#else
    SoStringA ssaLocale;
    int previousThreadConfig;
#endif
};

and

AutoSetAndRestoreThreadLocale::AutoSetAndRestoreThreadLocale()
{

#ifndef _WIN32
    // set to C locale, saving the old one (returned from useLocale)
    currentLocale = newlocale(LC_ALL_MASK,"C",NULL);
    oldLocale = uselocale(currentLocale);
#else
    // set locale will only change locale on the current thread
    previousThreadConfig = _configthreadlocale(_ENABLE_PER_THREAD_LOCALE);

    // get and store current locale
    ssaLocale.convert(setlocale(LC_ALL, NULL));

    // set to "C" locale
    setlocale(LC_ALL, "C");
#endif

}

AutoSetAndRestoreThreadLocale::~AutoSetAndRestoreThreadLocale()
{

#ifndef _WIN32
    // restore the previous locale and freeing the created locale
    uselocale(oldLocale);
    freelocale(currentLocale);
#else
    // thread specific
    setlocale(LC_ALL, ssaLocale.c_str());

    // set back to global settings]
    _configthreadlocale(previousThreadConfig);
#endif

}

Then in a block where you're making OCIO calls, have
AutoSetAndRestoreThreadLocale locale;
When it hits that line it saves and sets the locale, then when the object destructs it restores the old locale.

amyspark added a commit to amyspark/OpenColorIO that referenced this issue Sep 20, 2021
This commit brings in the amalgamated header of the fast_float library,
and applies it to our uses of sscanf with %f.

This library is locale-free, and does not incur the performance penalty of
using a thread locale RAII class.

Fixes AcademySoftwareFoundation#297
Fixes AcademySoftwareFoundation#379
Fixes AcademySoftwareFoundation#1022
amyspark added a commit to amyspark/OpenColorIO that referenced this issue Sep 20, 2021
This commit brings in the amalgamated header of the fast_float library,
and applies it to our uses of sscanf with %f.

This library is locale-free, and does not incur the performance penalty of
using a thread locale RAII class.

Fixes AcademySoftwareFoundation#297
Fixes AcademySoftwareFoundation#379
Fixes AcademySoftwareFoundation#1022

Signed-off-by: L. E. Segovia <13498015+amyspark@users.noreply.github.com>
amyspark added a commit to amyspark/OpenColorIO that referenced this issue Sep 20, 2021
This commit brings in the amalgamated header of the fast_float library,
and applies it to our uses of sscanf with %f.

This library is locale-free, and does not incur the performance penalty of
using a thread locale RAII class.

Fixes AcademySoftwareFoundation#297
Fixes AcademySoftwareFoundation#379
Fixes AcademySoftwareFoundation#1022

Signed-off-by: L. E. Segovia <13498015+amyspark@users.noreply.github.com>
amyspark added a commit to amyspark/OpenColorIO that referenced this issue Sep 20, 2021
This commit brings in the amalgamated header of the fast_float library,
and applies it to our uses of sscanf with %f.

This library is locale-free, and does not incur the performance penalty of
using a thread locale RAII class.

Fixes AcademySoftwareFoundation#297
Fixes AcademySoftwareFoundation#379
Fixes AcademySoftwareFoundation#1322

Signed-off-by: L. E. Segovia <13498015+amyspark@users.noreply.github.com>
amyspark added a commit to amyspark/OpenColorIO that referenced this issue Oct 9, 2021
This commit adds Daniel Lemire's fast_float library as an external dependency,
and applies it to our uses of sscanf with %f as well as istringstream.

This library is locale-free, and does not incur the performance penalty of
using a thread locale RAII class or imbuing a locale into a
stringstream.

See:
    Daniel Lemire, Number Parsing at a Gigabyte per Second, Software:
    Pratice and Experience 51 (8), 2021.
    <https://arxiv.org/abs/2101.11408>

Fixes AcademySoftwareFoundation#297
Fixes AcademySoftwareFoundation#379
Fixes AcademySoftwareFoundation#1322

Signed-off-by: L. E. Segovia <13498015+amyspark@users.noreply.github.com>
amyspark added a commit to amyspark/OpenColorIO that referenced this issue Oct 9, 2021
This commit adds Daniel Lemire's fast_float library as an external dependency,
and applies it to our uses of sscanf with %f as well as istringstream.

This library is locale-free, and does not incur the performance penalty of
using a thread locale RAII class or imbuing a locale into a
stringstream.

See:
    Daniel Lemire, Number Parsing at a Gigabyte per Second, Software:
    Pratice and Experience 51 (8), 2021.
    <https://arxiv.org/abs/2101.11408>

Fixes AcademySoftwareFoundation#297
Fixes AcademySoftwareFoundation#379
Fixes AcademySoftwareFoundation#1322

Signed-off-by: L. E. Segovia <13498015+amyspark@users.noreply.github.com>
amyspark added a commit to amyspark/OpenColorIO that referenced this issue Nov 10, 2021
This commit adds Daniel Lemire's fast_float library as an external
dependency, and applies it to our uses of sscanf with %f, strtod,
and istringstream.

Additionally, for parsing integer numbers, we implement a from_chars
shim that forwards the call to strtol_l along with a statically
initialized locale constant.

The usage of fast_float is warded by a new OCIO_USE_FAST_FLOAT
configuration variable; if disabled, an identical approach to integers
is followed.

See:
    Daniel Lemire, Number Parsing at a Gigabyte per Second, Software:
    Pratice and Experience 51 (8), 2021.
    <https://arxiv.org/abs/2101.11408>

Fixes AcademySoftwareFoundation#297
Fixes AcademySoftwareFoundation#379
Fixes AcademySoftwareFoundation#1322

Co-Authored-By: Patrick Hodoul <Patrick.Hodoul@autodesk.com>

Signed-off-by: L. E. Segovia <13498015+amyspark@users.noreply.github.com>
amyspark added a commit to amyspark/OpenColorIO that referenced this issue Nov 10, 2021
This commit adds Daniel Lemire's fast_float library as an external
dependency, and applies it to our uses of sscanf with %f, strtod,
and istringstream.

Additionally, for parsing integer numbers, we implement a from_chars
shim that forwards the call to strtol_l along with a statically
initialized locale constant.

The usage of fast_float is warded by a new OCIO_USE_FAST_FLOAT
configuration variable; if disabled, an identical approach to integers
is followed.

See:
    Daniel Lemire, Number Parsing at a Gigabyte per Second, Software:
    Pratice and Experience 51 (8), 2021.
    <https://arxiv.org/abs/2101.11408>

Fixes AcademySoftwareFoundation#297
Fixes AcademySoftwareFoundation#379
Fixes AcademySoftwareFoundation#1322

Co-Authored-By: Patrick Hodoul <Patrick.Hodoul@autodesk.com>

Signed-off-by: L. E. Segovia <13498015+amyspark@users.noreply.github.com>
amyspark added a commit to amyspark/OpenColorIO that referenced this issue Nov 10, 2021
This commit adds Daniel Lemire's fast_float library as an external
dependency, and applies it to our uses of sscanf with %f, strtod,
and istringstream.

Additionally, for parsing integer numbers, we implement a from_chars
shim that forwards the call to strtol_l along with a statically
initialized locale constant.

The usage of fast_float is warded by a new OCIO_USE_FAST_FLOAT
configuration variable; if disabled, an identical approach to integers
is followed.

See:
    Daniel Lemire, Number Parsing at a Gigabyte per Second, Software:
    Pratice and Experience 51 (8), 2021.
    <https://arxiv.org/abs/2101.11408>

Fixes AcademySoftwareFoundation#297
Fixes AcademySoftwareFoundation#379
Fixes AcademySoftwareFoundation#1322

Co-Authored-By: Patrick Hodoul <Patrick.Hodoul@autodesk.com>

Signed-off-by: L. E. Segovia <13498015+amyspark@users.noreply.github.com>
amyspark added a commit to amyspark/OpenColorIO that referenced this issue Nov 10, 2021
This commit adds Daniel Lemire's fast_float library as an external
dependency, and applies it to our uses of sscanf with %f, strtod,
and istringstream.

Additionally, for parsing integer numbers, we implement a from_chars
shim that forwards the call to strtol_l along with a statically
initialized locale constant.

The usage of fast_float is warded by a new OCIO_USE_FAST_FLOAT
configuration variable; if disabled, an identical approach to integers
is followed.

See:
    Daniel Lemire, Number Parsing at a Gigabyte per Second, Software:
    Pratice and Experience 51 (8), 2021.
    <https://arxiv.org/abs/2101.11408>

Fixes AcademySoftwareFoundation#297
Fixes AcademySoftwareFoundation#379
Fixes AcademySoftwareFoundation#1322

Co-Authored-By: Patrick Hodoul <Patrick.Hodoul@autodesk.com>

Signed-off-by: L. E. Segovia <13498015+amyspark@users.noreply.github.com>
amyspark added a commit to amyspark/OpenColorIO that referenced this issue Nov 10, 2021
This commit adds Daniel Lemire's fast_float library as an external dependency,
and applies it to our uses of sscanf with %f as well as istringstream.

This library is locale-free, and does not incur the performance penalty of
using a thread locale RAII class or imbuing a locale into a
stringstream.

See:
    Daniel Lemire, Number Parsing at a Gigabyte per Second, Software:
    Pratice and Experience 51 (8), 2021.
    <https://arxiv.org/abs/2101.11408>

Fixes AcademySoftwareFoundation#297
Fixes AcademySoftwareFoundation#379
Fixes AcademySoftwareFoundation#1322

Signed-off-by: L. E. Segovia <13498015+amyspark@users.noreply.github.com>
amyspark added a commit to amyspark/OpenColorIO that referenced this issue Nov 11, 2021
This commit adds Daniel Lemire's fast_float library as an external
dependency, and applies it to our uses of sscanf with %f, strtod,
and istringstream.

Additionally, for parsing integer numbers, we implement a from_chars
shim that forwards the call to strtol_l along with a statically
initialized locale constant. Unfortunately, to preserve consistency with
from_chars the character range must be copied to a temporary
std::string. The test suite has been adapted to account for this new
behaviour.

The usage of fast_float is warded by a new OCIO_USE_FAST_FLOAT
configuration variable; if disabled, an identical approach to integers
is followed.

See:
    Daniel Lemire, Number Parsing at a Gigabyte per Second, Software:
    Pratice and Experience 51 (8), 2021.
    <https://arxiv.org/abs/2101.11408>

Fixes AcademySoftwareFoundation#297
Fixes AcademySoftwareFoundation#379
Fixes AcademySoftwareFoundation#1322

Co-Authored-By: Patrick Hodoul <Patrick.Hodoul@autodesk.com>

Signed-off-by: L. E. Segovia <13498015+amyspark@users.noreply.github.com>
amyspark added a commit to amyspark/OpenColorIO that referenced this issue Nov 24, 2021
This commit adds Daniel Lemire's fast_float library as an external
dependency, and applies it to our uses of sscanf with %f, strtod,
and istringstream.

Additionally, for parsing integer numbers, we implement a from_chars
shim that forwards the call to strtol_l along with a statically
initialized locale constant. Unfortunately, to preserve consistency with
from_chars the character range must be copied to a temporary
std::string. The test suite has been adapted to account for this new
behaviour.

The usage of fast_float is warded by a new OCIO_USE_FAST_FLOAT
configuration variable; if disabled, an identical approach to integers
is followed.

See:
    Daniel Lemire, Number Parsing at a Gigabyte per Second, Software:
    Pratice and Experience 51 (8), 2021.
    <https://arxiv.org/abs/2101.11408>

Fixes AcademySoftwareFoundation#297
Fixes AcademySoftwareFoundation#379
Fixes AcademySoftwareFoundation#1322

Co-Authored-By: Patrick Hodoul <Patrick.Hodoul@autodesk.com>

Signed-off-by: L. E. Segovia <13498015+amyspark@users.noreply.github.com>
amyspark added a commit to amyspark/OpenColorIO that referenced this issue Dec 7, 2021
This commit adds support for parsing numbers without being influenced by
the current system locale. We implement a from_chars
shim that forwards the call to strto*_l along with a statically
initialized locale constant.

Fixes AcademySoftwareFoundation#297
Fixes AcademySoftwareFoundation#379
Fixes AcademySoftwareFoundation#1322

Co-Authored-By: Patrick Hodoul <Patrick.Hodoul@autodesk.com>

Signed-off-by: L. E. Segovia <13498015+amyspark@users.noreply.github.com>
amyspark added a commit to amyspark/OpenColorIO that referenced this issue Dec 8, 2021
This commit adds support for parsing numbers without being influenced by
the current system locale. We implement a from_chars
shim that forwards the call to strto*_l along with a statically
initialized locale constant.

Fixes AcademySoftwareFoundation#297
Fixes AcademySoftwareFoundation#379
Fixes AcademySoftwareFoundation#1322

Co-Authored-By: Patrick Hodoul <Patrick.Hodoul@autodesk.com>

Signed-off-by: L. E. Segovia <13498015+amyspark@users.noreply.github.com>
hodoulp pushed a commit that referenced this issue Dec 8, 2021
This commit adds support for parsing numbers without being influenced by
the current system locale. We implement a from_chars
shim that forwards the call to strto*_l along with a statically
initialized locale constant.

Fixes #297
Fixes #379
Fixes #1322

Co-Authored-By: Patrick Hodoul <Patrick.Hodoul@autodesk.com>

Signed-off-by: L. E. Segovia <13498015+amyspark@users.noreply.github.com>
hodoulp pushed a commit that referenced this issue Dec 8, 2021
This commit adds support for parsing numbers without being influenced by
the current system locale. We implement a from_chars
shim that forwards the call to strto*_l along with a statically
initialized locale constant.

Fixes #297
Fixes #379
Fixes #1322

Co-Authored-By: Patrick Hodoul <Patrick.Hodoul@autodesk.com>

Signed-off-by: L. E. Segovia <13498015+amyspark@users.noreply.github.com>
hodoulp pushed a commit that referenced this issue Dec 9, 2021
This commit adds support for parsing numbers without being influenced by
the current system locale. We implement a from_chars
shim that forwards the call to strto*_l along with a statically
initialized locale constant.

Fixes #297
Fixes #379
Fixes #1322

Co-Authored-By: Patrick Hodoul <Patrick.Hodoul@autodesk.com>

Signed-off-by: amyspark <13498015+amyspark@users.noreply.github.com>
hodoulp added a commit that referenced this issue Dec 9, 2021
This commit adds support for parsing numbers without being influenced by
the current system locale. We implement a from_chars
shim that forwards the call to strto*_l along with a statically
initialized locale constant.

Fixes #297
Fixes #379
Fixes #1322

Co-Authored-By: Patrick Hodoul <Patrick.Hodoul@autodesk.com>

Signed-off-by: amyspark <13498015+amyspark@users.noreply.github.com>

Co-authored-by: amyspark <13498015+amyspark@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants