WonderInventions / node-webrtc

node-webrtc is a Node.js Native Addon that provides bindings to WebRTC M98
Other
109 stars 8 forks source link

Electron support #6

Open mikey0000 opened 9 months ago

mikey0000 commented 9 months ago

I'm having some issues around rebuilding for electron and loading the native binary.

Any chance you've tried this or could build a binary for electron?

mikey0000 commented 9 months ago

To add it looks like debug info is still in the binaries?

duvallj commented 9 months ago

You should not need this for Electron, since Electron should have WebRTC built-in. I believe you'd have trouble trying to load a Node-API binary into an Electron process, since Electron uses a different javascript engine. Closing as out-of-scope.

mikey0000 commented 9 months ago

It doesn't, electron 25 does not contain webrtc its a node environment, the project you forked from supported electron.

duvallj commented 9 months ago

https://stackoverflow.com/a/57286645 ?

mikey0000 commented 9 months ago

No, we already have an electron app using the original webrtc that you forked from, however you have binaries for more platforms (mac) which we are exploring. Not to mention bug fixes and so on.

At the moment there is too much work involved in changing it to the frontend right now.

duvallj commented 9 months ago

I'm assuming you're trying to follow the instructions at https://www.electronjs.org/docs/latest/tutorial/using-native-node-modules . What part specifically is failing?

mikey0000 commented 9 months ago

So I haven't gone off to spend time recompiling for electron, it would probably work but then I would need to fork and host my own prebuilts. I figured it would be much easier to just ask for you to compile targeting electron and host them along your existing ones. The whole reason for the move in the first place is on the old wrtc it won't even install on a mac even if you target electron. With the current package that's published electron-rebuild isn't able to rebuild as there is not enough info/cmake etc

mikey0000 commented 9 months ago

Yes I could clone down the repo and do it but again, benefit everyone and have electron prebuilts for everyone?

mikey0000 commented 3 weeks ago

Just an update here, the mac and linux wrtc binaries work perfectly in Electron, Just the win32 binary does not, so its likely just a small change in the way its being built vs the original wrtc module. Pretty please could you take a look into this?

mikey0000 commented 3 weeks ago

I just found that 0.7.3 works fine with electron but 0.8.0 does not, so this should hopefully be an easy fix.

mikey0000 commented 3 weeks ago

one more update, it works fine in electron, only change I've made is how the node binaries are loaded.

mikey0000 commented 3 weeks ago

using Dumpbin this is the difference, sorry had reverted to wrtc without realising, the roamhq/wrtc still fails on windows 0.8.0, works on mac and linux

your build of wrtc

 Summary

        1000 .00cfg
       BC000 .data
        4000 .gxfg
       4F000 .pdata
      1D9000 .rdata
        C000 .reloc
        1000 .retplne
        1000 .rodata
        1000 .rsrc
      BB5000 .text
        1000 .tls
        1000 _RDATA

original wrtc

 Summary (original wrtc windows node works with electron)

       92000 .data
        D000 .pdata
       8D000 .rdata
        3000 .reloc
      158000 .text
duvallj commented 3 weeks ago

Just an update here, the mac and linux wrtc binaries work perfectly in Electron, Just the win32 binary does not, so its likely just a small change in the way its being built vs the original wrtc module.

oh wow that's really interesting! As far as I'm aware, I'm not really trying to do anything different on Windows compared to the original repo.

Can you help me understand what the Dumpbin difference you posted is telling me? I understand it is showing the different binary sections and their relocations, and that the binary I've provided has more sections with different relocations, but I don't understand why this would be an issue for Electron to load it.

mikey0000 commented 3 weeks ago

Do you apply the CMAKE patch? I think that might have something to do with it. I'm trying to get a running build locally so I can explore the problem better. Windows is such a pain...