gh-obsproject/obs-browser
Watch
1
0
Fork
You've already forked obs-browser
 0
Code Issues 37 Pull requests 13 Releases 15 Wiki Activity

Add Linux browser panel support [CEF 4280 / Chromium 87] #254

Merged
cg2121 merged 2 commits from linux-browser-panels into master 2021-01-26 16:39:12 -06:00
Conversation 43 Commits 2 Files changed 4 +90 -41
cg2121 commented 2020-12-08 15:11:53 -06:00 (Migrated from github.com)

TODO

  • Needs more testing (tested on Ubuntu 20.04 with QT 5.12.8)
  • Un-hardcode obs-browser.so lookup
  • Fix random crashes that sometimes happen

Description

Add Linux browser panel support.

Screenshot from 2020-12-08 14-39-41

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

  • New feature (non-breaking change which adds 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.
### TODO - [x] Needs more testing (tested on Ubuntu 20.04 with QT 5.12.8) - [x] Un-hardcode obs-browser.so lookup - [x] Fix random crashes that sometimes happen ### Description Add Linux browser panel support. ![Screenshot from 2020-12-08 14-39-41](https://user-images.githubusercontent.com/19962531/101541058-76f2c080-3966-11eb-845f-a946f41ae851.png) ### 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 - New feature (non-breaking change which adds functionality) ### Checklist: - [x] My code has been run through [clang-format](https://github.com/obsproject/obs-studio/blob/master/.clang-format). - [x] I have read the [**contributing** document](https://github.com/obsproject/obs-studio/blob/master/CONTRIBUTING.rst). - [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.
👍 9 ❤️ 9 🎉 3 🚀 1
RytoEX (Migrated from github.com) requested changes 2020-12-08 15:25:35 -06:00
RytoEX (Migrated from github.com) left a comment

Just a few notes based on a visual readthrough. I imagine between this and #197, one will need rebased when the other is merged.

Just a few notes based on a visual readthrough. I imagine between this and #197, one will need rebased when the other is merged.
CMakeLists.txt
RytoEX (Migrated from github.com) commented 2020-12-08 15:20:19 -06:00

Might as well change the comment to reflect this isn't just WIN32 anymore.

Might as well change the comment to reflect this isn't just WIN32 anymore.
panel/browser-panel.cpp
RytoEX (Migrated from github.com) commented 2020-12-08 15:21:14 -06:00

Shouldn't this be #if !defined(_WIN32) && !defined(__APPLE__)?
Note that you use define instead of defined for the second condition.

Shouldn't this be `#if !defined(_WIN32) && !defined(__APPLE__)`? Note that you use `define` instead of `defined` for the second condition.
panel/browser-panel.cpp
@ -238,4 +253,4 @@
/* Make sure Init isn't called more than once. */
if (cefBrowser)
return;
RytoEX (Migrated from github.com) commented 2020-12-08 15:22:11 -06:00

Probably don't need this whitespace addition, unless the visual separation is truly needed.

Probably don't need this whitespace addition, unless the visual separation is truly needed.
panel/browser-panel.cpp
@ -273,1 +278,4 @@
timer.stop();
if (!container) {
container =
RytoEX (Migrated from github.com) commented 2020-12-08 15:23:35 -06:00

I think in C++ files we use camelCase for variables. I'd probably use browserWindow for this.

I think in C++ files we use camelCase for variables. I'd probably use `browserWindow` for this.
cg2121 commented 2020-12-08 15:36:59 -06:00 (Migrated from github.com)

All requested fixes in the review are now fixed.

All requested fixes in the review are now fixed.
RytoEX (Migrated from github.com) requested changes 2020-12-08 15:59:27 -06:00
RytoEX (Migrated from github.com) left a comment

A couple more things.

A couple more things.
panel/browser-panel.cpp
@ -287,3 +300,3 @@
#elif
#else
QSize size = this->size() * devicePixelRatio();
#endif
RytoEX (Migrated from github.com) commented 2020-12-08 15:57:19 -06:00

What is the difference between RECT and CefRect? Is this a platform specific difference?

What is the difference between RECT and CefRect? Is this a platform specific difference?
panel/browser-panel.cpp
RytoEX (Migrated from github.com) commented 2020-12-08 15:57:27 -06:00

If you're removing the #ifdef _WIN32 here for SUPPORTS_FRACTIONAL_SCALING, doesn't that mean the instance on line 233/235 should also be moved/removed?

If you're removing the `#ifdef _WIN32` here for `SUPPORTS_FRACTIONAL_SCALING`, doesn't that mean the instance on line 233/235 should also be moved/removed?
WizardCM commented 2020-12-08 21:20:50 -06:00 (Migrated from github.com)

Ubuntu 20.04:

error: X Error: BadWindow (invalid Window parameter), Major opcode: XI_BadMode (invalid Mode parameter), Minor opcode: 46, Serial: 250

When I create a custom browser dock.

terminate called after throwing an instance of 'std::logic_error'
what(): basic_string::_M_construct null not valid
Aborted (core dumped)

When I try to open the Service Integration window (specifically to connect Twitch).

Browser sources work fine.

Ubuntu 20.04: > error: X Error: BadWindow (invalid Window parameter), Major opcode: XI_BadMode (invalid Mode parameter), Minor opcode: 46, Serial: 250 When I create a custom browser dock. > terminate called after throwing an instance of 'std::logic_error' what(): basic_string::_M_construct null not valid Aborted (core dumped) When I try to open the Service Integration window (specifically to connect Twitch). Browser sources work fine.
WizardCM commented 2020-12-09 01:42:05 -06:00 (Migrated from github.com)

OK, after a bit of debugging myself, the cause for the second error above is

github.com/obsproject/obs-browser@ce8f6f8269/panel/browser-panel.cpp (L99)

Essentially, path.Get() returns null.

github.com/obsproject/obs-browser@ce8f6f8269/panel/browser-panel.cpp (L82-L83)

I think this is because the value it's based on, rpath, ends up being an absolute path rather than an expected relative path, so the conversion fails.

OK, after a bit of debugging myself, the cause for the second error above is https://github.com/obsproject/obs-browser/blob/ce8f6f8269275c4fd672ef8ad25225a6b9418b78/panel/browser-panel.cpp#L99 Essentially, `path.Get()` returns `null`. https://github.com/obsproject/obs-browser/blob/ce8f6f8269275c4fd672ef8ad25225a6b9418b78/panel/browser-panel.cpp#L82-L83 I *think* this is because the value it's based on, `rpath`, ends up being an absolute path rather than an expected relative path, so the conversion fails.
WizardCM (Migrated from github.com) reviewed 2020-12-09 01:44:46 -06:00
panel/browser-panel.hpp
WizardCM (Migrated from github.com) commented 2020-12-09 01:44:45 -06:00

Would @tbodt's solution on macOS work here too?

https://github.com/obsproject/obs-browser/pull/197/files#diff-69dd8c025b947c895c80b4705d56a61d880c8841834614a0643b6cb727e4cd19R98-R99

Would @tbodt's solution on macOS work here too? https://github.com/obsproject/obs-browser/pull/197/files#diff-69dd8c025b947c895c80b4705d56a61d880c8841834614a0643b6cb727e4cd19R98-R99
WizardCM (Migrated from github.com) reviewed 2020-12-09 02:07:28 -06:00
panel/browser-panel.cpp
@ -287,3 +300,3 @@
#elif
#else
QSize size = this->size() * devicePixelRatio();
#endif
WizardCM (Migrated from github.com) commented 2020-12-09 02:07:28 -06:00

My guess based on the documentation is that CefRect is a platform-agnostic method.

https://magpcss.org/ceforum/apidocs3/projects/(default)/CefWindowInfo.html#SetAsChild(cef_window_handle_t,RECT)

My guess based on the documentation is that CefRect is a platform-agnostic method. https://magpcss.org/ceforum/apidocs3/projects/(default)/CefWindowInfo.html#SetAsChild(cef_window_handle_t,RECT)
cg2121 (Migrated from github.com) reviewed 2020-12-09 17:00:06 -06:00
panel/browser-panel.cpp
@ -287,3 +300,3 @@
#elif
#else
QSize size = this->size() * devicePixelRatio();
#endif
cg2121 (Migrated from github.com) commented 2020-12-09 17:00:06 -06:00

Changed to CefRect, still needs confirmation it still compiles on Windows.

Changed to CefRect, still needs confirmation it still compiles on Windows.
cg2121 (Migrated from github.com) reviewed 2020-12-09 17:00:29 -06:00
panel/browser-panel.cpp
cg2121 (Migrated from github.com) commented 2020-12-09 17:00:29 -06:00

Fixed.

Fixed.
cg2121 (Migrated from github.com) reviewed 2020-12-09 17:00:47 -06:00
panel/browser-panel.hpp
cg2121 (Migrated from github.com) commented 2020-12-09 17:00:46 -06:00

Didn't work for me.

Didn't work for me.
RytoEX (Migrated from github.com) reviewed 2020-12-09 17:04:58 -06:00
panel/browser-panel.cpp
@ -287,3 +300,3 @@
#elif
#else
QSize size = this->size() * devicePixelRatio();
#endif
RytoEX (Migrated from github.com) commented 2020-12-09 17:04:58 -06:00

My hope is that if we use CefRect everywhere, it's more uniform and less code in ifdefs.

My hope is that if we use CefRect everywhere, it's more uniform and less code in ifdefs.
cg2121 commented 2020-12-09 17:30:57 -06:00 (Migrated from github.com)

Service integration should be working with the latest push.

Service integration should be working with the latest push.
👍 2
WizardCM commented 2020-12-10 03:33:28 -06:00 (Migrated from github.com)

error: X Error: BadWindow (invalid Window parameter), Major opcode: XI_BadMode (invalid Mode parameter), Minor opcode: 46, Serial: 250

I've spent a few hours trying to bypass this error. No luck. If I remove the windowInfo.SetAsChild call, a secondary window does open, but that's not the goal.

I tried a number of variations, including using this->effectiveWindowId() or manually setting windowInfo.parent_window. Everything would return the same error.

The only way I was able to change the behaviour is using XConfigureWindow and XReparentWindow after CreateBrowserSync, which leaves an extra empty black window which then freaks out and cannot be closed. The panel itself renders correctly but cannot be interacted with or resized.

I don't know X well enough to be able to do this using X functions correctly.

> error: X Error: BadWindow (invalid Window parameter), Major opcode: XI_BadMode (invalid Mode parameter), Minor opcode: 46, Serial: 250 I've spent a few hours trying to bypass this error. No luck. If I remove the `windowInfo.SetAsChild` call, a secondary window does open, but that's not the goal. I tried a number of variations, including using `this->effectiveWindowId()` or manually setting `windowInfo.parent_window`. Everything would return the same error. The only way I was able to change the behaviour is using `XConfigureWindow` and `XReparentWindow` after `CreateBrowserSync`, which leaves an extra empty black window which then freaks out and cannot be closed. The panel itself renders correctly but cannot be interacted with or resized. I don't know X well enough to be able to do this using X functions correctly.
cg2121 commented 2020-12-10 14:14:05 -06:00 (Migrated from github.com)

@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).

@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).
WizardCM commented 2020-12-10 16:12:36 -06:00 (Migrated from github.com)

