-
Notifications
You must be signed in to change notification settings - Fork 230
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 Linux browser panel support [CEF 4280 / Chromium 87] #254
Conversation
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.
Just a few notes based on a visual readthrough. I imagine between this and #197, one will need rebased when the other is merged.
214af24
to
df2b745
Compare
All requested fixes in the review are now fixed. |
df2b745
to
189083e
Compare
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.
A couple more things.
Ubuntu 20.04:
When I create a custom browser dock.
When I try to open the Service Integration window (specifically to connect Twitch). Browser sources work fine. |
OK, after a bit of debugging myself, the cause for the second error above is obs-browser/panel/browser-panel.cpp Line 99 in ce8f6f8
Essentially, obs-browser/panel/browser-panel.cpp Lines 82 to 83 in ce8f6f8
I think this is because the value it's based on, |
206952e
to
89fefca
Compare
Service integration should be working with the latest push. |
I've spent a few hours trying to bypass this error. No luck. If I remove the I tried a number of variations, including using The only way I was able to change the behaviour is using I don't know X well enough to be able to do this using X functions correctly. |
89fefca
to
1531b67
Compare
@WizardCM could you check if it works better for you now? I removed all of the QT functions from the CEF threads, and it seems it is more stable. It also seemed to work better on the latest CEF (CEF 4280). |
1531b67
to
3755c2e
Compare
@cg2121 Nice work! I can confirm that they now correctly appear inside the dock (and within the Service Integration login container). As a side note, I'm using CEF 4183 as that's what we hope to upgrade to. However, while the child windows are correctly added, the following things don't work:
|
@WizardCM the crash on exit appears to only happen on CEF 4183, but resizing and clicking work fine for me. |
@cg2121 Hmm, in that case I might try bumping to 4240 or 4280 for my testing. I doubt that'll fix resizing and clicking though, if my experience so far is anything to go by. :( |
@cg2121 I can confirm that 4240 doesn't have any change in behaviour at all, but 4280 fixes the crash on shutdown and the two BadWindow errors when closing service integration login. The rest of the issues remain. |
What OS did you test on so far? Can you please add that information to the original post? |
I've tested it so far on Ubuntu 20.04 with QT 5.12.8. |
On Fri, 2020-12-11 at 11:44 -0800, Clayton Groeneveld wrote:
I've tested it so far on Ubuntu 20.04 with QT 5.12.8.
I've been testing on Debian Unstable
- QT 5.15
- cef_binary_87.1.11+g8bb7705+chromium-87.0.4280.66_linux64
- obs-studio HEAD
- obs-browser remotes/lbp/linux-browser-panels 24c371a
So far everything I have thrown at it has worked.
- enable gpu works brilliantly.
- docks work
- browser windows work perfect - no errors or crashes so far
- ninja and other GPU intensives work really well with --enable-gpu
For my builds I'm using this to make starting with Accell easy.
… commit 3cdddbb06910ffa6705041340b80c8f556114d7c
Author: Paul Hedderly ***@***.***>
Date: Mon Dec 7 22:10:03 2020 +0000
prh: change desktop file for accell
diff --git a/UI/xdg-data/com.obsproject.Studio.desktop b/UI/xdg-
data/com.obsproject.Studio.desktop
index 4b753a4f5..cdf466e95 100644
--- a/UI/xdg-data/com.obsproject.Studio.desktop
+++ b/UI/xdg-data/com.obsproject.Studio.desktop
@@ -6,7 +6,7 @@ GenericName[fr]=Logiciel d'enregistrement/diffusion
Comment=Free and Open Source Streaming/Recording Software
Comment[fr]=Logiciel libre d'enregistrement et de diffusion sur
Internet
Comment[ru]=Бесплатная программа с открытым кодом для
записи/трансляции видео
-Exec=obs
+Exec=obs --enable-gpu --enable-media-stream
Icon=com.obsproject.Studio
Terminal=false
Type=Application
|
@WizardCM by chance, are you using a VM to test it? |
3755c2e
to
02596dd
Compare
02596dd
to
cd548ea
Compare
Correct, I'm using my PXE boot in a Hyper-V VM. For some reason it has had trouble booting on real hardware recently, I hope to troubleshoot that this weekend. |
98cc676
to
3bb336a
Compare
The obs-browser library lookup is now un-hardcoded. Requires obsproject/obs-studio#3871 to work. |
I just built this code on Ubuntu 20.04:
|
3bb336a
to
918d71b
Compare
I've fixed the Windows resize issue. All I had to do was to set the window container as frameless. |
I can confirm that the lib lookup function works perfectly, even with nonstandard locations for OBS. Heads up: the issues I was running into with resizing and interaction are caused by the
@eNBeWe This is why we use 3809 on Linux. Basically, 3770 doesn't seem to set the default font size, so all text is tiny (everything else should be fine). Alternatively, add this commit.
This is likely because non-free codecs are not enabled in Spotify builds. |
With the newer CEF versions (tested with 4280), the USE_QT_LOOP on Linux doesn't seem necessary. From what I can tell, the crashes don't happen anymore with newer CEF versions, as I haven't been able to reproduce them. |
If that turns out to be the case, then this PR is ready to go. |
Confirmed with another developer that they aren't getting the crashes anymore either, so as is this can be merged without problems, at least for Linux. |
@tbodt added a fix for this in #197, so maybe adapt that here as well? I guess merging #197 first makes sense, then the PRs for "modern" CEF versions required for macOS (because that would include CEF 4280 as well), and finally this PR? |
918d71b
to
e3695cd
Compare
I've added the fix from #197. It also works on Linux without problems. |
Thank for working on this! I've been testing on Ubuntu 20.04 with OBS 26.0.1 with the following patches applied:
I've also dropped |
Tested the Snap package mentioned to me by @flexiondotorg ( https://snapcraft.io/obs-studio ), looking good over here on Arch Linux. Docked my Twitch chat window without issues. |
@cg2121 Now that macOS panel support has been merged, please rebase your branch. In the meantime, I'm going to try and get |
e3695cd
to
aac73e5
Compare
It has been updated to the latest master. |
I managed to get this going, but when I tried to embed https://pretzel.rocks/ the player seems to be unable to function - it errors out playing each song and immediately skips to the next song. |
aac73e5
to
e133c60
Compare
This adds browser panel and service integration support to Linux.
e133c60
to
3cd1a98
Compare
TODO
Description
Add Linux browser panel support.
Motivation and Context
Bringing the browser plugin to parity with Windows.
How Has This Been Tested?
Needs testing on other OSs to make sure it doesn't break anything.
Types of changes
Checklist: