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.

Sign up for GitHub

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

Jump to bottom

libobs: Export more libobs/util headers #9238

Merged
merged 1 commit into from
Jul 15, 2023
Merged

libobs: Export more libobs/util headers #9238

merged 1 commit into from
Jul 15, 2023

Conversation

gxalpha
Copy link
Member

@gxalpha gxalpha commented Jul 12, 2023

Description

Adds additional headers from libobs/util that plugins might want to use to the public_headers list.

These are the headers in libobs/util that with this PR are not exported:

    util/array-serializer.h
    util/bitstream.h
    util/cf-lexer.h
    util/cf-parser.h
    util/crc32.h
    util/curl/curl-helper.h
    util/file-serializer.h
    util/lexer.h
    util/pipe.h
    util/serializer.h
    util/task.h
    util/utf8.h
    util/uthash.h
    util/util_uint128.h

If someone thinks some of them might be useful as well, I could add them.

Motivation and Context

Fixes obsproject/obs-plugintemplate#79
Fixes obsproject/obs-plugintemplate#80
(This relies on the next OBS release of course)

How Has This Been Tested?

Hasn't. Should be simple enough in theory.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

@gxalpha gxalpha added the Bug Fix Non-breaking change which fixes an issue label Jul 12, 2023
@gxalpha gxalpha requested a review from PatTheMav July 12, 2023 09:27
PatTheMav
PatTheMav approved these changes Jul 12, 2023
View reviewed changes
Copy link
Member

@PatTheMav PatTheMav left a comment

Choose a reason for hiding this comment

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

I really dislike that we effectively need to expose the entire "kitchen sink" of libobs to plugins, but alas that's how plugins work right now.

@gxalpha
Copy link
Member Author

gxalpha commented Jul 12, 2023

I wouldn't call it kitchen sink, the libobs/util directory mostly implements stuff that is needed in all kinds of scenarios (hence the "util" name), having plugins reimplement things like threading or dynamic strings if they already exist seems like busywork.

@PatTheMav
Copy link
Member

I wouldn't call it kitchen sink, the libobs/util directory mostly implements stuff that is needed in all kinds of scenarios (hence the "util" name), having plugins reimplement things like threading or dynamic strings if they already exist seems like busywork.

True, but my gripe is that when you link against libobs and include the obs-module.h header, you get a whole lot of dependencies thrown into your code, of which you might not even use the majority of (you also get the entirety of obs.h), with the linker hopefully stripping out everything you didn't actually implement.

I'd prefer if I could choose to either include a "give me everything" header (which in turn should not only be made of #include directives itself) or be able to pick what I need (e.g. #include <libobs-threading.h>) with the benefit of libobs itself cherrypicking its own dependencies.

But that's an issue for another day, I approved the PR already.

@gxalpha
Copy link
Member Author

gxalpha commented Jul 13, 2023

I'd prefer if I could choose to either include a "give me everything" header (which in turn should not only be made of #include directives itself) or be able to pick what I need (e.g. #include <libobs-threading.h>) with the benefit of libobs itself cherrypicking its own dependencies.

Yeah that’s fair

@Lain-B Lain-B merged commit 55237ab into obsproject:master Jul 15, 2023
@gxalpha gxalpha deleted the more-libobs-exports branch July 15, 2023 23:18
@royshil royshil mentioned this pull request Jul 25, 2023
Closed
@RytoEX RytoEX added this to the OBS Studio (Next Release) milestone Aug 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PatTheMav PatTheMav PatTheMav approved these changes

Assignees
No one assigned
Labels
Bug Fix Non-breaking change which fixes an issue
Projects
None yet
Milestone
OBS Studio 30.0
Development

Successfully merging this pull request may close these issues.

Build failed in MacOS/Linux CI Missing util/threading.h on mac and enum OBS_TEXT_INFO on ubuntu
4 participants
@gxalpha @PatTheMav @RytoEX @Lain-B