pinterf / AviSynthPlus

AviSynth with improvements
http://avs-plus.net
208 stars 24 forks source link

some issues in autoload plugins #11

Open realfinder opened 7 years ago

realfinder commented 7 years ago

_1st one is colorsrgb.avsi it's not work in avs+ at all unless you load it, for more info see https://forum.doom9.org/showpost.php?p=1714618&postcount=1045 (this one is fixed)

2nd one is the order of directories https://forum.doom9.org/showthread.php?p=1800799#post1800799 and https://forum.doom9.org/showthread.php?p=1880154#post1880154 (this one is fixed too)

3rd is avs+ don't unload the loaded plugins after autoload like normal avs https://forum.doom9.org/showthread.php?p=1814912#post1814912 (this one not fixed yet)

and thanks

Groucho2004 commented 6 years ago

@realfinder The second one isn't an issue, it's just that you don't like the order ;-)

As for the third one - I'm too lazy to dig through the AVS+ code, how do you know that AVS+ doesn't unload/free the memory during auto-load enumeration?

realfinder commented 6 years ago

@Groucho2004

The second one can made some issues even if it not, well it should be clear that the avs+ plugins should loaded later to over load any old functions from normal avs plugins

and about the third, you can use some trick to test that, basically try overwrite or move some plugins that didn't used in encode script while you encode from that script, in avs you note that you can do anything to plugins that not used in the script, but in avs+ you can't and will get error said that files is used by some application

Groucho2004 commented 6 years ago

The second one can made some issues even if it not, well it should be clear that the avs+ plugins should loaded later to over load any old functions from normal avs plugins

That's not how it works. Read about the AVS+ autoload mechanism on avisynth.nl:

AviSynth+'s autoloader has a list of autoload directories. It iterates over all those directories and tries to load all plugins from each. But (and a big but!) it will not load a plugin from a directory if another plugin with the same basename is already loaded.

realfinder commented 6 years ago

but it did https://forum.doom9.org/showpost.php?p=1789735&postcount=2703

Groucho2004 commented 6 years ago

but it did https://forum.doom9.org/showpost.php?p=1789735&postcount=2703

OK, tested it myself and I can confirm that it loads from the "PluginDir2_5 " directory first.

pinterf commented 6 years ago

avsi autoload issue fixed in https://github.com/pinterf/AviSynthPlus/commit/f60c2b4f7d3f6d9b3b8b21cdac3dc1ae7917fa50

realfinder commented 5 years ago

@pinterf the "unload the loaded plugins after autoload like normal avs" is very important, especially for who using megui and has many plugins in main avs plugins folder, they will get many "Platform returned code 1114:A dynamic link library (DLL) initialization routine failed"

here what old avs do https://forum.doom9.org/showthread.php?p=1641891#post1641891

realfinder commented 4 years ago

in case someone want to see ScriptEnvironment::LoadPluginsMatching() in Avisynth.cpp

https://github.com/jeeb/avisynth/blob/00889df587c36fad1c6ba2c3df6baedf20e61ec6/src/core/avisynth.cpp#L1160

pinterf commented 4 years ago

I have a feeling that when unused plugins are unloaded it can conflict with ScriptClip.

Autoloading automatically occurs whenever a symbol is not found.

Variable? no. Internal function? no. Then autoload is triggered, becasue the function is perhaps in one of the DLLs somewhere.

For example: AvsMeter calls env->FunctionExist with "SetMtMode", this is its decision if the DLL is Avs 2.6mt or not - of course it does not exists in Avs+ but this check triggers a full autoload process again.

realfinder commented 4 years ago

but it don't has any problem with normal avs!

and what about the 2nd issue?

realfinder commented 4 years ago

I get a fix for 2nd one

changing

#ifdef AVS_WINDOWS
    plugin_manager->AddAutoloadDir("USER_PLUS_PLUGINS", false);
    plugin_manager->AddAutoloadDir("MACHINE_PLUS_PLUGINS", false);
    plugin_manager->AddAutoloadDir("USER_CLASSIC_PLUGINS", false);
    plugin_manager->AddAutoloadDir("MACHINE_CLASSIC_PLUGINS", false);

in avisynth.cpp to

#ifdef AVS_WINDOWS
    plugin_manager->AddAutoloadDir("MACHINE_CLASSIC_PLUGINS", false);
    plugin_manager->AddAutoloadDir("USER_CLASSIC_PLUGINS", false);
    plugin_manager->AddAutoloadDir("MACHINE_PLUS_PLUGINS", false);
    plugin_manager->AddAutoloadDir("USER_PLUS_PLUGINS", false);

so I think the wiki is wrong now, since pylorak/ultim rewrite Autoload things back then (before r1576) because there were some issues

now the only thing left is the 3rd one

pinterf commented 4 years ago

Does it have any reason that you changed not only user,machine to machine,user but the plus and classic order as well?

realfinder commented 4 years ago

yes, to make it what it should before the rewrite (before r1576)

old behavior was

