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

convert array to arguments, and others issues about array #226

Closed
realfinder opened this issue Aug 15, 2021 · 23 comments
Closed

convert array to arguments, and others issues about array #226

realfinder opened this issue Aug 15, 2021 · 23 comments

Comments

@realfinder
Copy link

realfinder commented Aug 15, 2021

as said here https://forum.doom9.org/showpost.php?p=1949717&postcount=1258

in this read me there are
Quote:

"array" or "val_array": array of any type. When unnamed, then this kind of parameter must be the very last one. Unnamed free-typed parametes cannot be followed by any other parameter. Translates to ".*" in a plugin parameter definition rule.

if I save this https://github.com/pinterf/Average wrapper as Average.avsi

Code:
Function Average(array a)
{
LoadPlugin("C:\path\Average.dll")
        Average(a)
}
then load it like this

Code:
ClearAutoloadDirs()
ColorBars(width=640, height=480, pixel_type="yv12")
import("Average.avsi")
Average(Blur(1),0.5,last,0.5)
it should work but I get


seems I missed something or something is already missing in avs side (way to convert array to arguments maybe?)
@realfinder
Copy link
Author

realfinder commented Aug 17, 2021

also: "+" mean "one or more" "*" mean "zero or more" right?

Scripts arrays only do "*" ?

@pinterf
Copy link

pinterf commented Sep 1, 2021

Yes, Script arrays can do only '*': zero or more.

Function Average(array a) translates to parameter definition ".*" (zero or more of anything, this is needed for handling an arbitrary length of clip, weight, [clip, weight...] sequence)

Inside your script parameter 'a' is an explicite 'array' type, which is a single type - single variable.

Arrays in old compatible behaviour are somewhat 'hybrid' ones: arrays are always 'flattened' before recognizing them against a function definition.

Array as single variable: [clip1, value] - one variable having two elements inside.

Array as a 'flattened' one:
clip1, value - two variables. Count of parameters is 2 which is even.
or
clip1, value, clip2, value2 - four variables. Count of parameters is 4 which is even.

Script level keeps 'a' as an array and does not 'flatten' the array content into a non-array parameter sequence.

If Average would be written that it checks if its first (and only) parameter is an embedded array then it would work with this syntax.

@realfinder
Copy link
Author

yes, I also think that the case

there is function that convert arguments (Array as a 'flattened') to single variable in avs+ already called "Array()" I wonder if it possible to add function for opposite case and called UnArray() or another better name?

also can script arrays get "+" (one or more) as new "_arrays" like int_arrays to distinguish from "_array" which mean "*" (zero or more)?

@pinterf
Copy link

pinterf commented Sep 1, 2021

Unfortunately one function can have only one result, In this case we'd need array.size number of results which is impossible.
I had many troubles on keeping the old behaviour (keep recognition of comma delimited parameters in script as an array) against the real array concept. So far this was the least failing approach that I was able to implement.

@pinterf
Copy link

pinterf commented Sep 1, 2021

I think your proposal, the ..._arrays syntax can be easily implemented, one can poll language lovers about another name, though I don't think they find a much better descriptive name.

@realfinder
Copy link
Author

Unfortunately one function can have only one result, In this case we'd need array.size number of results which is impossible.
I had many troubles on keeping the old behaviour (keep recognition of comma delimited parameters in script as an array) against the real array concept. So far this was the least failing approach that I was able to implement.

I see, so it's impossible...

I think your proposal, the ..._arrays syntax can be easily implemented, one can poll language lovers about another name, though I don't think they find a much better descriptive name.

good to know, will see what the others will say

also, I note that non-array parameters in plugins accept arrays (BM3D and fmtconv both are examples of that), while in scripts it will give error message, it will be good if script function do as plugins behavior

@pinterf
Copy link

pinterf commented Sep 1, 2021

Yep, similar behaviour would be welcomed.
I'd need an exact comparison what works for fmtconv/bm3d as a plugin and does not work an identically parameterized script function. Maybe that script function's parameter signature is not the very same as of those plugins?

@realfinder
Copy link
Author

here https://github.com/WolframRhodium/VapourSynth-BM3DCUDA/blob/8339115ff736005b9c9f8e60e4ac8718e3a508bc/cpu_source/source_avs.cpp#L461 (note they all not array type) but they work with array https://forum.doom9.org/showpost.php?p=1949485&postcount=64
but with

ClearAutoloadDirs()
ColorBars(width=640, height=480, pixel_type="yv12")
convertbits(32)
Function BM3D_CPU(clip clip, clip "ref", float "sigma", int "block_step", int "bm_range", int "radius", int "ps_num", int "ps_range", bool "chroma")
{
LoadPlugin("C:\path\BM3DCPU.dll")
        BM3D_CPU(clip, ref, sigma, block_step, bm_range, radius, ps_num, ps_range, chroma)
}
BM3D_CPU(sigma=[0.5, 0.0, 0.0])