@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:

  1. I cannot click within the webpage to interact with it (either within a dock, or service integration login)
  2. Resizing the dock/login doesn't resize the webpage, and instead shows the empty container
  3. When closing service integration login, both BadWindow: 25 and BadWindow: BadName occur (if I resize before closing, I also get BadWindow: BadColor for every resize event) Fixed in CEF 4280
  4. After triggering 3, on shutdown I get Trace/breakpoint trap (core dumped) - the breakpoint is at ../sysdeps/nptl/futex-internal.h Fixed in CEF 4280
@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: 1. I cannot click within the webpage to interact with it (either within a dock, or service integration login) 2. Resizing the dock/login doesn't resize the webpage, and instead shows the empty container 3. ~~When closing service integration login, both `BadWindow: 25` and `BadWindow: BadName` occur (if I resize before closing, I also get `BadWindow: BadColor` for every resize event)~~ Fixed in CEF 4280 4. ~~After triggering 3, on shutdown I get `Trace/breakpoint trap (core dumped)` - the breakpoint is at `../sysdeps/nptl/futex-internal.h`~~ Fixed in CEF 4280
cg2121 commented 2020-12-10 18:00:56 -06:00 (Migrated from github.com)

@WizardCM the crash on exit appears to only happen on CEF 4183, but resizing and clicking work fine for me.

