-
Notifications
You must be signed in to change notification settings - Fork 76
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update to branch-head/58 and add ARM support. #61
Conversation
Needs testing that amd64 still works and no idea how the go build works with pkg-config.
Cross compiling was causing issues with native builds
Requires testing. Solves the issue of gn not producing build files for libjingle. Includes darwin magic library
Lumped as one due to sheer size of commit
Builds with this config, but tests fail.
Currently appears to be broken on arm
Add pkg-config defintion for ARM. Fix incorrect value in DataConnection.BufferedAmmount Change a couple of depreacted import paths. Add extra assertions to debug why SetConfig test is failing.
Add more debug and error handling with standard application. Change Set and Get Configuration test to use a new peer connection, as changing ICE servers on an existing connection is officially unsupported. Maybe the castConfig_ should take the values that have been set into consideration.
I should also add that although I'm not sure how to get travis to test this, it has been tested using resin.io on a real device and functions perfectly. |
Thanks! This looks promising. @keroserene, if you don't mind, I can try and get this merged. |
else | ||
gn gen out/$CONFIG --args='is_debug=false' || exit 1 | ||
fi | ||
ninja -C out/$CONFIG webrtc webrtc/test webrtc/pc:pc_test_utils || exit 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think ninja -C out/$CONFIG webrtc field_trial metrics_default fake_audio_device
is sufficient.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/fake_audio_device/pc_test_utils/
@tomtom5152 This looks great, thanks for adding ARM support and updating all the things! |
Alright, I merged the branch-heads/58 part of this in 9c40f4d and efd9a1a, rebuilt the magic in ab1b648, and opened #63 for fixing up the configuration methods. @keroserene I tested snowflake with the above and it bootstrapped tor to 100%. We're gonna need a corresponding tor-browser-bundle patch, which I'll work on next. Then I'll get back to merging the arm parts here. Thanks again @tomtom5152! |
Thats some git-foo magic going on right there, are you able to test with ARM or would you like me to do it. I also notice that the arm magic didn't get rebuilt here so would you like a new one of those? |
@tomtom5152 I haven't merged anything related to arm yet. I'll be getting to that soon. Thanks for the offer but we probably wouldn't take any binary patches (hence, why I recompiled the magic). |
Ah fair enough, I can appreciate not taking binary patches, although it wasn't something I had considered. If it could make an appearance soonish though I would be very grateful. You've probably figured it out, but the build command is |
That's done at, https://trac.torproject.org/projects/tor/ticket/21748#comment:3 Going to work on merging the arm parts now. |
@@ -57,6 +57,13 @@ else | |||
popd | |||
fi | |||
|
|||
if [ "$ARCH" = "arm" ]; then | |||
echo "Manually fetching arm sysroot" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was doing this manually necessary? gclient sync
seems to do,
________ running '/usr/bin/python src/build/linux/sysroot_scripts/install-sysroot.py --running-as-hook' in '/Sandbox/go-webrtc/third_party/webrtc'
Wheezy arm sysroot image already up to date: /Sandbox/go-webrtc/third_party/webrtc/src/build/linux/debian_wheezy_arm-sysroot
Precise amd64 sysroot image already up to date: /Sandbox/go-webrtc/third_party/webrtc/src/build/linux/ubuntu_precise_amd64-sysroot
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gah, sorry, nevermind. I see you're cross-compiling.
@tomtom5152 Can you test the |
@arlolra That works perfectly for me on my BeagleBone, |
No, that's ok. I merged the branch as is. Think we got everything here now. Thanks for all your help. |
@tomtom5152 Is this support for ARM64 or ARM that you added? I am trying get this running on an ARMV7l machine, but when I run go build I get I believe I successfully cross-compiled for ARM V7 using this setting while building: I have a newly generated libwebrtc.a file, and libwebrtc_common.a. Do you know how I can use these .a files to get go-webrtc working on ARM v7? |
I tried swapping out the arm binary with one I built myself, as well as another prebuild arm v7 binary I found online and I'm still seeing the same error. I made sure that the webrtc-linux-arm.pc script is getting called successfully, and I changed the name of the binary here: No matter what I do whenever I build I get the same error: Are my binaries bad, or am I just doing something wrong? |
@tomtom5152 do you have any ideas for how I can use my WebRTC binary with this project? |
Apologies @circuitry2, I've been mostly offline since just before Christmas and just going through my emails now. My CGO is also rather rusty at this point as I haven't had to argue with it since this PR, I've just been using Go. By the looks of it you have issues with the include directory config, I'm not sure if this in itself indicates a good binary or not, as I believe it may come before the binary is used. That should solve the include issues, but you might now have an issue with the binary. I would strongly suggest you checkout build.sh for how to create a standard binary without all the bloat that the WebRTC instructions give. Particularly L77 would be of some interest, as the latest install-sysroot.py supports ARM64, and L117 would be a good starting point for the compile command. Hope that's of some use, although I have no use for it at the moment, this is of some interest to me still as I may use ARM64 in the future. |
@circuitry2 This error message is the same as at #76. It probably means that your build is missing the necessary Or, if you built your own webrtc that's newer than branch-heads/58, the problem may be that upstream, the header files lost a "webrtc/" prefix. So "webrtc/api/peerconnectioninterface.h" needs to become "api/peerconnectioninterface.h" in ctestenums.cc and other places. Your best bet is to build your webrtc using the current commit hash in build.sh. |
Quite a bit going on in this, my original idea was just to add support for ARM based devices (specifically the BeagleBone Black for my use case). This got a bit out of hand when the gyp build no longer support cross compiling (couldn't figure out why). Easiest way to overcome this was to bump WebRTC to the latest version and switch to using
gn gen
to create the ninja files.As such this PR contains the following changes.