will give
image

@pinterf
Copy link

pinterf commented Sep 1, 2021

Thanks, I'm just looked into its syntax at http://avisynth.nl/index.php/BM3DCUDA
BM3D_CUDA (clip, clip "ref", float[] "sigma", int[] "block_step", int[] "bm_range", int "radius", int[] "ps_num", int[] "ps_range", bool "chroma", int "device_id", bool "fast", int "extractor_exp")
Thanks for the example, I'm saving it for myself.
EDIT: didn't you mean to write float_array "sigma" instead of float "sigma" ?

@realfinder
Copy link
Author

EDIT: didn't you mean to write float_array "sigma" instead of float "sigma" ?

with float_array it will be endless loop and then crash since it will use the script function over and over since both dll and script functions not match

@pinterf
Copy link

pinterf commented Sep 1, 2021

Clearly, you cannot pass [] array of floats to a float "sigma". It will always report "Invalid arguments" error message.
You have to make the two function call different. We can explicitely prefix the original DLL function with the loaded DLL's name, e.g. for purposely calling BM3D_CPU of BM3DCPU.DLL you need to use

  LoadPlugin("C:\path\BM3DCPU.dll")
  BM3DCPU_BM3D_CPU(clip, ref, sigma, block_step, bm_range, radius, ps_num, ps_range, chroma)

@realfinder
Copy link
Author

realfinder commented Sep 1, 2021

You have to make the two function call different.

IIRC I did try that and it's not work too, the problem is in dll it's literally (clip clip, clip "ref", float "sigma", int "block_step", int "bm_range", int "radius", int "ps_num", int "ps_range", bool "chroma") so any thing different in the script wrapper will give endless loop and then crash

edit: I was think you mean "two function call" with same name but with different parameters..., well if the script one different than the dll it will work indeed but that not what I want, I want it with same name...

edit of edit: it will not have endless loop and then crash but it will not work, since arrays not undefined so it will still need the ugly way below...

the only ugly way that seems work (I didn't test enough though) is https://forum.doom9.org/showpost.php?p=1949856&postcount=70 and maybe after adding "_arrays" ('+': one or more) it can be edit to be less ugly

also for fmtconv (also has non-array parameters but accept arrays) I think it will be worst since I plan to porting Dither_resize16nr to fmtc_resamplenr which is not simple function as the wrapper above

@realfinder
Copy link
Author

I note another weird thing https://github.com/EleonoreMizo/fmtconv/blob/aece6c775c8e9d244c2564b759f60e62418365b2/src/main-avs.cpp#L42 planes parameter is string but it not show error with fmtc_bitdepth(16,planes=array(3,-128,1)) but it not do what it should so I think the plugins will accept anything if it array right?

@pinterf
Copy link

pinterf commented Sep 2, 2021

Back to BM3D: I checked the _CPU function signature in the DLL and "sigma" is really a single float, not an array.

@realfinder
Copy link
Author

realfinder commented Sep 2, 2021

Back to BM3D: I checked the _CPU function signature in the DLL and "sigma" is really a single float, not an array.

yes, but it work with array BM3D_CPU(sigma=[0.5, 0.0, 0.0]) unlike fmtconv

@realfinder
Copy link
Author

seems I misunderstanding fmtconv doc, it's not like BM3D, it's use val "." to use array inside string

@pinterf
Copy link

pinterf commented Sep 6, 2021

Ok. Then I'll investigate why BM3D_CPU(sigma=[0.5, 0.0, 0.0]) is allowed to work

@pinterf
Copy link

pinterf commented Sep 6, 2021

This build is giving error message when an explicite array is passed to a non-array argument.
https://drive.google.com/uc?export=download&id=19Bq8y-jUTQ0J_M3aj5foEyugIwKCG8Up
e.g. BM3D_CPU(sigma=[0.5, 0.0, 0.0]) is invalid because sigma is of a single 'f' (float) type.

@realfinder realfinder changed the title convert array to arguments convert array to arguments, and others issues about array Sep 7, 2021
@realfinder
Copy link
Author

thanks, so that mean BM3D need to update to + array now?

@WolframRhodium :)

@WolframRhodium
Copy link

Fixed in 611f56. Thanks for the clarification.

@realfinder
Copy link
Author

I will close this, continuation of the discussion #203 (comment)

@realfinder
Copy link
Author

is this https://forum.doom9.org/showpost.php?p=1953137&postcount=42 plugin problem or avs+ problem?

@pinterf
Copy link

pinterf commented Sep 26, 2021

Don't know yet the exact use case, but tried to explain it a bit.

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

No branches or pull requests

3 participants