-
Notifications
You must be signed in to change notification settings - Fork 227
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
Add full support for CEF 4638 #323
Conversation
b41fd92
to
adf42ac
Compare
adf42ac
to
a499db2
Compare
a499db2
to
d9b1b53
Compare
d9b1b53
to
ba148fc
Compare
Added |
After discussion with the team, we came to the following conclusion:
As the latter point mirrors behaviour of current browsers, we deem it a "won't fix" coupled with updated user guidance once we update. |
d0f3d3e
to
cdf36fe
Compare
Issue A: Building on Ubuntu 20.04 with 4280 fails, with a few errors (See Discord) Issue B: Every instance of a browser source spams this message to the log, and there is no video: The more browsers that are added, the more spam there is. The browser can be removed, stopping the spam, but it starts up again when you go to close OBS, and requires killing. OBS functions as expected when no browser stuff is ever loaded. |
That's weird given that most changes are guarded by a version check for newer CEF versions only, the changes that affect every version is proper use of |
@PatTheMav I can try using a fresh dev environment in the morning, just to make sure that there isn't some random issue. |
Fresh ENV - Building with 4280 still errors, and I also discovered that I was only able to build with the spotify 4638 CEF because of previous cmake cache. I need a download of 4638 with the correct CEF wrapper. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've submitted a bugfix for the reported issue on older CEF.
Co-authored-by: Matt Gajownik <matt@wizardcm.com>
Using the latest version: CEF 4280 builds and works as expected CEF 4638 builds and works as expected, with the bonus that it doesn't throw any threadrunner errors. Big thing to note: At least on Ubuntu 20.04, in order to use 4638 you need to use a fresh build environment. For example, having a 4280-based environment then reconfiguring with cmake will cause tons of wacky errors on runtime, and the browser source will simply not work. Perhaps there is some way to detect an old version of cef in cmakelists, to throw an explicit configuration error if the version changes? |
Remaining things to test/fix:
|
From what I could tell that is by design - The intended way is to delete the cache variables associated with CEF (except for the root directory) and re-run CMake again (so As for a way to fix this: CEF would have to produce a proper CMake package with version and target configuration - then you could do |
On Wed, 2021-11-24 at 04:35 -0800, Patrick Heyer wrote:
The intended way is to delete the cache
Would you be able to 'remind' (:D) me on the correct or most efficient
way to do that?
Many thanks!
|
Either run Or edit |
This seems to fix AJAX calls to local files in local file mode, but may break large media files. diff --git a/browser-scheme.hpp b/browser-scheme.hpp
index 62b4d75e..dd28614f 100644
--- a/browser-scheme.hpp
+++ b/browser-scheme.hpp
@@ -22,7 +22,7 @@
#include <string>
#include <fstream>
-#if CHROME_VERSION_BUILD >= 3440
+#if CHROME_VERSION_BUILD >= 3440 && CHROME_VERSION_BUILD < 4638
#define ENABLE_LOCAL_FILE_URL_SCHEME 1
#else
#define ENABLE_LOCAL_FILE_URL_SCHEME 0
Edit: Verified this breaks large media files. But only sometimes? |
Large media file playback seems fine to me, I checked local CORS with your test suite, both XHR and Fetch seem to work fine. Pointing to a non-existing Loading remote will still be blocked by every host that doesn't use Edit: Added a fix for the missing file issue - how did you recreate the large media file issue? I managed to watch an entire TV episode via browser source without issue just now. |
c8f3434
to
d20e95c
Compare
For some reason, the diff below fixes the browser panel resize issue on Linux. If we end up using this fix temporarily, I recommend we scope it to 4638. diff --git a/panel/browser-panel.cpp b/panel/browser-panel.cpp
index e56565b29..55af1a1bf 100644
--- a/panel/browser-panel.cpp
+++ b/panel/browser-panel.cpp
@@ -427,6 +427,8 @@ void QCefWidgetInternal::Resize()
changes.height = size.height();
XConfigureWindow(xDisplay, (Window)handle,
CWX | CWY | CWHeight | CWWidth, &changes);
+ XSync(xDisplay, false);
#endif
}); This is probably not the best solution, but it does work. |
4855804
to
865113a
Compare
Confirmed the fix, added as additional commit. |
Description
Introduces necessary fixes for compatibility with CEF 4638 due to API changes within CEF.
Note: This PR stays a draft for now as we need a proper solution to the upstream changes regarding web security disabling for local file support.
Motivation and Context
Make obs-browser compatible with CEF 4638 which is required for M1 support.
How Has This Been Tested?
Types of changes
Checklist: