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

Make source content metadata available to shaders as preprocessor variables #9914

Open
agyild opened this issue Feb 25, 2022 · 11 comments
Open

Comments

@agyild
Copy link

agyild commented Feb 25, 2022

This was already requested in another ticket, but I am opening this ticket to officially follow-up on the request.

As the title suggests, source content metadata (e.g., __PRIMARIES__, __GAMMA__, __COLORMATRIX__ etc.) could be useful for automating necessary conversions in shaders without requiring the user to configure them manually.

In addition, make them available for use in !WHEN directives, so for instance a conversion pass is only executed if it is actually needed.

I am personally in need of this feature right now, and I would really appreciate it if it was available.

@agyild agyild changed the title Make source content metadata available to shaders as macro variables Make source content metadata available to shaders as preprocessor variables Feb 25, 2022
@haasn
Copy link
Member

haasn commented Feb 25, 2022

I'm not a huge fan of this because it's not clear to me how this value should be interpreted by users, as it's just a number - how do you implement something useful based on it without essentially hard-coding a big conditional list?

I'm also uncomfortable having anything resembling internal enum constants leaking into the public API like that, since it essentially enshrines the current list of transfer functions for all eternity - and these are enums that are not intended to have permanent significance. It's why mpv itself uses strings to refer to them in all public APIs. But the shader system, of course, does not have strings. (Though we could work around it by exposing them as a hash of the string name.. ugh)

What's your use case? Maybe there's a better solution.

@agyild
Copy link
Author

agyild commented Feb 25, 2022

What's your use case? Maybe there's a better solution.

Applying transform functions depending on the source gamma. CAS requires linear input, FSR requires sRGB/Gamma 2.0 input. Shader development for video content is not the same as developing for real-time rendered content where everything is either sRGB or PQ or Linear, and the shader code is baked into the overall code where colorspace decisions happen before the shader compilation. In mpv's case I need to dynamically modify the code via preprocessor macros to adapt itself to the source content. The alternative is a code management nightmare where we have several versions of the same code to apply via autoprofiles or whatever.

I'm also uncomfortable having anything resembling internal enum constants leaking into the public API like that, since it essentially enshrines the current list of transfer functions for all eternity - and these are enums that are not intended to have permanent significance. It's why mpv itself uses strings to refer to them in all public APIs. But the shader system, of course, does not have strings. (Though we could work around it by exposing them as a hash of the string name.. ugh)

In reality is no such thing called strings, it's just an unique combination of RW byte values in a memory. So in this fashion, hashing is not really "bad design" as it still refers to that unique combination. The shader developer does not need to know what preprocessor variables refer to. I can basically consume those variables like this:

#if (__GAMMA__ == __GAMMA_PQ__)
   // do something
#endif

So the upstream can change those values in the feature, and essentially they become strings-like.

@haasn
Copy link
Member

haasn commented Feb 25, 2022

Applying transform functions depending on the source gamma. CAS requires linear input, FSR requires sRGB/Gamma 2.0 input. Shader development for video content is not the same as developing for real-time rendered content where everything is either sRGB or PQ or Linear, and the shader code is baked into the overall code where colorspace decisions happen before the shader compilation. In mpv's case I need to dynamically modify the code via preprocessor macros to adapt itself to the source content. The alternative is a code management nightmare where we have several versions of the same code to apply via autoprofiles or whatever.

Does the approach from here not work for you?

@agyild
Copy link
Author

agyild commented Feb 25, 2022

Does the approach from here not work for you?

It looks okay, but realistically it is unusable until gpu-next becomes stable. Also how do I use it? I would appreciate documentation.

Other issues are: if those are not constant compile-time values (or even when they are) we risk branching in shaders, and it is not flexible because it only captures a single use. The design I have mentioned is more flexible and is open to more use cases.

@haasn
Copy link
Member

haasn commented Feb 25, 2022

Also how do I use it? I would appreciate documentation.

vec4 color = MAIN_tex(MAIN_pos);
color = linearize(color);
// ...
return delinearize(color);

Other issues are: if those are not constant compile-time values (or even when they are) we risk branching in shaders, and it is not flexible because it only captures a single use.

I would argue that it is more flexible because it allows us to extend the list of gamma functions without every shader needing to be updated.

Not sure what you mean by "constant compile-time values", these are functions.

@agyild
Copy link
Author

agyild commented Feb 25, 2022

Does linearize() accept every possible data type?

In that case I want two additional functions besides linearize() to transform source gamma to Gamma 2.0 (or any custom Gamma) and sRGB, and their inverse functions. And I want them to be available in gpu besides gpu-next.

@haasn
Copy link
Member

haasn commented Feb 25, 2022

Does linearize() accept every possible data type?

No, just vec4. You can apply gamma 2.2 (or sRGB) yourself, e.g. pow(color, vec2(2.2)).

@haasn
Copy link
Member

haasn commented Feb 25, 2022

(Something to keep in mind is that not all gamma functions can be independently applied per channel. For example, HLG cannot.)

@agyild
Copy link
Author

agyild commented Feb 25, 2022

You can apply gamma 2.2 (or sRGB) yourself, e.g. pow(color, vec2(2.2)).

Do you mean to the linearized output?

(Something to keep in mind is that not all gamma functions can be independently applied per channel. For example, HLG cannot.)

What about LUMA textures? How does it work there?

@haasn
Copy link
Member

haasn commented Feb 25, 2022

What about LUMA textures? How does it work there?

float luma_lin = linearize(LUMA_texOff(0).rrra).r;

@agyild
Copy link
Author

agyild commented Feb 25, 2022

Very well. I'll give it a try once the bugs in gpu-next becomes fixed, because right now it doesn't play well with = in reverse expressions and gives errors unless the shader is applied after upscaling. Or once the same becomes available in gpu. Personally I am not a fan of requiring a WIP backend for a production shader.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants