-
Notifications
You must be signed in to change notification settings - Fork 4
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
Bad plugin class filtering #9
Comments
Hi. Most of the plugins you've mentioned use subclasses of the supported types. Looks like the only not supported ones are few descendants of the UtilityPlugin. But most should work and do work for me: auto cls_uri = lilv_plugin_class_get_uri(cls);
auto parent_cls_uri = lilv_plugin_class_get_parent_uri(cls);
for (auto const& supported_cls : supported_classes) {
if (lilv_node_equals(cls_uri, supported_cls) ||
(parent_cls_uri != NULL && lilv_node_equals(parent_cls_uri, supported_cls))) {
skip = false;
}
} Not sure why those are getting filtered out for you. I've added some extra logging to print types of the discarded plugins if you want to give that a shot :-) I agree that having a filtering like this is far from ideal. It's a rather naïve attempt on listing only plugins that make sense in the context of OBS. It should be replaced with something that just checks if plugin has workable set of inputs and outputs. |
OK, thanks. I'll check a bit later and give additional information. |
Yes, the extra logging is now in |
Also, one more idea: is it really so needed to strictly require from plugin to support HardRT? |
Here's the output:
Hmmm... It sees any plugin class as instance of
|
That can be an older
|
Interesting. According to NEWS 0.24.10 was released 27 Sep 2020 and I'm pretty it has worked ever since I've implemented the filtering in May of 2020 as I've been using some LSP plugins back then (thanks!) Maybe that particular release has this bugged? FWIW I'm on lilv 0.24.18 and lv2 1.18.8 and I have 1.2.2 version of LSP installed and the classes resolve correctly for me. |
I'm not opposed to having a way for lifting that requirement. Should be trivial to add the checkbox. I don't know much about audio, Hard RT made sense to me because we are working in livestreaming environment and applying filters on the fly to live audio. From what I understand if plugin is not hard-rt-capable it's meant for "offline" use, i.e. when processing audio is detached from playback and can willy-nilly cause underruns. If that's not true I'm happy to be corrected though :-) |
HardRT compatible means that plugin does not perform any sleeps/memory allocations or system calls in the
I don't know. Maybe @drobilla can clarify something to us. |
Host support for that stuff is a bit sketchy since everything works if you just ignore it, but hardRTCapable is about the real-time requirements @sadko4u mentioned (no blocking, basically). It is not about "offline" type things at all (that's isLive). |
To the original topic, nothing says that plugins even have to use the standard classes, and new standard classes may be added at any time. So, hosts allow-listing plugin classes strikes me as a very bad idea. If you want to keep the spam of weird utility plugins (or whatever) down, I recommend inverting this and instead (optionally) hiding classes you know you don't want to show up. |
@drobilla and what about the invalid detection of plugin class like |
No idea. I'm traveling at the moment and don't have the time to dig in to the code of another project right now (where does It's possible a regression has snuck in at some point, but nothing has changed about this code in quite a long while and everything else seems to still work fine 🤷 Ulitmately, though, this is a "don't do that" situation, this:
is the right way, and generally how hosts hide plugins: it hides just the ones that literally won't work at all. Anything beyond that based on plugin class is just to fuzzy to rely on to outright hide plugins from the user. We could so some work on the spec and tools like |
8fd3232 should help the situation. It's still fairly naïve - we make sure that there's enough input and output audio ports for the number of channels the OBS is set up to use. Adding a patch bay would be nice, especially since OBS allows us to access other sources so we could use them for sidechaining. But that's out of the scope for now, there are more pressing issues :-) PS. I've also removed the hard RTC requirement: 86321f4. |
At this moment while discovering the issue lsp-plugins/lsp-plugins#256 I see the following messages in the log:
According to the code, I see that plugins are filtered by the plugin class:
This is VERY small subset of plugins that actually can be supported by the host.
At least, following additional plugin types are not supported but looks like should be:
Anyway, I find the filtering of the strict list a bad practice that should be avoided. Instead of this, it is much better to specify plugin class selector as an additional combo or checkbox list.
So I would like to suggest to change the way the plugin list is formed.
The text was updated successfully, but these errors were encountered: