Skip to content

Conversation

@JustinTArthur
Copy link
Contributor

Keeping these hidden can cause the VapourSynth plugin to try to catch a made-up version of the exception classes instead of the ones actually used by libbestsource. See https://gcc.gnu.org/wiki/Visibility#Problems_with_C.2B-.2B-_exceptions_.28please_read.21.29

Fixes #114.

meson.build Outdated
)

libs = []
api_args = ['-DBSSHARED_API_BUILDING']
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be behind a check for default_library != 'static'? Though I guess even that wouldn't cover all cases.
As far as I can see, the relevant cases are:

  • We're building a libbestsource.dll (vapoursynth plugin or not): We want to export the exception types.
  • We're building another executable or library that will dynamically link bestsource: We want to import the exception types.

The above two cases are covered by this code. However:

  • We're building a dynamic library that statically links bestsource: Here it depends on how that library calls bestsource. If it catches all of its exceptions and rethrows none of them, then the exception types don't need to be exported (though it probably doesn't hurt to export them either). If it doesn't catch all of its exception or rethrows some of them, the exception types do need to be reexported. But either way, only the exception types should be reexported, and none of bestsource's other API functions.

So, I think this current logic works, but only if BSSHARED_API_EXPORT is only applied to the exceptions and not to any of bestsource's other API functions. (Which I guess results in this comment having net zero information, but I wanted to bring this up for posterity and/or see if I missed something)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I only export the exceptions. In case it's important, I've gone ahead and added a gate against visibility in a static build.

@myrsloik
Copy link
Member

myrsloik commented Dec 17, 2025

I never even intended for this to be compiled as a separate library and only imagined people dropping in the files when writing it.

No idea if this is the best solution since I rarely poke this kind of C++ details

@HolyWu
Copy link
Collaborator

HolyWu commented Dec 18, 2025

I have reunited them as a single library now.

@JustinTArthur
Copy link
Contributor Author

The switch to monolithic plugin libs in a4c240b resolves the bug this PR was intended to fix. Closing.

@JustinTArthur JustinTArthur deleted the lib-exception-classes-visible branch December 18, 2025 17:48
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

Successfully merging this pull request may close these issues.

Many exceptions don't get injected into Python layer when built with shared libs

4 participants