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

Enable building with CEF 6261 & 6367 (Chromium 122 & 124) #434

Merged
merged 3 commits into from
Jul 30, 2024

Conversation

WizardCM
Copy link
Member

@WizardCM WizardCM commented Feb 4, 2024

Description

This allows compiling OBS Browser with Chromium 124-based CEF versions.

This also adds basic support for the new, official, shared textures API on both Windows and macOS.

Sources:

Fun note; Remote debugging now requires --remote-allow-origins=* otherwise Remote Debugging immediately disconnects - even for localhost.

Motivation and Context

At some point we'd like to upgrade to modern CEF. To do that, obs-browser needs to compile.

How Has This Been Tested?

Launch OBS with a CEF 6261 or 6367 build. Have not tested downgrading CEF to our currently supported versions.

Only tested on Windows.

Types of changes

  • Tweak (non-breaking change to improve existing functionality)

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.

@WizardCM WizardCM added the Enhancement New feature or improvement label Feb 4, 2024
jmickelonis added a commit to jmickelonis/obs-browser that referenced this pull request Feb 7, 2024
@jmickelonis
Copy link

jmickelonis commented Feb 9, 2024

Slightly off-topic, but you might be interested to know.
I built with 6261 (beta) on Ubuntu 23.10. Browser sources and docks do show up, but calls to ExecuteJavascript fail silently without effect. 6167 stable works fine.

@PatTheMav
Copy link
Member

@WizardCM do you want to include the texture flip override for builds above 6261 in this PR as well (as discussed off-thread)?

As the texture shared with us is originally created for video encoding, it's pre-flipped already.

@RytoEX RytoEX mentioned this pull request Apr 2, 2024
6 tasks
@RytoEX RytoEX requested a review from PatTheMav April 2, 2024 22:53
@RytoEX RytoEX self-assigned this Apr 2, 2024
@RytoEX
Copy link
Member

RytoEX commented Apr 3, 2024

@WizardCM do you want to include the texture flip override for builds above 6261 in this PR as well (as discussed off-thread)?

As the texture shared with us is originally created for video encoding, it's pre-flipped already.

Gentle ping, @WizardCM ?

@WizardCM WizardCM changed the title Enable building with CEF 6261 (Chromium 122) Enable building with CEF 6261 & 6367 (Chromium 122 & 124) Apr 28, 2024
@WizardCM
Copy link
Member Author

@RytoEX @PatTheMav Per prior discussion, this PR now makes the necessary adjustments (including flip) to correctly run shared textures with CEF 6367. I have tested it on Windows 10 with the 6367 CEF build only. I've also kept the two commits separate for easier review, and they can realistically be combined for final merge.

browser-client.cpp Outdated Show resolved Hide resolved
browser-client.cpp Show resolved Hide resolved
At some point between 5563 and 5938, the parameter's type changed.
This allows compiling OBS Browser with Chromium 124-based CEF versions.

This also enables basic support for the new, official, shared texture
API, purely because building without it would be *more* difficult.

Sources:
 - https://bitbucket.org/chromiumembedded/cef/commits/260dd0ca2
@WizardCM
Copy link
Member Author

WizardCM commented Jun 9, 2024

Alright, commits have been updated. Tested on macOS, and OnScheduleMessagePumpWork changed somewhere between 5563 and 5938, so our code handles that case too. Tested on Linux, and no additional changes were needed on our side (obviously without shared textures).

This now builds and runs on Windows, macOS and Linux without issue, and the first 2 with shared textures. In my opinion this seems good to merge.

For now, I've left the code to set bs->last_handle but it's not read. I'll let others confirm what we want to do there.

@reitowo
Copy link

reitowo commented Jun 9, 2024

Great work!

I forgot why, but is there a chance to import gbm in Linux? Seems OBS is the easiest user to make this work because it has a handy render pipeline.

@jpietek
Copy link

jpietek commented Jun 18, 2024

Great work!

I forgot why, but is there a chance to import gbm in Linux? Seems OBS is the easiest user to make this work because it has a handy render pipeline.

I tried once (obs discord long discussion), but failed on Nvidia (OK with Intel and AMD) with some old fork of yours. Would it makes sense to try again with 126 chromium? I see the api has changed not void* but some structure with the handle now.

@RytoEX
Copy link
Member

RytoEX commented Jun 18, 2024

There are still be open feedback threads. They should be resolved before this can be merged.

Copy link
Member

@RytoEX RytoEX left a comment

Choose a reason for hiding this comment

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

The CEF API changes look good to me. I haven't done a local test build of this yet.

Copy link
Member

@RytoEX RytoEX left a comment

Choose a reason for hiding this comment

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

I can confirm that I can build OBS Studio on Windows against both CEF 103 (5060) and 126 (6478) with these changes.

@RytoEX RytoEX merged commit 879a9d2 into obsproject:master Jul 30, 2024
1 check passed
@WizardCM WizardCM deleted the cef-upgrade-122 branch August 10, 2024 03:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants