Skip to content

Conversation

@Slkoshka
Copy link
Contributor

@Slkoshka Slkoshka commented Apr 8, 2023

I was trying to investigate YorVeX/ObsCSharpExample#2 and turns out ObsBase.blog and ObsBase.blogva do not work on Linux and it is not possible to make them work in their current form.

The current way to use ObsBase.blogva with a null pointer is undefined behavior (I did not check the standard, but I am pretty sure we cannot pass a null pointer instead of C struct, the way C structs are passed to functions definitely depends on the ABI), and we are lucky that it worked at all on Windows.

The alternative is to use ObsBase.blog with an undocumented __arglist keyword. The issue is that it is not currently implemented in .NET on non-Windows platforms. And while it may be possible to emulate it in C# (although, I am not sure about this), it is ABI-dependent (so, we will have to make a different implementation for each platform) and the way it works on x64 is complicated, to say the least.

Unfortunately, the pull request in ClangSharp that added support for variadic arguments does not mention that it is Windows-only and does not produce any warnings about its usages. I did not check if ClangSharp's repo has an open issue for this yet, I will probably do it later and open a new one if I do not find any.

The change I propose in this pull request is to get rid of the automatically generated bindings for these functions and to make a new manual binding for ObsBase.blog that does not use variadic arguments and has only one hard-coded argument (which is an ABI-compatible way to call a C function with variadic arguments).

I tested the change on both Linux and Windows and I did not find any issues, plugins (I tested the sample plugin in this repository and YorVeX/ObsCSharpExample) now work on Linux and they continue working on Windows.

This is a breaking change!

@YorVeX
Copy link
Contributor

YorVeX commented Apr 8, 2023

Wow, I was so much hoping for someone to figure this out, thanks for taking the time to look into this @Slkoshka !

@kostya9
Copy link
Owner

kostya9 commented Apr 8, 2023

Thanks for the contribution and awesome job investigating this!

Could you please bump the version NetObsBindings.csproj to 0.0.1.33-alpha (so that auto-publish kicks in when we merge this) and add a little comment to the manual blog function mentioning that it is implemented manually because the generated versions do not work on non-Windows platforms(so that maybe we could revise that later when there will be more capabilities in .NET/ClangSharp)?

@Slkoshka
Copy link
Contributor Author

Slkoshka commented Apr 8, 2023

@kostya9
Done and done.

@kostya9 kostya9 merged commit 631a6e6 into kostya9:main Apr 8, 2023
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.

3 participants