old AviSynth+'s autoloader The expected use case is that you can now overlay a new plugin directory on top of another one. AviSynth+ then would load all plugins from the first folder, then load only those plugins from the second that weren't loaded from the first, then those from the third that weren't loaded from the first or second and so on. For example, let's say your usual plugin folder has a lot of plugins you normally use. But at one time you have a small number of updated plugins that you only want to use from a few scripts, but you do not yet want to replace your existing plugins globally. Then you'd just add a new plugin overlay folder, with only the new plugins in it, and that's it. All scripts that specify the new folder will autoload all plugins from your usual one, except for the new plugins, which would get loaded from the new folder. All your other scripts will still use your old plugins. By default, AviSynth+'s autoload folder list has four paths in it, in this order: PluginDir+ in Software/Avisynth in HKEY_CURRENT_USER PluginDir+ in Software/Avisynth in HKEY_LOCAL_MACHINE PluginDir2_5 in Software/Avisynth in HKEY_CURRENT_USER PluginDir2_5 in Software/Avisynth in HKEY_LOCAL_MACHINE This means, if there are ever plugins which will only work with AviSynth+ but not with classic AviSynth, you can put them into one of the "PluginDir+" folders. AviSynth+ will then use the classic plugins from the normal AviSynth, but if there are versions of some plugins written for AviSynth+, it will use them instead, and the classic avisynth.dll will still not be bothered with them. This is all without you having to lift a finger (except for adding the "PluginDir+" values to the registry once, until we have an installer). So to summarize all this, you have the ability to define a plugin autoload folder in the registry which will only be used by AviSynth+, but not by AviSynth, in addition to your classic plugins.

from http://avisynth.nl/index.php/AviSynth%2B#AviSynth.2B.27s_Plugin_Autoloader

since old avs+ behavior was reverse behavior of r1576 and up, from r1576 it start over load the loaded one just like the classic avs

so with the changes I made if some function that in both machine and user then the one from user will be used since it load after the machine, same for classic and plus, the plus will be used since it load after

and that what it should be done to get same old behavior with newer avs+

pinterf commented 4 years ago

Wiki may not be clear when puts an order as PluginDir+ in Software/Avisynth in HKEY_CURRENT_USER PluginDir+ in Software/Avisynth in HKEY_LOCAL_MACHINE PluginDir2_5 in Software/Avisynth in HKEY_CURRENT_USER PluginDir2_5 in Software/Avisynth in HKEY_LOCAL_MACHINE

Becasue it is good if the order there means the priority.

So that plugins loaded from HKCU+PluginDir+ override HKLM+PluginDir+ which override HKCU+PluginDir2_5 which override HKLM+PluginDir2_5 .

And yes, the real behaviour should work like that. For example mt_merge from a masktools2 in HKCU+PluginDir+ takes precedence over another mt_merge found HKCU+PluginDir2_5

realfinder commented 4 years ago

Wiki may not be clear when puts an order as PluginDir+ in Software/Avisynth in HKEY_CURRENT_USER PluginDir+ in Software/Avisynth in HKEY_LOCAL_MACHINE PluginDir2_5 in Software/Avisynth in HKEY_CURRENT_USER PluginDir2_5 in Software/Avisynth in HKEY_LOCAL_MACHINE

Becasue it is good if the order there means the priority.

well, it's copy paste from https://forum.doom9.org/showthread.php?p=1646315#post1646315

yes it mean priority, but this not how it work now since there were rewrite (well he said he rewrite it only in IRC but he only mention the bug fix in autoloaded functions in r1576 changes log)

and now, what about the 3rd issue :)

pinterf commented 4 years ago

I can easily multitask, but not over four tasks at a time :), pls be patient, cpp2.5, xp, and grunt are on my queue and they all affect the basic working, I'll need to insert and cleanup them properly in source and make understandable and separated - hopefully single - commits from them. Only then can I change other things or else the source gets messy. Patience, I'll return on it over the next week, probably.

pinterf commented 4 years ago

Well, checked the code.

'''plugin_manager->AddAutoloadDir("USER_PLUS_PLUGINS", false); plugin_manager->AddAutoloadDir("MACHINE_PLUS_PLUGINS", false); plugin_manager->AddAutoloadDir("USER_CLASSIC_PLUGINS", false); plugin_manager->AddAutoloadDir("MACHINE_CLASSIC_PLUGINS", false); ''' That false parameter is important, because it is specifying the order. Internally - in the AutoLoadDirs array - USER_PLUS_PLUGINS will be the first. If a dll is loaded from there it won't be overridden by a DLL found in one of the other folders. So masktools2.dll in Plugins+ will override masktools2.dll found in MACHINE_CLASSIC_PLUGINS. This is the for https://github.com/pinterf/AviSynthPlus/blob/master/avs_core/core/PluginManager.cpp#L577 and this is the actual decision to skip the reload if names are the same https://github.com/pinterf/AviSynthPlus/blob/master/avs_core/core/PluginManager.cpp#L613

pinterf commented 4 years ago

But for some reason it's not quite true. I have a 2.6 grunt.dll in the plugins+ folder and a 2.5 grunt in the classic plugins folder. And I have a test case where avs 2.5 is crashing, 2.6 is not. I expected that the 2.6 plugin will work but if both plugins are there, the crash occurs. So both plugins were loaded for some reason. Let's test it now.

pinterf commented 4 years ago

I got it. The reload is ignored because of a wrongly placed 'continue'. Let's fix it now

pinterf commented 4 years ago

And the same bug was allowing reload of .avsi scripts for a lower-priority plugin folder. They should be ignored as well since they would redefine and override functions.

pinterf commented 4 years ago

You can try a fixed build, see actual download link at https://forum.doom9.org/showthread.php?t=181351

realfinder commented 4 years ago

I did all the tests with mvtools2, since it show half green screen with yv16 in 2.5 version, I will try the test build soon

realfinder commented 4 years ago

yes, it work now! since you read the codes of the autoload things, I think the 3rd one will easy for you now