keroserene / go-webrtc

WebRTC for Go
Other
455 stars 76 forks source link

Webrtc win #69

Open asicerik opened 7 years ago

asicerik commented 7 years ago

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

arlolra commented 7 years ago

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.

asicerik commented 7 years ago

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.

arlolra commented 7 years ago

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.

blizzardplus commented 7 years ago

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

asicerik commented 7 years ago

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

blizzardplus commented 7 years ago

@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

asicerik commented 7 years ago

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

blizzardplus commented 7 years ago

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

asicerik commented 7 years ago

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.

arlolra commented 7 years ago

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

asicerik commented 7 years ago

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?

arlolra commented 7 years ago

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

Zentendo commented 6 years ago

Any plans to continue working on this? @asicerik

Currently using this fork in production.

asicerik commented 6 years ago

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

arlolra commented 6 years ago

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.

blizzardplus commented 6 years ago

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

Zentendo commented 6 years ago

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

asicerik commented 6 years ago

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

Zentendo commented 6 years ago

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

asicerik commented 6 years ago

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