mpv-player / mpv

🎥 Command line video player
https://mpv.io
Other
28.25k stars 2.9k forks source link

Shaders fail to compile on macOS due to setlocale() call #7161

Open Midar opened 4 years ago

Midar commented 4 years ago

mpv version and platform

mpv 0.30.0, macOS Catalina, built from pkgsrc. GeForce GT 750M.

Reproduction steps

Just try to play any video.

Expected behavior

I see a video.

Actual behavior

The video plays, the screen shows blue. Looking at the termial, the shader failed to compile because of insufficient arguments to the vec3 initializer.

Looking at it, it seems vec3(x) is used rather than vec3(x, x, x). I'm guessing vec3(x) was added in a later OpenGL version not supported by macOS on a GT 750M.

Log file

[vo/gpu] opengl cocoa backend is deprecated, use vo=libmpv instead AO: [coreaudio] 48000Hz stereo 2ch floatp VO: [gpu] 1920x1080 yuv420p [vo/gpu/opengl] fragment shader source: [vo/gpu/opengl] [ 1] #version 410 [vo/gpu/opengl] [ 2] #define tex1D texture [vo/gpu/opengl] [ 3] #define tex3D texture [vo/gpu/opengl] [ 4] #define LUT_POS(x, lut_size) mix(0.5 / (lut_size), 1.0 - 0.5 / (lut_size), (x)) [vo/gpu/opengl] [ 5] out vec4 out_color; [vo/gpu/opengl] [ 6] in vec2 texcoord0; [vo/gpu/opengl] [ 7] in vec2 texcoord1; [vo/gpu/opengl] [ 8] in vec2 texcoord2; [vo/gpu/opengl] [ 9] uniform mat3 colormatrix; [vo/gpu/opengl] [ 10] uniform vec3 colormatrix_c; [vo/gpu/opengl] [ 11] uniform vec3 src_luma; [vo/gpu/opengl] [ 12] uniform vec3 dst_luma; [vo/gpu/opengl] [ 13] uniform sampler2D texture0; [vo/gpu/opengl] [ 14] uniform vec2 texture_size0; [vo/gpu/opengl] [ 15] uniform mat2 texture_rot0; [vo/gpu/opengl] [ 16] uniform vec2 texture_off0; [vo/gpu/opengl] [ 17] uniform vec2 pixel_size0; [vo/gpu/opengl] [ 18] uniform sampler2D texture1; [vo/gpu/opengl] [ 19] uniform vec2 texture_size1; [vo/gpu/opengl] [ 20] uniform mat2 texture_rot1; [vo/gpu/opengl] [ 21] uniform vec2 texture_off1; [vo/gpu/opengl] [ 22] uniform vec2 pixel_size1; [vo/gpu/opengl] [ 23] uniform sampler2D texture2; [vo/gpu/opengl] [ 24] uniform vec2 texture_size2; [vo/gpu/opengl] [ 25] uniform mat2 texture_rot2; [vo/gpu/opengl] [ 26] uniform vec2 texture_off2; [vo/gpu/opengl] [ 27] uniform vec2 pixel_size2; [vo/gpu/opengl] [ 28] void main() { [vo/gpu/opengl] [ 29] vec4 color = vec4(0.0, 0.0, 0.0, 1.0); [vo/gpu/opengl] [ 30] color.r = 1,000000 vec4(texture(texture0, texcoord0)).r; [vo/gpu/opengl] [ 31] color.g = 1,000000 vec4(texture(texture1, texcoord1)).r; [vo/gpu/opengl] [ 32] color.b = 1,000000 vec4(texture(texture2, texcoord2)).r; [vo/gpu/opengl] [ 33] color = color.rgbr; [vo/gpu/opengl] [ 34] color.rgb = mat3(colormatrix) color.rgb + colormatrix_c; [vo/gpu/opengl] [ 35] color.a = 1.0; [vo/gpu/opengl] [ 36] // color mapping [vo/gpu/opengl] [ 37] color.rgb = vec3(1,000000); [vo/gpu/opengl] [ 38] color.rgb = vec3(1,000000); [vo/gpu/opengl] [ 39] out_color = color; [vo/gpu/opengl] [ 40] } [vo/gpu/opengl] fragment shader compile log (status=0): [vo/gpu/opengl] ERROR: 0:37: Too few arguments to constructor of 'vec3' [vo/gpu/opengl] ERROR: 0:38: Too few arguments to constructor of 'vec3' [vo/gpu/opengl] [vo/gpu/opengl] shader link log (status=0): ERROR: One or more attached shaders not successfully compiled [vo/gpu/opengl]

Sample files

Pick any file, it's not specific to any file.

sfan5 commented 4 years ago

color.rgb *= vec3(1,000000);

This is incorrect syntax, it should say 1.000000 Looks like some code set locale to German thereby breaking number output, we've had this locale bullshit before.

CounterPillow commented 4 years ago

It's also worth noting that that entire line is wholly unnecessary, what even generates it?

ghost commented 4 years ago

Something sets locale.

Midar commented 4 years ago

@sfan5 Ah, good catch - I completely missed the "," instead of the ".". German locale indeed.

@CounterPillow I think it's this:

    // Pre-scale the incoming values into an absolute scale
    GLSLF("color.rgb *= vec3(%f);\n", mp_trc_nom_peak(src.gamma));
Midar commented 4 years ago

