go-webrtc icon indicating copy to clipboard operation
go-webrtc copied to clipboard

Webrtc win

Open asicerik opened this issue 6 years ago • 20 comments

Hey there! We needed a version of this to run under Windows, so I put in all the work to get DLL linkage in. Sooner or later, hopefully Google or MSFT will get their act together and get Go / CGO to work properly in Windows using Visual Studio.

The DLL approach is not the cleanest since we have to do: Go -> CGO -> Go -> DLL. I did not want to change your Go code to be able to access the DLL directly, so there is an extra layer of CGO there that we don't need. If we put an abstraction layer in the Go code, we could get rid of the CGO layer for Windows, but it introduces another layer in the POSIX code. So, I don't recommend that.

We are going to be working on Video and Audio with this library, so I hope to be able to continue to contribute to the project. If you are not happy with something here, let me know how you would like to see it done.

I also made some changes to the test programs. It seems the enums were backwards w.r.t. expected vs. actual.

The DLL build in Visual Studio right now, but we could move that to a command line system if you wish. Thanks! Erik

asicerik avatar Oct 02 '17 22:10 asicerik

Thanks for the pull. Haven't had a chance to look at it yet, but a few quick things.

We needed a version of this to run under Windows, so I put in all the work to get DLL linkage in.

Have you seen https://github.com/golang/go/issues/11058

We are going to be working on Video and Audio with this library, so I hope to be able to continue to contribute to the project.

See #65, which also needs review / merge.

arlolra avatar Oct 11 '17 17:10 arlolra

Thanks for the reply and heads-up. I have not seen either of those. The first appears to be a way to make Go into a DLL. I suppose that path could work too, but it turns everything upside down from the Linux/etc builds. I will have to ponder that. The second (#65) looks interesting. That is a very different approach to what I am doing. I am not sure which is better at this point. Are you guys planning on merging that in? It was submitted in May.

asicerik avatar Oct 12 '17 16:10 asicerik

I suppose that path could work too, but it turns everything upside down from the Linux/etc builds. I will have to ponder that.

Sorry, I was confused with another project. It's not really relevant. Please ignore.

Are you guys planning on merging that in? It was submitted in May.

Yes, I was planning on reviewing it shortly. As apparent, we don't always have a ton of cycles available.

arlolra avatar Oct 12 '17 16:10 arlolra

@asicerik: In the latest commit, build_win.bat is removed, so how can I reproduce the build for webrtc.dll? Was it removed because it breaks the Linux build? Also, we should not have the headers in the "include" folder in git. It should be generated as part of the build process.

blizzardplus avatar Oct 18 '17 17:10 blizzardplus

@blizzardplus - thanks for getting back to me. That file should not have disappeared, I will look into it. On the include folder, I thought that was already there? The cgo files have to reference the include files if you use the pre-build binaries (for Linix or Win). I pulled in the audio changes already that were merged in yesterday, and I am adding video. Let me get all that done, and I will put in a new pull request. Does that make sense?

asicerik avatar Oct 18 '17 20:10 asicerik

@asicerik We would like to have everything reproducible, so it means that the windows build script should generate the DLL/LIB and include headers from the source code. Take a look at build.sh:

echo "Copying headers ..." pushd $WEBRTC_SRC || exit 1 rm -rf "$INCLUDE_DIR" for h in $(find webrtc/ -type f -name '*.h') do mkdir -p "$INCLUDE_DIR/$(dirname $h)" cp $h "$INCLUDE_DIR/$h" done popd

blizzardplus avatar Oct 19 '17 20:10 blizzardplus

go ahead and reject this pull request. I will another one in a few days that includes audio and video support. Is there anyone on your side that can assist with the build script? I am not very good script writer :)

asicerik avatar Oct 19 '17 22:10 asicerik

@asicerik As long as you have clear instructions on how to build the DLL, I should be able to help you write the script, or can probably write it myself. And by instructions, I mean what steps that have to be taken to create the DLL and what process looks like. Also, we have to make sure not to break the Linux build. Thanks for your help.

blizzardplus avatar Oct 20 '17 03:10 blizzardplus

The dll currently has a Visual Studio Solution for it. You can't run it from the command line, but it is easy to build. Do you need a command line version for the dll? It can be treated like the checked in binaries for the Linux/Mac builds where you don't have to build the dll to run the Windows version. Linux support is fully maintained. I have not tried building Mac since I don't have a Mac. I have no reason to expect Mac should fail though.

asicerik avatar Oct 20 '17 03:10 asicerik

The checked in binaries are only for convenience. In practice, we build from source when including them as part of a product.

See https://gitweb.torproject.org/builders/tor-browser-build.git/tree/projects/webrtc

arlolra avatar Oct 20 '17 15:10 arlolra

Is project being used as part of Tor? That would be cool. I understand about the binaries. Are you going to have a Windows build server?

asicerik avatar Oct 20 '17 16:10 asicerik

Is project being used as part of Tor? That would be cool.

Yes, it's used in a pluggable transport, https://trac.torproject.org/projects/tor/wiki/doc/Snowflake

I understand about the binaries. Are you going to have a Windows build server?

We cross-compile with Mingw-w64, http://mingw-w64.org/doku.php

arlolra avatar Oct 20 '17 16:10 arlolra

Any plans to continue working on this? @asicerik

Currently using this fork in production.

Zentendo avatar Mar 02 '18 17:03 Zentendo

We are currently using it in our application. We are working on getting Mac support to work right now. Once that is done, I want to pull in all the changes from here, and re-push

asicerik avatar Mar 02 '18 22:03 asicerik

From our perspective, @blizzardplus has gotten our app to build with the visual studio project here, but we're still working on cross-compilation.

Before merging though, it'd be nice to automate generating some of the redundant parts of this patch so that there's less of a maintenance burden as the code evolves.

arlolra avatar Mar 03 '18 00:03 arlolra

@asicerik any idea on how to maintain the code moving forward, there seems to be a lot of dependencies between the DLL wrapper and the actual code, so wondering if there's a way to automate or at least reduce the maintenance overhead.

blizzardplus avatar Mar 06 '18 21:03 blizzardplus

@asicerik Is there any way to pass options into CreateDataChannel in this fork?

Currently I can't seem to set unreliable/UDP options on the data channel (which is the primary reason to use WebRTC) on windows like the latest branch of keroserene/go-webrtc on linux. It just creates a reliable channel by default regardless of the webrtc.Init{} options I pass into it.

Zentendo avatar Apr 01 '18 14:04 Zentendo

Hey all, It seems like the original project has a way to cross-compile into Windows now. You might want to check that out. If that does not work for you, let me know. https://github.com/keroserene/go-webrtc/pull/79

asicerik avatar Apr 01 '18 18:04 asicerik

@asicerik Windows support appears not to be coming for a while: https://github.com/keroserene/go-webrtc/issues/83

It would be great if you could update these bindings to allow unreliable / UDP messaging as a temporary solution in the meanwhile.

Zentendo avatar Apr 27 '18 10:04 Zentendo

I will see what I can do to get the project synced up, etc. I am pretty busy with work, so we'll see.

asicerik avatar Apr 27 '18 22:04 asicerik