Skip to content

vo_gpu: initial support for tunable parameters#17700

Merged
kasper93 merged 5 commits intompv-player:masterfrom
N-R-K:nrk/gpu-params
Apr 17, 2026
Merged

vo_gpu: initial support for tunable parameters#17700
kasper93 merged 5 commits intompv-player:masterfrom
N-R-K:nrk/gpu-params

Conversation

@N-R-K
Copy link
Copy Markdown
Contributor

@N-R-K N-R-K commented Apr 4, 2026

does not support everything supported by gpu-next/libplacebo,
only a subset that was needed to run some user shaders.

N-R-K added 2 commits April 4, 2026 18:09
@N-R-K
shader_cache: add gl_sc_uniform_{f,i}_bstr()
6d320f2 
this enables callers that already have bstr names to avoid
unnecessary string conversions.
@N-R-K
vo_gpu: initial support for tunable parameters
741c5a7 
does not support everything supported by gpu-next/libplacebo,
only a subset that was needed to run some user shaders.
@N-R-K
Copy link
Copy Markdown
Contributor Author

N-R-K commented Apr 4, 2026 •
edited
Loading

Could use some more testing, but it seems to be enough to run CfL_Prediction.glsl for example.

Also if anyone knows any other user shader that uses PARAM blocks, please share.

@na-na-hi
Copy link
Copy Markdown
Contributor

na-na-hi commented Apr 4, 2026

Also if anyone knows any other user shader that uses PARAM blocks, please share.

Many shaders from the following do:

https://github.com/hooke007/mpv_PlayKit/tree/main/portable_config/shaders
https://github.com/natural-harmonia-gropius/hdr-toys/tree/master/shaders/hdr-toys
https://github.com/hhirtz/mpv-retro-shaders

@kasper93
Copy link
Copy Markdown
Member

kasper93 commented Apr 4, 2026

Also if anyone knows any other user shader that uses PARAM blocks, please share.

https://github.com/kasper93/mpv360

@N-R-K
Copy link
Copy Markdown
Contributor Author

N-R-K commented Apr 5, 2026

https://github.com/hooke007/mpv_PlayKit/tree/main/portable_config/shaders
https://github.com/natural-harmonia-gropius/hdr-toys/tree/master/shaders/hdr-toys
https://github.com/hhirtz/mpv-retro-shaders
https://github.com/kasper93/mpv360

Thanks. From some quick grepping, it seems like nearly all the features are used in practice aside from DYNAMIC.

I can work on the the lower hanging fruits. But implementing a couple these (I presume) will not be straightforward.

So is it okay to only support the lower hanging parts initially and leave out the non-trivial stuff for later, or would that be a blocker?

@na-na-hi
Copy link
Copy Markdown
Contributor

na-na-hi commented Apr 5, 2026

I can work on the the lower hanging fruits. But implementing a couple these (I presume) will not be straightforward.

So is it okay to only support the lower hanging parts initially and leave out the non-trivial stuff for later, or would that be a blocker?

I think it should be evaluated if implementing the remaining features requires substantial rewrite of the code added in this PR. If the substantial rewrite is not needed, then this PR can stay as an incremental change.

@N-R-K
Copy link
Copy Markdown
Contributor Author

N-R-K commented Apr 5, 2026

I think it should be evaluated if implementing the remaining features requires substantial rewrite of the code added in this PR. If the substantial rewrite is not needed, then this PR can stay as an incremental change.

Majority of the code added here is just parser code, which is not an accident since I wanted to start with simple changes (enough to make CfL work).

I think enum is the only thing that might be a bit of a problem, but it's should be doable with the current structure. I plan on adding this since it seems interesting.

I actually had parsing support for unit too but dropped it since it'd require adding RA_VARTYPE_UINT and making changes wherever the type is switched/branched upon. Not hard, just tedium I didn't want to deal with.

The DYNAMIC and CONSTANT thing requires more investigation. I'll do that tomorrow and assess if that changes anything fundamentally.

@kasper93
Copy link
Copy Markdown
Member

kasper93 commented Apr 5, 2026 •
edited
Loading

Thanks. From some quick grepping, it seems like nearly all the features are used in practice aside from DYNAMIC.

That's good point, I should be using DYNAMIC. Will update when I get time.

The DYNAMIC and CONSTANT thing requires more investigation. I'll do that tomorrow and assess if that changes anything fundamentally.

Worse case you can treat all variables as const, but for bigger shaders recompilation on every update may be problematic.

@N-R-K
Copy link
Copy Markdown
Contributor Author

N-R-K commented Apr 6, 2026

Added support for ENUM and DEFINE (as separate commits to show how new additions would look like, can squash if needed).

N-R-K
N-R-K commented Apr 7, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor Author

@N-R-K N-R-K left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR can also run mpv360 at it's current state. Marking this ready for review since I think it's a good enough baseline.

The missing stuff can be added incrementally later and shouldn't require any large rewrite of the parser code added here.

Comment thread video/out/gpu/user_shaders.h
double max;
bool has_min;
bool has_max;
struct bstr enum_body;
Copy link
Copy Markdown
Contributor Author

@N-R-K N-R-K Apr 7, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Felt like it'd be easier to just collect the enum body in a single bstr and iterate on demand. Avoids placing static limits and also avoids having to carry a dynamic array.

Could just change it to either dynamic array or array with static limit if that's preferred.

Comment thread video/out/gpu/user_shaders.h
struct bstr hook_tex[SHADER_MAX_HOOKS];
struct bstr bind_tex[SHADER_MAX_BINDS];
struct gl_user_shader_param params[SHADER_MAX_PARAMS];
int num_params;
Copy link
Copy Markdown
Contributor Author

@N-R-K N-R-K Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seemed reasonable. But could change to dynamic array too.

@N-R-K N-R-K marked this pull request as ready for review April 7, 2026 04:08
kasper93
kasper93 reviewed Apr 8, 2026
View reviewed changes
Comment thread video/out/gpu/video.c Outdated
@N-R-K
vo_gpu/utils: add gpu_get_auto_param()
0bc5a76 
deduplicates some code between vo_gpu and vo_gpu_next
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 9, 2026

Download the artifacts for this pull request:

Windows
macOS

kasper93
kasper93 approved these changes Apr 17, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@kasper93 kasper93 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

Shouldn't break old shaders, so why not.

@kasper93 kasper93 merged commit c415c66 into mpv-player:master Apr 17, 2026
30 checks passed
@N-R-K N-R-K deleted the nrk/gpu-params branch April 17, 2026 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kasper93 kasper93 kasper93 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@N-R-K @na-na-hi @kasper93