FWIW, I think it might be that some AppKit stuff does setlocale(LC_ALL, ""), which does affect printf. There's printf_l(3) to work around this.

Midar commented 4 years ago

Another more portable, but hacky way (in case this also happens on other platforms - I know gtk2 does similar shenanigans), would be to look at localeconv()->decimal_point and if this is not ., replace all occurrences of localeconv()->decimal_point with ..

I did something like this in a printf wrapper I wrote (using printf_l on macOS, falling back to replacing for that specific part of the printf (rather than the whole result)): https://github.com/ObjFW/ObjFW/blob/master/src/of_asprintf.m#L525

Feel free to steal if it's useful, as it's GPL :)

ghost commented 4 years ago

It would be more useful to track down the setlocale call which changes this.

Midar commented 4 years ago

It comes from AppKit initialization, which triggers a bunch of init of Foundation, CoreFoundation and ICU, that eventually ends up with setlocale(), so will be hard to impossible to avoid:

* thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
  * frame #0: 0x00007fff7093a604 libsystem_c.dylib`setlocale
    frame #1: 0x00007fff6e5dd93d libicucore.A.dylib`uprv_getDefaultLocaleID + 39
    frame #2: 0x00007fff6e5ca286 libicucore.A.dylib`___lldb_unnamed_symbol186$$libicucore.A.dylib + 92
    frame #3: 0x00007fff6e5ca924 libicucore.A.dylib`icu::Locale::getDefault() + 64
    frame #4: 0x00007fff6e5ca8dd libicucore.A.dylib`___lldb_unnamed_symbol191$$libicucore.A.dylib + 9
    frame #5: 0x00007fff6e6543c5 libicucore.A.dylib`___lldb_unnamed_symbol1159$$libicucore.A.dylib + 43
    frame #6: 0x00007fff6e652b6f libicucore.A.dylib`___lldb_unnamed_symbol1156$$libicucore.A.dylib + 644
    frame #7: 0x00007fff6e5ce2c0 libicucore.A.dylib`uloc_getTableStringWithFallback + 154
    frame #8: 0x00007fff6e5c7652 libicucore.A.dylib`___lldb_unnamed_symbol152$$libicucore.A.dylib + 109
    frame #9: 0x00007fff6e5c5fd9 libicucore.A.dylib`uloc_getDisplayLanguage + 180
    frame #10: 0x00007fff394081fe CoreFoundation`__CFLocaleLanguageName + 73
    frame #11: 0x00007fff39408007 CoreFoundation`CFLocaleCopyDisplayNameForPropertyValue + 296
    frame #12: 0x00007fff39407ecb CoreFoundation`-[__NSCFLocale displayNameForKey:value:] + 15
    frame #13: 0x00007fff3ba7af3e Foundation`-[NSUserDefaults(NSUserDefaults) init] + 1646
    frame #14: 0x00007fff3ba83146 Foundation`+[NSUserDefaults(NSUserDefaults) standardUserDefaults] + 66
    frame #15: 0x00007fff365cdaf4 AppKit`+[NSApplication initialize] + 87
    frame #16: 0x00007fff6f54f9b5 libobjc.A.dylib`CALLING_SOME_+initialize_METHOD + 17
    frame #17: 0x00007fff6f5502ec libobjc.A.dylib`initializeNonMetaClass + 638
    frame #18: 0x00007fff6f5500ba libobjc.A.dylib`initializeNonMetaClass + 76
    frame #19: 0x00007fff6f5509c1 libobjc.A.dylib`initializeAndMaybeRelock(objc_class*, objc_object*, mutex_tt<false>&, bool) + 214
    frame #20: 0x00007fff6f54241b libobjc.A.dylib`lookUpImpOrForward + 969
    frame #21: 0x00007fff6f541bd9 libobjc.A.dylib`_objc_msgSend_uncached + 73
    frame #22: 0x00000001000e3a61 mpv`cocoa_main + 753
    frame #23: 0x00007fff708b02e5 libdyld.dylib`start + 1
    frame #24: 0x00007fff708b02e5 libdyld.dylib`start + 1
ghost commented 4 years ago

Simply amazing Apple brokenness.

We can probably restore the locale after initializing Cocoa for now.

Midar commented 4 years ago

@wm4: Im not sure if this will not break more things. I think after locale has been initialized in AppKit, you cannot change it. This is also not brokenness that is exclusive to Apple: Gtk does the same thing. Locale as a concept is broken, as it thinks all printf-style functions are only ever used for user-visible output. If anything, Apple somewhat fixes is with the _l variant: No such thing exists on Linux, unfortunately, meaning if you use Gtk on Linux, you're in for a fun ride.

How about just defining printf and friends to printf_l using the C locale? Or, if mpv never uses locale: A simple wrapper script with LC_ALL=C will do and makes sure that AppKit also uses no locale.

ghost commented 4 years ago

GTK doesn't set locale, though Qt does.

I'd rather go with libinsanity in the long run.

Midar commented 4 years ago

Sure, though that doesn't sound all that different from the printf implementation I linked above :). It makes all the floating point formats always use ".", unless they are prefixed with a ",", like in "%,d", to specify it should be localized, still giving you both options.

As for Gtk: At least on my Mac it does the same - but now that I think about it, that might just be because it uses AppKit eventually.

Akemi commented 4 years ago

out of curiosity, i wonder if this is another thing that changed in 10.15.