frontend: Use system locale with UTF-8 instead of 'C'

[Dankirk]

Description

The changes here are based on PR #13097 that was split from this issue and does some preliminary work to support changes introduced here.

• Sets C runtime locale to system locale with UTF-8 codepage on Windows.
  This has always been default behavior on unix, but Windows defaults to minimal 'C' locale.

  • LC_NUMERIC is still set to "C". Now on all platforms instead of just unix.
    This is so decimal point is a dot (not a comma) for string <-> float conversions.

• The configured CRT locale is copied to be the default std::locale for C++.
  All platforms have been using minimal "C" until now. This change affects new facet and ios_base instances without a specified locale or imbue() call.

• OBS Studio language setting no longer changes QLocale's default locale and instead always uses system locale.
  This gives conformity with non Qt functions, but most importantly is likely what user wants as well. Ie. sorting and formatting functions should follow OS locale rules instead of OBS Studio translations language. (Reverts c4840dd)

• obs_get_locale() still returns OBS language locale, which is used for Python and LUA apis, GDI+ text widget transformations, and HTTP accepted languages header.

Motivation and Context

Locale-aware operations like sorting and time formatting in C are not available on Windows, but are on unix, as pointed out in PR #12577.
Fixes #11133, fixes #12953

The C++ locale and QLocale changes make the locale-aware functions of all layers work in similiar fashion.

For example: On unix currently the used locales are: OS locale for CRT, minimal "C" for C++ and OBS language for QLocale.
A weekday name can be in three different languages depending if you used strftime(), std::time_get facet or QLocale.
This makes string transformations between C, C++ and Qt very tricky.

How Has This Been Tested?

An important point is that the CRT locale settings introduced here have always been this way for unix, which suggests there aren't any insurmountable problems with the new locales. Windows specific functions should be tested for CRT locale. Changes for C++ and QLocale defaults affect all platforms.

Searched the codebase for affected areas and addressed as necessary:

• CRT: ctype.h character classification function parameters and expected return values
• CRT: strftime() formatting with % placeholders
• CRT: scanf() and printf() formatting with % placeholders
• CRT: FILE operations
• C++: fstream operations
• C++: facet locale usage
• QLocale: Expected return values of formatting functions
• QLocale: QString locale-aware methods

Some general testing with Japanese characters

• Edited recording path with %A (weekday) variable and some Japanese characters. Recorded a video. Weekday name was localized and recording worked fine.
• Remuxed said file. Worked fine.
• Renamed some sources with Japanese characters and exported the scene collection, removed it from OBS and re-imported it. No problems.
• Wrote names of those sources to logFile with blog()

I'm on Windows 11 English US version, but with Finnish locale settings (fi_FI). OBS language is English.

Types of changes

• New feature (non-breaking change which adds functionality)

• Breaking change (fix or feature that would cause existing functionality to change)
  • 3rd party Python, LUA and rtmp that have been using _mbs_ conversion functions directly or via file io operations have to input text in utf-8 and expect utf-8 output (except for wchar/_wcs_ which is OS defined).

Checklist:

• My code has been run through clang-format.
• I have read the contributing document.
• My code is not on the master branch.
• The code has been tested.
• All commit messages are properly formatted and commits squashed where appropriate.
• I have included updates to all appropriate documentation.

[Dankirk]

Scouted the web and the codebase for potential issues. Here's some observations...
EDIT: These have been accounted for in the PR description.

General stuff about setlocale() on Windows
For list of things C runtime locale affects https://cppreference.com/w/c/locale/setlocale.html

• Important distinction is some functions only care about codepage/encoding of the locale, not the language_region rules specifically.
• We can ignore all number related things, because we use the minimal 'C' locale for LC_NUMERIC.
• Affected string.h and time.h functions are all functions that are specifically for locale-aware things, Things like weekday names will now be localized using strftime() and strcoll() will do a locale-aware comparison.
• ctype.h character classification ranges are extended. ie. isalnum() may return true for more characters, so there is reason to check if any part using these functions is okay with that. Couldn't hurt to cast the parameters to unsigned char either, since many functions expect value to be 0-255, which char using utf-8 casted to int might not be (char range is -128 to 127). Then again, the functions in use have worked fine on unix until now...
• stdio.h Formatting of the % placeholders in scanf() and printf() and the sort is affected. Decimals will still be dots (controlled by LC_NUMERIC), but %s will match more. More about file operations below.

multibyte <-> utf8 <-> wchar

In platform.h there are various string conversion functions. From these only the multibyte functions with _mbs_ are affected by this change. The rest use Windows API, which doesn't follow C runtime locale modified by setlocale(). All of these don't care about the language_region, only about the codepage/encoding, which should be UTF-8, not Windows default ANSI 1252 for example.

The _mbs_ functions are currently not used in OBS Studio itself, but are offered for external usage for Python, LUA and rtmp. This means there's no change for OBS Studio itself, but external things might see different results from these conversion functions on Windows, which will now be more alike to return values on unix. On Windows mb* functions are affected by _setmbcp() while setlocale() will suffice on unix.

The utf8 <-> wchar functions (ie os_utf8_to_wcs()) use MultiByteToWideChar() and WideCharToMultiByte() functions with utf-8 codepage, which will work after this update. Unlike mbstowcs() and the sort in _mbs_ implementations, these functions are independent from CRT locale, but do follow the manifest declaration (though only for CP_ACP, which we don't use). On unix these functions use a custom implementation for conversion, which naturally assumes the text is/is-to-be utf-8 encoded.

Streams and file operations

C++ streams, like fstream are controlled by std::locale::global() or facets, which is separate setting from CRT setlocale(). Thus, C++ stream operations have been using the minimal "C" locale by default (both unix and Windows). This change copies the CRT locale as default for C++ too. Any fstreams initialized before std::locale::global() call should call imbue() to match the new locale. Any streams initialized after inherit the global locale.

C-style FILE wide char streams pick the locale available when first io operation is used and continue using that. So it is important setlocale() is called before these streams are used or they are re-opened with freopen().

printf() and scanf() -type functions use locale for % placeholders, as explained above.

When to setlocale() ?

On principle locale should be one of the first things to set, since many things inherit it and it's cumbersome to retroactively reset it's state to existing things. However, since Qt overwrites locale during construction of QApplication (OBSApp) for unix we could use OBSApp constructor, as we have been to reset LC_NUMERIC back to "C". If it is decided that OBS translations locale should be followed instead of OS's, initLocale() also seems acceptable.

[PatTheMav]

• OBS Studio language setting no longer changes QLocale's default locale and instead always uses system locale.
  This gives conformity with non Qt functions, but most importantly is likely what user wants as well. Ie. sorting and formatting functions should follow OS locale rules instead of OBS Studio translations language. (Reverts c4840dd)

Highlighting this because this is a severe change, even though I think it's correct in principle. Changing an application's display language should not change the regional settings (which encompass sorting rules as well as decimal point character, etc.), and at least that's how it works on macOS.

@Warchamp7 @Fenrirthviti would be good to hear if you'd be fine with this change conceptually as well.

[Fenrirthviti]

My main concern here, as someone who only uses the English/USA locale/region, is that I'm unsure what the expectation for a Windows application is. The current motivation seems to be "Unix does it this way" and that to me, is not sufficient. Do we have examples and recommendations from Microsoft, or other prominent Windows applications on how they handle this kind of setting for apps that use translations?

[Dankirk]

Microsoft general guidelines for globalization suggests:

Don't use language to assume a user's region; and don't use region to assume a user's language.

My own take is that OS regional settings + app translations is the desired output with no additional settings in UI, good likelyhood being fine by default, but allows configuration when needed. The obvious drawback is that to change the regional settings one needs to change OS settings, which could be an issue when using a shared device, but OS should provide options for it.

Many Microsoft apps understandably just follow the OS region settings. That includes the file explorer. Apps like Office do offer a separate in app setting for regional settings as well, but that's because they specialize in that sort of thing. For apps in general many have translation + maybe time formatting options and don't follow a specific region per se. The sorting order is a gamble. Steam seems to sort games and friends using the app display language. Spotify sorts playlists by Windows display language (not region settings, nor Spotify display language, I don't recommend this). Whatsapp uses OS regional settings.

In any case, any locale is still better than the current situation with non-changeable "C" locale.

[Fenrirthviti]

Thanks for the additional context here. My, admittedly mostly uninformed opinion based on the discussion here, is that this seems fine. Without lack of a clear "best practice" on Windows, moving things in-line with our cross-platform implementation seems like the best option.

@PatTheMav or @jcm93 Does macOS follow a similar approach to what is being proposed here?

[PatTheMav]

Thanks for the additional context here. My, admittedly mostly uninformed opinion based on the discussion here, is that this seems fine. Without lack of a clear "best practice" on Windows, moving things in-line with our cross-platform implementation seems like the best option.

@PatTheMav or @jcm93 Does macOS follow a similar approach to what is being proposed here?

On macOS language and regional settings are also separate things and it's expected that your app's language is actually changed from the OS' language settings (rather than within the application itself) which in gendered languages also includes choice of preferred pronoun:

The language setting indeed only changes the display language, decimal format, sorting, et. al. still follow the regional setting (at least in "native" apps).

[Dankirk]

In the current state the PR works as designed, but I'm thinking of adding the following lines to obs.manifest to set active code page to utf-8 for Win32 APIs (the A versions of functions). While OBS uses those relatively little, with hard coded strings mostly, it could ease 3rd party code adaptation, plugins etc.

It also allows utf-8 encoding of commandline arguments for loading a specific collection, scene or profile by their name.
For example ./obs64.exe --profile キアラ only works with the utf-8 manifest on Windows. Without it arguments are in one of the ANSI codepages instead and we don't have existing ways to deal with that in OBS codebase (they all assume utf-8 or utf-16 wchar).

Lines to add to obs.manifest

<application>
    <windowsSettings>
        <activeCodePage xmlns="http://schemas.microsoft.com/SMI/2019/WindowsSettings">UTF-8</activeCodePage>
    </windowsSettings>
</application>

Doing this would mean the locale setup routine would need to be done much earlier in the program than the OBSApp constructor, preferably at the start of main(). However, since Qt overwrites locales in the base constructor on unix, unix platforms would need to re-do the locale setup routine after, which is a bit of an ugly design.

[Dankirk]

The obs.manifest changes has been added and locale setup routine moved to beginning of the app, with unix re-running it after OBSApp has been created.

Here's some info about the manifest if you wish to read. https://learn.microsoft.com/en-us/windows/apps/design/globalizing/use-utf8-code-page

If there's any issues with locale setup routine being the first thing to do please let me know.
The reasoning for it is that many things inherit the locale when they are initialized and retroactively updating that is cumbersome. This includes things like outputting to anything to stdout or using fstream. On my observation load_debug_privilege() is the first function to use blog(), so the locale setup routine should happen before that.

[PatTheMav]

The main "problem" this PR has to address is that OBS on the whole is conceptually not built to be aware (much less so capable of handling) of locales (and locale differences), similarly to how it pretends that any set of bytes in memory is "UTF-8" but then happily treats it as "just ASCII".

For better or worse all that OBS can handle without issue is US-American text and formats, anything beyond that is a "happy accident".

Introducing these capabilities has to go far beyond just slapping on some setlocale() calls or adding facets where necessary, because of the following aspects:

• C only supports global locales that indeed have to be set almost immediately at an app's run time to ensure all system APIs follow suit
• The global locale is set per process on POSIX, but per thread on Windows, which makes locale-awareness trickier to ensure for all the threads libobs uses in the latter case (and that's not even getting into the weeds of whether certain threads need to be locale aware and if they suddenly become so - because of the global switch on POSIX - we introduce hard to debug/notice side-effects).
  • And many (most?) Windows C APIs do not even respect setlocale?
• C++ supports setting up custom locale objects and allows to apply them where needed, thus allowing the app to be much more "locale-aware" (e.g. one can use different locales for "reading" and "writing" of data, at least for locale-aware library functions).
• As if two (or three in the case of Windows) different layers of locale handling are not enough we also use Qt on top of it all, which seems to follow a similar approach like C++ (with QLocale instances)
• Note that C++'s and Qt's approach is considered "correct" by requiring the app to explicitly use locale awareness where needed instead of flipping a "magic switch" that changes everything behind the scenes. Because it makes sense for an app to use an internal "canonical" format, but use local-awareness when interfacing with the user and the user's data.

That's already a big pile of work to get through and figure out where/how/if to change OBS (and libobs, and the 1st party modules) to become locale-aware, but on top of that we have the (largely non-existent) Unicode-handling:

• Qt's QStrings are all more or less expected to be encoded in UTF-16 (I haven't checked if it's actually UTF-16 or just UCS-2 with UTF-16 surrogate pairs)
• std::string instances and char * data is ostensibly thought of as "UTF-8" but indeed is treated as plain ASCII or simply a sequence of bytes in memory.
• As long as such a "string" is not truncated, manipulated, or collated (and the unchanged array of bytes copied/transferred as-is) the underlying strings will be retained
  • Because those strings are passed as-is to Qt and Qt is indeed UTF-8 aware, the UTF-8 code points will be correctly decoded and re-encoded as UTF-16 when stored in a QString
• On Windows any wchar_t-based string is considered to be UTF-16 as well (in the case of file system paths that might actually still be UCS-2), which is then passed to Windows APIs to convert to/from UTF-8 (similar to Qt)
• Almost all internal APIs (or functions) that handle char * data or std::string do not support UTF-8: All bytes are considered "ASCII characters", there is no capability to detect the variable-length encoding of UTF-8 code points, much less so any capability to detect grapheme clusters, or any awareness of language-specific differences in meaning of what "upper case" means.
  • Again, as long as no manipulation or interpretation of the strings take place this is fine, because UTF-8 is then just a "transport" format.
• As mentioned in the description, on Windows there is the additional complexity of many APIs being available as an "ANSI" variant (which uses the current code page for encoding/decoding) and the "UNICODE" variants (which might either be actual variable-length UTF-16 or fixed-length UCS-2).
  • And as if all of that weren't bad enough, Microsoft added the mentioned support for a "UTF-8 code page" which allows one to use UTF-8 encoded text with the ANSI variants of APIs, which in theory requires developers to un-adopt the use of their UNICODE APIs.

---

Mind you, I'm not saying that we shouldn't adopt changes like these, but that these changes require a great deal of thought and need to address many architectural design flaws in OBS as it is right now. Simply adding it in bits and pieces runs the risk of fixing symptoms but not fixing the core issue(s), particularly as the entire app has not been designed to properly handle locale's or anything beyond ASCII text and making it handle text "right" in one area might collapse a whole house of cards of assumptions about character data in another.

For that (and a few other reasons) it might take same time (and might even require splitting the whole endeavour up into separate "units") to get it over the finish line.

[Dankirk]

The input is much appreciated. I aknowledge the uncertainty and I'm fine with whatever approach is taken, but I have some counterpoints too.

For better or worse all that OBS can handle without issue is US-American text and formats, anything beyond that is a "happy accident".

I don't believe this. The fact that locales, the region and utf-8 enforcement, have been in use on non-Windows OSs for all of OBS existence while it has been using Qt is a point for that. Some degree of good design choices is needed for that.

The global locale is set per process on POSIX, but per thread on Windows.

This is not true. The threads on Windows have the locale, as long as it is set before spawning threads. References below.
https://learn.microsoft.com/en-us/cpp/parallel/multithreading-and-locales?view=msvc-170

And many (most?) Windows C APIs do not even respect setlocale?

Some only care about the encoding, others about the regional things. Others follow other ways of setting locale, like the manifest. Some really don't care, but considering their limited usability, it might not matter to us either. If it does it's limited job fine it's ok, if not then another approach is in order.

Regarding OBS, Probably biggest culprit we have is the dstr library which comes with the idea that char by char comparison works, but this is not too prevalent in the codebase. dstr' and low level string manipulation functions using ctype.h stdio.h et al libraries is from limited sources and many of them do expect to only have ASCII from them, which does work as intended even with the changes, because of the library limitations. The only fix I have been applying for this is the unsigned char cast, which ensures they don't crash for non-ASCII characters (for being negative when casted from signed char to int by the libraries). Could probably do better, but didn't really see cases where it would have made a difference.

I also expected this to be a hurdle. So I did search the codebase for all prominent standard C and C++ functions that care about the locale and addressed them in this PR if there was a need. That includes tracking down their input origins and where they go to see if UTF-8 and localization is acceptable. I have looked if they care about the region or the encoding, and by set by what (setlocale, active codepage manifest, std::locale::global, or something else). These are not a random list of magic switches in the pr, it's all very much mappable, and have reason to be here.

My experience about utf-8 readiness is that it is almost there. Kind of like you said, many things pretend it already is utf-8 while we have been playing around with ASCII. Other things don't really need to become locale or encoding aware, just as long as their limitation is recognized and used in contexts where you don't expect anything special. I'm weirdly enough expecting this to fix more things by collateral than break, especially those cases where the encoding is coincidentally in ANSI because there was nothing to tell the source we'd like utf-8.

The unicode handling in OBS is otherwise... ok. We have a few ways and can keep using those, nothing wrong with that, they are built correctly for the purpose and work fine after too. This doesn't touch Qt, -W Apis (or defaults selected by UNICODE build flag), utf-16 or ucs2. It is not as if we need to adapt or handle more things, quite the opposite. It's one of the things that coercing utf-8-ness is about. The ANSI pages being one of those things exactly to prevent. The -A apis currently return ANSI coded strings, which simply do not function in OBS if they happen to contain anything non-ASCII.

std::string and char are byte containers with programmer hints about using it for text. They support utf-8 as well as any other encoding with multibyte bytes. Things like strlen will tell the size in bytes, which is usually good for example allocating arrays and when you iterate over them you are often times looking for something that is in ASCII anyway (that is compatible with utf-8 codepoints).

Some after thoughts

The need for this started from trying to get locale-awarness to C, for sorting specifically. Locale availability on C++ or Qt could probably be leveraged with some externs to C, true. Or a signaling mechanism in case module encapsulation paradigms would fight against direct usage, though it is a round about way for something relatively low level.

For commandline arguments I don't see a nice way to fix them without the also -A API changing manifest. I suppose we could use MultibyteToWideChar(), but with CP_ACP (Current code page, OS default if not changed) instead of CP_UTF8 flag and then follow with WideCharToMultibyte with the UTF-8 flag. Should work, but I don't think that type thing should become any sort of standard.

I might look into this a bit more from broader, design point of view too later. Any decision is fine though.

[Dankirk]

Continuing a bit here.

Mind you, I'm not saying that we shouldn't adopt changes like these, but that these changes require a great deal of thought and need to address many architectural design flaws in OBS as it is right now. Simply adding it in bits and pieces runs the risk of fixing symptoms but not fixing the core issue(s), particularly as the entire app has not been designed to properly handle locale's or anything beyond ASCII text and making it handle text "right" in one area might collapse a whole house of cards of assumptions about character data in another.

I do not share the sentiment about the the kind of architectural flaws present here. Reasoning being that this is already being done on non-Windows systems and the research done as described to introduce this PR. The parts of code that supposedly have a problem with non-ASCII data have that now and will, without this update, still be fed non-ASCII data if they are used as such in what ever encoding. In short, parts that don't work after this, never have, and this will not make fixing them more difficult. We treat all non utf-16/32 text as if it were utf-8 encoded already and use proper conversion functions to convert between that and target. Any other encoding text might be in is not supported by our conversion functions, so we should do everything we can to coerce external strings, and internal functions to work with the assumption the text is indeed what we believe, utf-8. While we can and have played around with ASCII, the actual data has always been what it is, it has never actually been limited to codepoints representable in ASCII.

[Dankirk]

Some more details about the standard string apis, that don't fully work with utf-8, but why we shouldn't care.

The standard string/ctype/std:.string functions take locale (LC_CTYPE) and try to accommodate it, but are limited to single-byte characters, which utf-8 is not. This means function like strcmp() can only recognize localized equivalency in ASCII range. Outside of that it does byte-order comparison. This means that it is fine for (in)equivalency checking of utf-8 (with ==0 or !=0), because the bytes match. What doesn't work is sorting and case-insensitivity outside ASCII range. It is fine when you know the data you are working with is in ASCII, locale and encoding be whatever. Otherwise, you will need the wide variants like wcscmp().

This is the the state of things now and after. Nothing will break in this regard because we changed locale or encoding. They have always done this and the results have been the same.

Now for future, if one was to improve this, they should search the codebase for ctype, std::string, and the dstr et all functions and see if they are used with unknown data to do more than equivalency checking, where locale matters. In such case, they should be done with a wide comparison method instead. I'm not suggesting we convert everything to wide, that's expensive, but a template to perform standard wide string operations on narrow string should be there. Perhaps a new job for dstr ? Should be something along lines widen_cmp(char*, char*, c_wide_comparison_func_cb) to make usage painless as possible.
Then have a look at dstr library as a whole. It is currently a byte based string manipulation library that has comparison custom tailored. The error it does, is trying to do that while trying to accommodate case folding which is localization specific. Both cannot be done, because it's not always singlebyte even with wide on Windows, so it should be set to use the standard functions, and then aknowledge and document the remaining flaws about it still being single-byte for the -a variants. It's overkill to try to make those wide by default for correct localization, when it doesn't matter in practice and the usecases, so they should still remain like that. But it should offer a function like the widen_cmp described above.

I'm only singling out only these, because frankly anything higher, QString, utf8<->wide is already done correctly by my observation and that is reflected in the code commits in this PR.

[PatTheMav]

Thank you for your comments, but the point I tried to make (a goal I possibly failed to achieve) is that as maintainers I'd prefer us to take multiple steps back and understand a few things first before diving head-first into changes:

• Is Qt's default locale handling broken? Wouldn't that suggest that Qt has a major bug that we should report? Or is Qt's handling indeed correct and we just use it "wrong"?
• Isn't the actual issue with command line arguments on Windows that obs_set_cmdline_args is used to just naively iterate over argv instead of being aware that command line arguments use a code page on Windows and thus GetCommandLineW should be used to operate on UTF-16 strings instead? (And indeed on my system the code page used by PowerShell is 850 and not 65001).

The changes to write_header and CurrentDateTimeString are correct insofar as both need to produce time strings used for external processing and need to produce uniform output regardless of locale (and indeed both could just use %F, %T instead). But the larger issue here to me is that the function is primarily used by OBSApp and OBSBasic which are Qt-based classes and instead should use QDateTime to create their time strings and not reach into a global free function that uses a "raw" C API to achieve the same.

And that immediately leads to the issue that OBS should just use ISO formats here, which would allow the use of Qt::ISODate with QDateTime to create the time string and no reinvention of the wheel with home-grown formats and date formatting functions would be necessary.

---

The changes to ParseThemeVariables seem correct overall for historical, very C-specific, reasons: Historically implementations of isdigit might have been macro-based and used the character value as an index into a lookup table, so passing in negative values would yield undefined behaviour (e.g. macOS checks for the value being in the range [0,255] and immediately returns false otherwise, but only as long as USE_ASCII is not defined).

Because we cannot know the robustness of any given implementation, casting to an unsigned type is indeed the safer approach (because "is value between 48 and 57" is not how the check seems to be commonly implemented).

[Dankirk]

I appreciate the conversation here, and I too may misjudge what was intended, but let's get cracking.

The Qt locale setup seems to default to "", which sets the default system locale and also default encoding. Unix is mainly utf-8 default so this works for most setups I believe. I wanted to be sure though, and enforce UTF-8. The code itself has a bunch of lines like // FIXME: Shouldn't we still setlocale("UTF-8")? Which also contributed to the fact that I probably should. Otherwise the new locale routine follows Qt's idea of what localization should be, like falling back to C.UTF-8, so there's no harm done. Also on principle Qt naturally only does what is required by Qt anyway, which may not be what we wanted for C locale (as long as we are compatible). On the real though I admit it would be pretty hard to find a failing setup for the Qt's version, so for that it's also acceptable if we revert to just setting LC_NUMERIC to C for unix in the constructor. (We still need the std::locale:.global though)

For commandline, yes GetCommandLineW would work and also be better than the first workaround that came to my mind about acp->wide->utf8 conversion. I still think the manifest approach is somewhat preferable as an environment configuration rather than introducing more platform specific code. I also see changing of the -A apis by the active code page as thing that would help adoption of 3rd party code. If they used -A apis they either didn't know better or maybe they maintained some existing code later called MultibyteToWideChar() with CP_ACP flag. Both of these cases is fixed/compatible by changing the ACP to UTF-8. Not saying we should convert from -W apis. We can keep that.

About MultibyteToWideChar() in OBS, we always use it with CP_UTF8 flag, which implies all non-wide chars should be utf-8. If it's not and it is outside ASCII range, it will be corrupted by the call.

For time things, yes we should use a standardized format and use the tools that most approriate in context (Qt). It is somewhat out of the scope of this, so I settled with preserving current results/expectations as much as possible. This can be changed to do ISO formatting though.

[PatTheMav]

The Qt locale setup seems to default to "", which sets the default system locale and also default encoding. Unix is mainly utf-8 default so this works for most setups I believe. I wanted to be sure though, and enforce UTF-8. The code itself has a bunch of lines like // FIXME: Shouldn't we still setlocale("UTF-8")? Which also contributed to the fact that I probably should. Otherwise the new locale routine follows Qt's idea of what localization should be, like falling back to C.UTF-8, so there's no harm done. Also on principle Qt naturally only does what is required by Qt anyway, which may not be what we wanted for C locale (as long as we are compatible). On the real though I admit it would be pretty hard to find a failing setup for the Qt's version, so for that it's also acceptable if we revert to just setting LC_NUMERIC to C for unix in the constructor. (We still need the std::locale:.global though)

So, without having looked at Qt's source code (on purpose in this case), my naive assumption as an application developer would be that if I implement a Qt application the regional settings set up in the operating system are followed.

So if my OS is set to French language and European regional settings (using Celsius, metric units, week starts on Monday, floating comma instead of floating point, etc.) I'd expect Qt to adapt the same regional settings automatically for all Qt-specific functionality (e.g. QTime and others). Is that not the case?

For commandline, yes GetCommandLineW would work and also be better than the first workaround that came to my mind about acp->wide->utf8 conversion. I still think the manifest approach is somewhat preferable as an environment configuration rather than introducing more platform specific code. I also see changing of the -A apis by the active code page as thing that would help adoption of 3rd party code. If they used -A apis they either didn't know better or maybe they maintained some existing code later called MultibyteToWideChar() with CP_ACP flag. Both of these cases is fixed/compatible by changing the ACP to UTF-8. Not saying we should convert from -W apis. We can keep that.

The problem is that any existing use of the ANSI APIs without accompanying conversion functions would be wrong conceptually as any such implementation would have always been just one non-ASCII character away from breaking in unpredictable ways (that's precisely one of the "happy accidents" I mentioned above).

And rather than trying to retroactively "fix" code that is broken by design (and would not even pass code review if it were submitted as a PR these days), I'd want to see a correct implementation that is actually aware of character encodings, code pages, and the appropriate Windows APIs. If we change code, we might as well change it into what it should've been from the get-go.

Case in point: The code should have understood that due to it using main (and not wmain) it will be considered a "legacy" application by Windows and thus will have the string in argv encoded in the current Windows code page. It's a wonderful example for when using platform-specific code would have resulted in less code complexity because with wmain all command line arguments would simply be encoded using UTF-16.

For time things, yes we should use a standardized format and use the tools that most approriate in context (Qt). It is somewhat out of the scope of this, so I settled with preserving current results/expectations as much as possible. This can be changed to do ISO formatting though.

It's probably fine here, even though I wouldn't mind a more holistic refactoring (that looks at an issue at a conceptual level first and is happy to discard existing code for something better), because we'd want to make that change sooner rather than later anyway. But I wouldn't hoist that requirement onto this PR.

[Dankirk]

So, without having looked at Qt's source code (on purpose in this case), my naive assumption as an application developer would be that if I implement a Qt application the regional settings set up in the operating system are followed.

So if my OS is set to French language and European regional settings (using Celsius, metric units, week starts on Monday, floating comma instead of floating point, etc.) I'd expect Qt to adapt the same regional settings automatically for all Qt-specific functionality (e.g. QTime and others). Is that not the case?

For Qt APIs it is. Default instance of QLocale is that of systems. It doesn't adapt the codepage though. Qt manages encoding by itself and once initialized only interfaces in utf-8 or itself by default. We do that interfacing correctly.

Qt doesn't change locale/encoding of C/C++ runtime on Windows, but does on unix. It's only the C/C++ we are trying to match. But as said, we could revert and trust the locale setup routines here and in Qt are compatible.

The problem is that any existing use of the ANSI APIs without accompanying conversion functions would be wrong conceptually as any such implementation would have always been just one non-ASCII character away from breaking in unpredictable ways (that's precisely one of the "happy accidents" I mentioned above).

And rather than trying to retroactively "fix" code that is broken by design (and would not even pass code review if it were submitted as a PR these days), I'd want to see a correct implementation that is actually aware of character encodings, code pages, and the appropriate Windows APIs. If we change code, we might as well change it into what it should've been from the get-go.

Isn't this a win for manifest, because it would make them valid. It relaxes the technical expertise to successfully interface. We also don't control all 3rd party code to deny them, like plugins loaded in as dlls (which inherit OBS C\C++ locale, encoding and active code page).

Case in point: The code should have understood that due to it using main (and not wmain) it will be considered a "legacy" application by Windows and thus will have the string in argv encoded in the current Windows code page. It's a wonderful example for when using platform-specific code would have resulted in less code complexity because with wmain all command line arguments would simply be encoded using UTF-16.

Just to clarify, the codepage declaration in the manifest is used to encode argv, despite terminals or Windows default encoding. Since we want argv in utf-8 internally, the manifest would be the least complex way. wmain is Windows only, so we would need multiple mains in ifdefs (Does it work like that?). If we don't use the manifest declaration, I recommend GetCommandLineW for Windows as replacement too.

Finally I want you know I will update the PR and adapt if anything just doesn't sit right despite my arguments.

[PatTheMav]

Explicit casts to unsigned char should be fine IMO, using the locale-independent time format strings is also more correct, though we could use the same corresponding fixed format for the date as well (as commented).

The other changes are still pending discussion.

[PatTheMav]

For Qt APIs it is. Default instance of QLocale is that of systems. It doesn't adapt the codepage though. Qt manages encoding by itself and once initialized only interfaces in utf-8 or itself by default. We do that interfacing correctly.

Qt doesn't change locale/encoding of C/C++ runtime on Windows, but does on unix. It's only the C/C++ we are trying to match. But as said, we could revert and trust the locale setup routines here and in Qt are compatible.

We actually have to distinguish between Windows, macOS, and "the rest", because only on *nix does Qt use low level C APIs for regional formatting due to lack of a universal high-level platform API. On Windows the appropriate modern Windows APIs are used and on macOS both Cocoa and Core Foundation are used, neither of which is affected by setlocale.

Thus from Qt's own perspective there is no need to call setlocale on those two platforms and it's kinda OBS' homegrown problem if it uses low level C APIs for things that we could instead use Qt APIs for (particularly when it's application-layer code).

And if a lower-level C++ API call is necessary (and C API calls should be avoided entirely), then a corresponding facet should be set up based on the current QLocale, as that's the source of truth for locales.

Isn't this a win for manifest, because it would make them valid. It relaxes the technical expertise to successfully interface. We also don't control all 3rd party code to deny them, like plugins loaded in as dlls (which inherit OBS C\C++ locale, encoding and active code page).

The UTF-8 code page is still considered "Beta" even in Windows 11 (because some ANSI methods are still incompatible with it) and its existence is mostly just a kludge. C API methods that accept char * do the same thing as ANSI variants of Windows API methods: They convert the string data into UTF-16 and then call the Unicode variant.

But OBS is already aware of Unicode APIs and has implemented them thoroughly and it's only the command line handling code that is the outlier. So I rather have us fix the outlier to do the right thing than retroactively reward an "ignorant" implementation, particularly if we can avoid superfluous encoding/decoding (that will happen anyway) by making Windows pass us the command line in UTF-16 directly (either by using wmain or GetCommandLineW).

Trying to "help" 3rd party code that exhibits the same ignorance is a non-goal. If we encounter such a broken implementation we should obviously report it to its author(s), but it's not OBS' place to change its app behaviour to "fix" their bugs.

---

IMO it would make more sense to refactor our start up code and move as much of it out of "low level" functions into "high level" application layer code that can use Qt functions and thus side-step those issues entirely.

Such a refactor is planned anyway, and the command-line encoding issue is just another nail in the coffin for the current code.

[PatTheMav]

I forgot to mention in my previous comment that I am still undecided whether to still use the UTF-8 codepage as a "stopgap" until we properly fix the command line parsing code though. It's still a kludge, but as there are plans to clean up that code anyway, it would be a temporary kludge we can live with.

[PatTheMav]

Alright, slept on it, here's my suggestion:

Could you please package all changes of this PR except for the changes related to setlocale into a separate PR?

Off the top of my head that should be:

• Adjustment of time format strings to %F, %T
• Explicit cast to unsigned char
• Use of UTF-8 code page for ANSI/C API methods

I'll probably want to have a comment in obs-main.cpp above obs_set_cmdline_args that explicitly mentions that this requires the UTF-8 code page to be set in the manifest.

I can then have someone else review those changes as well and we can discuss those in isolation from the larger issue of how/where to do locale-specific stuff in the app.

[Dankirk]

Could you please package all changes of this PR except for the changes related to setlocale into a separate PR?

It's here #13097 I'll rebase this branch on to that soon.

In the mean time about how/where to do locale-specific stuff in the app.

Thus from Qt's own perspective there is no need to call setlocale on those two platforms and it's kinda OBS' homegrown problem if it uses low level C APIs for things that we could instead use Qt APIs for (particularly when it's application-layer code).

And if a lower-level C++ API call is necessary (and C API calls should be avoided entirely), then a corresponding facet should be set up based on the current QLocale, as that's the source of truth for locales.

If the facet is set up like it is in obs-text

// obs_get_locale() returns language_region string
const locale loc = locale(obs_get_locale()); 
const ctype<wchar_t> &f = use_facet<ctype<wchar_t>>(loc);

The resulting localization would be no different from what is achieved with setlocale. Both are based on language_region string that is effectively the same in QLocale got from system or one acquired from setlocale() (the example is using translations locale, but anyway) Setting up locale this way only applies language_region defaults per category while QLocale also applies customized localization rules. ( I was wrong about the capability in a code comment) Like if I customized decimal point in OS settings without changing language_region.

Not sure about the rest of the things here, as it is more about architecture I still might be missing some things:

If we wanted more granular localization and broader utf-8 support for the low level char apis identical to what is available in Qt in C++, some work would be needed:

1. Make subclasses of the standard facets
2. Override their member functions to use QLocale/Qt to do the actual thing.
3. Combine the facets for a custom std::locale
4. Potentially call std::locale::global() with the custom locale.

Either way avoiding C API calls sounds like we should move to or extern c++ a bit more, I'm not yet sure how would providing locale to 3rd party plugins go either unless it's just the language_region string(s). On OBS own plugins there's still atleast vlc and slideshow that's in C that could use locale aware sorting for the playlist/slides. Though that could also be bound to the platform libraries that do the directory listing to sort, which suggests libobs should have access. If not rewritten / externed to C++, an api like the procedure/signal would be needed, which is starting to get murky for something as standard as locale-aware operations.

[PatTheMav]

As I mentioned earlier, the core issue is that nothing in OBS' design is built to be aware of locale-specific changes to C APIs and indeed not to the finer points of UTF-8 encoded data:

• Actual locale-awareness is limited to two functions: os_strtod and os_dtostr.
• For anything else "locale" just means "selected language" or "translation"
• Indeed I found that on macOS we use NSLocale to get the list of preferred languages, then convert those into locale names, and return them as a list of locales. So another example of "language" and "locale" being conflated.
• The locale-aware sorting of profile names and scene collections in the menu is a relatively recent addition from when I refactored both. Originally items in those menus appeared in the order in which C file system APIs provided them.

Indeed any first-party as well as third-party code is currently on its own when it wants to do "the right thing" (see examples like updateSortedProfiles). And low-level C code (as plugins tend to be) should not rely on Qt or OBS itself having done the "right thing" but check the current locale setting and switch it accordingly for its own needs.

I also haven't checked whether all C library functions whose behaviour is influenced by the current locale have a corresponding variant that allows passing in a different locale. Otherwise, and because OBS is a multithreaded program, the "current locale" becomes a global state variable that will need to be "locked" similarly to the graphics context.

And while that will allow functions like strcoll to do the right thing (e.g. putting "ch" between "h" and "i" in Czech), it will not be sufficient for converting characters in languages like Turkish. Even with the correct locale set, toupper and tolower will not be able to handle sinirli/SİNİRLİ (angry) and sınırlı/SINIRLI (limited). Lowercase "angry" becomes uppercase "limited", but their opposites become gibberish when not handled correctly ("S??N??RL??" in the case of sınırlı, which seems to suggest that the function took those bytes and just incremented their value which then created invalid UTF-8 code units).

For better (and in this case for worse), OBS provides a bit of a "free-for-all" when it comes to plugins and 3rd party code, providing almost unlimited access to internal functionality. But "with great power comes great responsibility", which means that plugins have to gain all that knowledge about locales and UTF-8 specificities themselves. OBS cannot provide any form of assistance.

(And before OBS would be able to do so, it would need to get its own house in order first, e.g. properly differentiating between "display language" and "locale" among other things).

[Dankirk]

As I mentioned earlier, the core issue is that nothing in OBS' design is built to be aware of locale-specific changes to C APIs and indeed not to the finer points of UTF-8 encoded data

We still do not need everything to be locale or encoding aware. We can use them for what we have and just understand that they have that limitation about them. We only need them to not crash and be deterministic. It would still be good if there was something available to use when we do need the awareness.

I also haven't checked whether all C library functions whose behaviour is influenced by the current locale have a corresponding variant that allows passing in a different locale. Otherwise, and because OBS is a multithreaded program, the "current locale" becomes a global state variable that will need to be "locked" similarly to the graphics context.

Standard C doesn't support passing locales, it (std::locale and facets) are a C++ feature for which the overcoming the earlier process/thread-basedness was one of the core things to address. It would work without locks.

For C there is the non-standard, platform specific _locale_t (windows) and locale_t (posix) and corresponding *_l functions that do the same. Haven't checked if they are cover all use cases either.

Actual locale-awareness is limited to two functions: os_strtod and os_dtostr.

A bit beside the point, but interestingly these functions currently and after do nothing different from standard strtod and dtostr. Because the numeric locale is and will be 'C' on all platforms, which means decimal points are dots in all setups. Except for Qt since it's independent. If you did supply a string/double from Qt api to this function or vice versa, it will fail to convert the decimal point.

And while that will allow functions like strcoll to do the right thing (e.g. putting "ch" between "h" and "i" in Czech), it will not be sufficient for converting characters in languages like Turkish. Even with the correct locale set, toupper and tolower will not be able to handle sinirli/SİNİRLİ (angry) and sınırlı/SINIRLI (limited). Lowercase "angry" becomes uppercase "limited", but their opposites become gibberish when not handled correctly ("S??N??RL??" in the case of sınırlı, which seems to suggest that the function took those bytes and just incremented their value which then created invalid UTF-8 code units).

Indeed using tolower/toupper (or strmpi for case insensitive comparison) is not sufficient for anything locale-aware. If locale awareness is needed, in standard C (otherwise we could use Qt or some other locale library) they should be done with wide char functions for the reasons mentioned, like wcscoll. Tolower/upper also being kind of means to end, it should rather be considered whats the end goal, if it's locale aware case-insesitive comparison we could use wcscmpi_l if we adopt the non-standard C platform specifics.

We also need to think whether we actually need true localized case sensitivity or is the current ascii limited/byte-wise comparison desired. For example checking for existing profile or source name, because then you could change locale and affect the case sensitivity comparison to match/not match an existing profile / source name.

This has given me some thoughts on what to look for and address though, so I will be looking for those, though that may take some time.

[Dankirk]

For reference, I just introduced some new locale functionality for libsobs/util/platform in another pr here #12577

It follows the idea of using locale only where needed instead of setting a global one. So if this approach doesn't bear fruit, or is too much of a lengthy process, maybe that one could for now.

[PatTheMav]

If you don't mind coarse language you might be interested in reading the commit "stream_libarchive: workaround various types of locale braindeath" on the mpv repository which encapsulates a whole lot of frustration many people have with locales in C and why it's usually better to ignore that functionality entirely. It's a classic (including the discussions about what Unicode format Windows might actually use under the hood and all). 😉

[Dankirk]

If you don't mind coarse language you might be interested in reading the commit "stream_libarchive: workaround various types of locale braindeath" on the mpv repository which encapsulates a whole lot of frustration many people have with locales in C and why it's usually better to ignore that functionality entirely. It's a classic (including the discussions about what Unicode format Windows might actually use under the hood and all). 😉

It's great, and I agree. We have touched many of the same points. But here we are weighting options how to best apply locale awareness to stuff written in C. If not global locale or the per call locale_t way proposed in #12577 then it would need a way to use Qt api with some type conversion overhead with wrapping the sortables in a struct with QString to sort with or some such.