Kagami / mpv.js

:movie_camera: mpv pepper plugin
Creative Commons Zero v1.0 Universal
408 stars 63 forks source link

Fix for new Electron (no-sandbox) #64

Closed qdimka closed 4 years ago

qdimka commented 4 years ago

Hi everyone

Electron 8.2.0 Node 13.11

I want to use mpv.js with Electron. I installed the plugin and turned on the "no-sandbox" flag. But instead of video, I got an empty black screen. There are no errors, no warnings here.

I get "ready" and "property changed" events. I've tried to add "MPVJS_TERMINAL=1 MPVJS_VERBOSE=1", but no additional output here.

I can produce a minimal repro repository if needed.

qdimka commented 4 years ago

This is solution gcc -Wl,--no-as-needed -shared -lavformat -o node_modules/electron/dist/libffmpeg.so

zSoulweaver commented 4 years ago

Can confirm is also an issue on windows using Electron 8.2.3, went through the latest stable releases and it seems to break anything post 4.x.

Initial 5.0 electron version reverts back to blackscreen with nothing.

Edit: Can get working via adding app.commandLine.appendSwitch('no-sandbox') but obviously is not an ideal solution. Since 5.0 they've enabled mixed sandbox which can affect pepper plugins. Looks like somebody attempted to PR a switch for certain plugins https://github.com/electron/electron/pull/18939 but it was never merged. So either you run old Electron versions or if you are running post 5.0 versions to disable sandboxing or wait to see if electron ever fix anything.

Kagami commented 4 years ago

What's wrong with disabling sandbox?

The worst thing is that Pepper plugins are deprecated as a whole, it might be completely removed from the codebase one day...

zSoulweaver commented 4 years ago

Sorry, didn't notice there was already an existing issue #51 but I suppose this is where it'll continue now anyway.

For most, I don't believe there will be a major concern with disabling sandbox as the code in the application should be trusted (i.e VSCode and Discord both have sandbox disabled). It would only really impact apps that are going to be using any random web content, where the additional security would be handy, which wouldn't be many.

So I don't think it's a major issue, but under the usage section in the readme might be a good idea to state that anyone using Electron post 5.x will need to have app.commandLine.appendSwitch('no-sandbox') in their main process.