vapoursynth / vapoursynth

A video processing framework with simplicity in mind
http://www.vapoursynth.com/
GNU Lesser General Public License v2.1
1.64k stars 151 forks source link

API v4: What is the new API for `vsscript_getVariable` #743

Closed CrendKing closed 3 years ago

CrendKing commented 3 years ago

Previously, if the script creates a Python variable foo, after evaluating the file vsscript_getVariable could retrieve the variable value. What is the equivalent API in v4? Is it getOptions? According to its comment, there would be a set_option in Python, but I don't find any doc of this function in standard Python. Could you give me some pointer?

AkarinVS commented 3 years ago

it's in the vapoursynth package, and you can use get_option/get_options to retrieve set options as well. e.g.

import vapoursynth as vs vs.set_option('a',1) vs.get_options() mappingproxy({'a': 1})

CrendKing commented 3 years ago

So set_option is one of the new Python function in VapourSynth v4? Is it mentioned somewhere in the API v4 change doc? The closest I found is the set_options(environment, options) from https://github.com/vapoursynth/vapoursynth/blob/R55-API4-RC1/doc/pythonreference.rst, but it doesn't seem to be the one.

Also, what is the significant design consideration to add this extra layer of "options" map instead of the old direct VSMap output? Would there be different kind of outputs other than "options" in the future?

Finally, I suggest adding "output" to the function (e.g. getOutputOptions) so that users know the nature of that map. Same for that vars argument for the evaluate* functions in VSScript (e.g. inputVars).

myrsloik commented 3 years ago

Yes. No, vsscript/python bit isn't finished so I didn't document it at all.

Design consideration: Assigning special meaning to what appears to be normal variables is not very intuitive and I wanted a clearer way to specify what information should be returnable/readable. I'm also not aware of anyone reading out variables (more than my own enable_v210/alt_output use) and this is a less bad solution for that. Judging by your reaction you were probably using it for something else...

I'll probably make alt_output an extra argument of set_output() before the final release. Feel free to give examples of how I've ruined your scripts and how to make it less bad.

I suppose adding output/input to things makes sense.

CrendKing commented 3 years ago

Understood. Making output variables standing out is not bad idea, but the name "option" is not very straightforward to me. I wouldn't guess it's for output the first glance.

I'm the author of https://github.com/CrendKing/avisynth_filter/. I use this this variable to let user to tell the DS filter if he wants to "shutdown" the functionality. It's similar to how I use the input variable (and similar to mpv's video_in) to convey source node. I think the variable form is easier for user to deal with than function form, which is commonly used in AviSynth.

CrendKing commented 3 years ago

Hmmm, I just realized that the "vars" argument for evaluate* functions are not the same as the old vsscript_setVariable, since they only allow simple types and I need to pass a Node (like mpv's video_in). Since you mentioned the VSScript part is not yet finalized, so I just want to chime in for that use case.

myrsloik commented 3 years ago

Don't worry. I just hit the same problem myself in another project and things will be reverted to something much closer to the previous state. Will post here again when all the changes are in.

myrsloik commented 3 years ago

Functionality restored. Take another look.

CrendKing commented 3 years ago

Thank you! I see you restored getVariable, but how about setVariable (my questions at https://github.com/vapoursynth/vapoursynth/issues/743#issuecomment-906833892)? Is there alternative or new way to do that?

myrsloik commented 3 years ago

I restored the input vars to their full power so now you can set nodes and other complex types too. Maybe I should split that bit off into a setVariables function too...

CrendKing commented 3 years ago

OK, let me try these.

myrsloik commented 3 years ago

Now setVariables is its own thing again. Failed experiments in API design since it's more or less gone back to the old way now.

CrendKing commented 3 years ago

Tested and seems working as before. Thanks.