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

Externals don't work in plugin (only in standalone) #34

Open
FlachyJoe opened this issue Feb 14, 2022 · 14 comments
Open

Externals don't work in plugin (only in standalone) #34

FlachyJoe opened this issue Feb 14, 2022 · 14 comments
Labels
wontfix This will not be worked on

Comments

@FlachyJoe
Copy link

Hi,
I tried to add the purr-data external path to PlugData ( /opt/purr-data/lib/pd-l2ork/extra ) and C coded libraries fail to load.
Adding
[iemlib/init] : undefined symbol s_float
[iem_ambi/ambi_decode] : undefined symbol s_list
[lyonpotpourri/bv_play~] : undefined symbol garray_class

NB: These types are defined in Libraries/pure-data/src/m_pd.h

@timothyschoen
Copy link
Collaborator

Do these libraries load on vanilla pd? PlugData has the same core as pd-vanilla, it's possible that some externals for purr-data don't work because they use a different pd core.

@FlachyJoe
Copy link
Author

They load in vanilla.

@timothyschoen
Copy link
Collaborator

Then they should load in PlugData, I'll check it out!

@timothyschoen
Copy link
Collaborator

This problem is caused by the fact that the externals are compiled without the PDINSTANCE and PDTHREAD flag, while PlugData needs to be compiled with those flags. I need to compile with PDINSTANCE because we need to be able to use multiple instances to make it work as a plugin. Camomile suffers from the same issue, there's unfortunately not much I can do about it.

I'll leave the issue open, so people can see that this doesn't work yet.

@timothyschoen
Copy link
Collaborator

Externals now work in the standalone! I'll have to do some more testing but it's looking good.

For the plugin, support for externals is still a problem because we need to have multiple instance support.

PS: For M1 Mac users: remember to use PlugData in Rosetta if you want support for x64 externals.

@timothyschoen timothyschoen changed the title C coded externals fail to load Externals don't work in plugin (only in standalone) Apr 2, 2022
@timothyschoen timothyschoen reopened this Apr 2, 2022
@FlachyJoe
Copy link
Author

Hi,
I still have the error in Standalone :
Capture d’écran_2022-04-07_22-58-01

CMakeCache.txt

@timothyschoen
Copy link
Collaborator

Looks like I made a mistake in the cmakefile, I'll fix it right now.

@timothyschoen
Copy link
Collaborator

Screenshot 2022-04-07 at 23 20 48

Can reproduce that it doesn't work on linux

@timothyschoen
Copy link
Collaborator

Screenshot 2022-04-08 at 00 49 12

It works now! Turns out I needed to pass the -export-dynamic linker flag on Linux to get this kind of backwards linking to work.

@timothyschoen
Copy link
Collaborator

I don't know if you're building from source, but if you're not I'll create a release soon when I'm done with some other bug fixes!

@timothyschoen
Copy link
Collaborator

I see that windows has the same problem (unfortunately not with the same solution), so I'll look into that as well. My bad for assuming that it'd just work on all platforms, oops!

@FlachyJoe
Copy link
Author

I confirm it works on linux. Thank you !

@timothyschoen
Copy link
Collaborator

timothyschoen commented Jul 18, 2022

I may have found a solution...

If we define PD_THIS with a function, get_this() which is defined in an .c file instead of a header, we can make any new externals that are compiled work both with and without PDINSTANCE.

I still need to investigate this further, but it would mean that externals only need to be recompiled against the new header in order to work, without having to change any source code at all. If it works, I hope the pd devs will accept this change. It would also mean that Camomile could start supporting this (also by just recompiling!).

Because the PD_THIS external in existing externals already points to the &pd_maininstance symbol (which still exists), it shouldn't break anything.

EDIT: Okay, so I think the only problem would be that newly built externals would stop working in old pd versions. So maybe this is not a perfect solution after all... It's an interesting discussion. It might be possible to add some kind of fallback mechanism for this.

EDIT 2: I've managed to make externals built with new the PDINSTANCE system work in an unmodified pd-vanilla! Basically, you can just add a backup definition for these functions into your external, since dlopen won't overwrite these when it finds a duplicate. This can be done automatically in m_pd.h.

@alfonso73
Copy link

Edit 2 almost deserve a "wont fix" tag removal from the issue :-D
Great stuff tho! Finger crossed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

3 participants