@WizardCM the crash on exit appears to only happen on CEF 4183, but resizing and clicking work fine for me.
WizardCM commented 2020-12-10 18:04:36 -06:00 (Migrated from github.com)

@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 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. :(
WizardCM commented 2020-12-10 18:20:45 -06:00 (Migrated from github.com)

@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.

@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.
RytoEX commented 2020-12-11 02:12:58 -06:00 (Migrated from github.com)

Needs testing on other OSs to make sure it doesn't break anything.

What OS did you test on so far? Can you please add that information to the original post?

> Needs testing on other OSs to make sure it doesn't break anything. What OS did you test on so far? Can you please add that information to the original post?
cg2121 commented 2020-12-11 13:44:43 -06:00 (Migrated from github.com)

I've tested it so far on Ubuntu 20.04 with QT 5.12.8.

I've tested it so far on Ubuntu 20.04 with QT 5.12.8.
phedders commented 2020-12-11 13:49:42 -06:00 (Migrated from github.com)

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 <paul+github@.mjr.org>
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

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 <paul+github@.mjr.org> > 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
cg2121 commented 2020-12-11 13:52:02 -06:00 (Migrated from github.com)

@WizardCM by chance, are you using a VM to test it?

@WizardCM by chance, are you using a VM to test it?
cg2121 (Migrated from github.com) reviewed 2020-12-11 14:38:43 -06:00
panel/browser-panel.cpp
@ -287,3 +300,3 @@
#elif
#else
QSize size = this->size() * devicePixelRatio();
#endif
cg2121 (Migrated from github.com) commented 2020-12-11 14:38:43 -06:00

Nope. It has to use RECT on Windows.

Nope. It has to use RECT on Windows.
cg2121 commented 2020-12-11 14:40:48 -06:00 (Migrated from github.com)

I've now tested on Windows. Resizing doesn't quite work as expected, though.

I've now tested on Windows. Resizing doesn't quite work as expected, though.
WizardCM commented 2020-12-11 15:37:48 -06:00 (Migrated from github.com)

@WizardCM by chance, are you using a VM to test it?

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.

> @WizardCM by chance, are you using a VM to test it? 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.
RytoEX (Migrated from github.com) reviewed 2020-12-11 15:54:05 -06:00
panel/browser-panel.cpp
@ -287,3 +300,3 @@
#elif
#else
QSize size = this->size() * devicePixelRatio();
#endif
RytoEX (Migrated from github.com) commented 2020-12-11 15:54:05 -06:00

That's unfortunate. Oh well.

That's unfortunate. Oh well.
cg2121 commented 2020-12-12 13:47:50 -06:00 (Migrated from github.com)

The obs-browser library lookup is now un-hardcoded. Requires https://github.com/obsproject/obs-studio/pull/3871 to work.

The obs-browser library lookup is now un-hardcoded. Requires https://github.com/obsproject/obs-studio/pull/3871 to work.
cg2121 (Migrated from github.com) reviewed 2020-12-12 13:48:43 -06:00
panel/browser-panel.hpp
cg2121 (Migrated from github.com) commented 2020-12-12 13:48:43 -06:00

This library lookup is fixed now.

This library lookup is fixed now.
eNBeWe commented 2020-12-13 10:37:12 -06:00 (Migrated from github.com)

I just built this code on Ubuntu 20.04:

  • With the CEF version from the obs-studio documentation (3770) the browser contents are super tiny (approx 1% zoom level)
  • With CEF 4280 the first tests look quite promising
    • The embedded browser is unable to play twitch streams. I would like to use it to monitor the outgoing stream, which doesn't work because of this.
    • Apart from that it seems to be able to do what I need it to do
  • I would like to test the twitch integrations, but can't authenticate to twitch with the locally built version.
I just built this code on Ubuntu 20.04: - With the CEF version from the obs-studio documentation (3770) the browser contents are super tiny (approx 1% zoom level) - With CEF 4280 the first tests look quite promising - The embedded browser is unable to play twitch streams. I would like to use it to monitor the outgoing stream, which doesn't work because of this. - Apart from that it seems to be able to do what I need it to do - I would like to test the twitch integrations, but can't authenticate to twitch with the locally built version.
cg2121 commented 2020-12-13 17:01:36 -06:00 (Migrated from github.com)

I've fixed the Windows resize issue. All I had to do was to set the window container as frameless.

I've fixed the Windows resize issue. All I had to do was to set the window container as frameless.
WizardCM commented 2020-12-13 17:41:59 -06:00 (Migrated from github.com)

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 USE_QT_LOOP CMake flag, which is designed to fix conflicts between Qt, CEF & GTK. I have this enabled to emulate production builds as much as possible. Getting browser panels to work properly with this flag will require a fair bit of rework of the panel code, but should solve a lot of issues.

  • With the CEF version from the obs-studio documentation (3770) the browser contents are super tiny (approx 1% zoom level)

@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.

  • The embedded browser is unable to play twitch streams. I would like to use it to monitor the outgoing stream, which doesn't work because of this.

This is likely because non-free codecs are not enabled in Spotify builds.

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 `USE_QT_LOOP` CMake flag, which is designed to fix conflicts between Qt, CEF & GTK. I have this enabled to emulate production builds as much as possible. Getting browser panels to work properly with this flag will require a fair bit of rework of the panel code, but should solve a lot of issues. > * With the CEF version from the obs-studio documentation (3770) the browser contents are super tiny (approx 1% zoom level) @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](https://github.com/obsproject/obs-browser/pull/228). > * The embedded browser is unable to play twitch streams. I would like to use it to monitor the outgoing stream, which doesn't work because of this. This is likely because non-free codecs are not enabled in Spotify builds.
tbodt (Migrated from github.com) reviewed 2020-12-15 20:18:29 -06:00
panel/browser-panel.hpp
tbodt (Migrated from github.com) commented 2020-12-15 20:18:29 -06:00

I just tried to port this over to the mac branch and got a "use of undeclared identifier": there apparently is no such function. Is this a typo, or in an unmerged branch or something?

I just tried to port this over to the mac branch and got a "use of undeclared identifier": there apparently is no such function. Is this a typo, or in an unmerged branch or something?
tbodt (Migrated from github.com) reviewed 2020-12-15 20:20:10 -06:00
panel/browser-panel.hpp
tbodt (Migrated from github.com) commented 2020-12-15 20:20:10 -06:00

Ah, https://github.com/obsproject/obs-studio/pull/3871.

Ah, https://github.com/obsproject/obs-studio/pull/3871.
cg2121 commented 2020-12-19 15:59:22 -06:00 (Migrated from github.com)

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.

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.
cg2121 commented 2020-12-19 16:02:18 -06:00 (Migrated from github.com)

If that turns out to be the case, then this PR is ready to go.

If that turns out to be the case, then this PR is ready to go.
cg2121 commented 2020-12-19 16:42:15 -06:00 (Migrated from github.com)

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.

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.
🎉 3
PatTheMav commented 2020-12-22 10:22:46 -06:00 (Migrated from github.com)

OK, after a bit of debugging myself, the cause for the second error above is

github.com/obsproject/obs-browser@ce8f6f8269/panel/browser-panel.cpp (L99)

Essentially, path.Get() returns null.

github.com/obsproject/obs-browser@ce8f6f8269/panel/browser-panel.cpp (L82-L83)

I think this is because the value it's based on, rpath, ends up being an absolute path rather than an expected relative path, so the conversion fails.

@tbodt added a fix for this in https://github.com/obsproject/obs-browser/pull/197, so maybe adapt that here as well?

I guess merging https://github.com/obsproject/obs-browser/pull/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?

> OK, after a bit of debugging myself, the cause for the second error above is > > https://github.com/obsproject/obs-browser/blob/ce8f6f8269275c4fd672ef8ad25225a6b9418b78/panel/browser-panel.cpp#L99 > > Essentially, `path.Get()` returns `null`. > > https://github.com/obsproject/obs-browser/blob/ce8f6f8269275c4fd672ef8ad25225a6b9418b78/panel/browser-panel.cpp#L82-L83 > > I _think_ this is because the value it's based on, `rpath`, ends up being an absolute path rather than an expected relative path, so the conversion fails. @tbodt added a fix for this in https://github.com/obsproject/obs-browser/pull/197, so maybe adapt that here as well? I guess merging https://github.com/obsproject/obs-browser/pull/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?
cg2121 commented 2020-12-23 04:15:38 -06:00 (Migrated from github.com)

I've added the fix from #197. It also works on Linux without problems.

I've added the fix from #197. It also works on Linux without problems.
flexiondotorg commented 2020-12-29 12:24:31 -06:00 (Migrated from github.com)

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 USE_QT_LOOP=TRUE from my build. Working great, no issues encountered.

Thank for working on this! I've been testing on Ubuntu 20.04 with OBS 26.0.1 with the following patches applied: * https://github.com/obsproject/obs-studio/pull/3871 * https://github.com/obsproject/obs-browser/pull/254 I've also dropped `USE_QT_LOOP=TRUE` from my build. Working great, no issues encountered.
LiamDawe commented 2020-12-31 06:32:27 -06:00 (Migrated from github.com)

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.

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.
WizardCM commented 2021-01-16 20:25:08 -06:00 (Migrated from github.com)

@cg2121 Now that macOS panel support has been merged, please rebase your branch.

In the meantime, I'm going to try and get USE_QT_LOOP on linux to behave, just so we don't destroy existing configurations if possible. It won't be a requirement for this PR to be merged, but it's something I think would be worth doing.

@cg2121 Now that macOS panel support has been merged, please rebase your branch. In the meantime, I'm going to try and get `USE_QT_LOOP` on linux to behave, just so we don't destroy existing configurations if possible. It won't be a requirement for this PR to be merged, but it's something I think would be worth doing.
cg2121 commented 2021-01-17 01:43:55 -06:00 (Migrated from github.com)

It has been updated to the latest master.

It has been updated to the latest master.
Krutonium commented 2021-01-26 10:33:09 -06:00 (Migrated from github.com)

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.

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.
Sign in to join this conversation.
Reviewers
No reviewers
RytoEX
Labels
Clear labels   
area/accessibility
Improves accessibility of the program

  
area/dependency
Issue is in external library or dependency, not OBS itself

  
Confirmed

   
Crash Report

   
Delayed
Merging has been delayed due to an issue

  
Design Issue
There is an issue with incorporating this pull request's design. It may take some time to finalize.

  
Duplicate

   
Feedback required
Issue does not contain enough information or is missing the template

  
Good first issue
New to contributing to OBS? Start here!

  
kind/bug
Categorizes issue or PR as related to a bug.

  
kind/cleanup
Non-breaking change which makes code smaller or more readable

  
kind/enhancement
Enhancements are not bugs or new features but can improve usability or performance.

  
platform/linux

   
platform/macos

   
platform/windows

   
priority/high
This is an important PR

  
Question

   
Request for Comments
More feedback & discussion is requested

  
Seeking Other Contributors
This PR needs help from any other developers who are willing to give a crack at this code

  
Seeking Testers

   
Won't Fix / By Design
No labels
area/accessibility
area/dependency
Confirmed
Crash Report
Delayed
Design Issue
Duplicate
Feedback required
Good first issue
kind/bug
kind/cleanup
kind/enhancement
platform/linux
platform/macos
platform/windows
priority/high
Question
Request for Comments
Seeking Other Contributors
Seeking Testers
Won't Fix / By Design
Milestone
Clear milestone
No items
No milestone
OBS Studio 27.0
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
gh-obsproject/obs-browser!254
No description provided.