obs-deps icon indicating copy to clipboard operation
obs-deps copied to clipboard

Add HTTP/2 and Brotli support

Open derrod opened this issue 1 year ago • 7 comments

Description

Adds brotli and nghttp2 libraries to curl for HTTP/2 and brotli compression support. Also switches curl to being a static library.

Motivation and Context

Currently all HTTP traffic OBS receives is uncompressed, which is wasting bandwidth. Additionally, we want to use HTTP/2 for multiplexing and asnychronous requests in the updater and possibly other parts of OBS.

How Has This Been Tested?

Built locally, but needs to be checked more thouroughly as updater changes are still in progress and we might need to build curl with a static VCRT for it (and thus probably twice in total), so the PR is a draft.

Types of changes

  • New feature (non-breaking change which adds functionality)
  • Tweak (non-breaking change to improve existing functionality)

Checklist:

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

derrod avatar May 22 '23 12:05 derrod

Also switches curl to being a static library.

Unfortunately, OBS Studio is not the only binary relying on curl, some plugins do. So building statically is not a good thing, it will introduce duplication of the curl lib in various binaries.

tytan652 avatar May 22 '23 14:05 tytan652

Also switches curl to being a static library.

Unfortunately, OBS Studio is not the only binary relying on curl, some plugins do. So building statically is not a good thing, it will introduce duplication of the curl lib in various binaries.

Please be specific. What plugins rely on curl?

RytoEX avatar May 22 '23 14:05 RytoEX

What plugins rely on curl?

win-capture and rtmp-services, both use it through the file-updater.

tytan652 avatar May 22 '23 14:05 tytan652

Unfortunately, OBS Studio is not the only binary relying on curl, some plugins do. So building statically is not a good thing, it will introduce duplication of the curl lib in various binaries.

Honestly, not worth caring about. It's a few kilobytes or maybe a megabyte. But it's either that or compiling Curl and it's dependencies twice.

derrod avatar May 22 '23 16:05 derrod

Just to leave some notes on where this PR is at:

  • libcurl's cmake is annoying because it exports with absolute paths to dependent libraries (nghttp2/brotli)
    • Probably requires some patch to their cmake to fix
    • We'd probably have to write finders as well for use in OBS
  • Does compile and HTTP/2 works in OBS with minimal changes if you manually fix linking
  • static+shared is still annoying, libcurl itself would probably support both as separate targets but its dependencies might not, haven't investigated

derrod avatar Dec 11 '23 21:12 derrod

Just to leave some notes on where this PR is at: * libcurl's cmake is annoying because it exports with absolute paths to dependent libraries (nghttp2/brotli) * Probably requires some patch to their cmake to fix * We'd probably have to write finders as well for use in OBS * Does compile and HTTP/2 works in OBS with minimal changes if you manually fix linking * static+shared is still annoying, libcurl itself would probably support both as separate targets but its dependencies might not, haven't investigated

On libcurl, I would prefer that if it needs changes, that those changes are done on the curl repo. They release pretty frequently these days.

RytoEX avatar Dec 12 '23 21:12 RytoEX

Based on some discussions off-thread we can probably use the dynamic CRT instead of static even for the updater, which makes things a little easier (see my standalone updater repo).

I don't even know where to start with the cmake changes or if upstream would even care about them.

derrod avatar Dec 13 '23 18:12 derrod