Skip to content
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

Closed
wants to merge 12 commits into from

Conversation

tomtom5152
Copy link
Contributor

@tomtom5152 tomtom5152 commented Mar 13, 2017

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.

  • Add ARM support to build script.
  • Update build script commit hash to latest version.
  • Add arm pkg-config file.
  • Update WebRTC to branch-head/58.
    • Update WebRTC headers.
  • Update tests to reflect breaking changes to the WebRTC API (specifically peer connections).
  • Add new required methods in the fake data channel.

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.
@tomtom5152
Copy link
Contributor Author

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.

@arlolra
Copy link
Collaborator

arlolra commented Mar 14, 2017

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
Copy link
Collaborator

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.

Copy link
Collaborator

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/

@keroserene
Copy link
Owner

@tomtom5152 This looks great, thanks for adding ARM support and updating all the things!
The travis build looks happy at the moment. @arlolra Sounds good, thanks -- so long as it continues to work with snowflake I'd be stoked to see this merged 👍

@arlolra
Copy link
Collaborator

arlolra commented Mar 14, 2017

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!

@tomtom5152
Copy link
Contributor Author

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?

@arlolra
Copy link
Collaborator

arlolra commented Mar 14, 2017

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

@tomtom5152
Copy link
Contributor Author

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 GOARCH=arm ./build.sh. I used an AWS EC2 m4.large instance with arm-linux-gnueabi-hf to do it because I didn't have access to a linux machine locally, but your milage might vary.

@arlolra
Copy link
Collaborator

arlolra commented Mar 26, 2017

We're gonna need a corresponding tor-browser-bundle patch, which I'll work on next.

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"
Copy link
Collaborator

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

Copy link
Collaborator

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 added a commit to CodedInternet/godynastat that referenced this pull request Mar 27, 2017
@arlolra
Copy link
Collaborator

arlolra commented Mar 27, 2017

@tomtom5152 Can you test the arm branch and confirm if it works for you? (I merged your patch there and build the magic.) I only have access to an old armv6 pi and setting --args='arm_version=6 ...' seems to have bit rotted, since they don't test that in their matrix.

@tomtom5152
Copy link
Contributor Author

@arlolra That works perfectly for me on my BeagleBone, go test passes, as does the demo chat system. If you absolutely want to I might be able to set one up on a non standard SSH port for a while to test?

@arlolra
Copy link
Collaborator

arlolra commented Mar 27, 2017

If you absolutely want to I might be able to set one up on a non standard SSH port for a while to test?

No, that's ok. I merged the branch as is. Think we got everything here now. Thanks for all your help.

@arlolra arlolra closed this Mar 27, 2017
@circuitry2
Copy link

@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
../../../github.com/keroserene/go-webrtc/ctestenums.cc:2:48: fatal error: webrtc/api/peerconnectioninterface.h: No such file or directory #include "webrtc/api/peerconnectioninterface.h"

I believe I successfully cross-compiled for ARM V7 using this setting while building:
export GYP_DEFINES="OS=linux target_arch=arm arm_version=7 arm_use_neon=1"

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?
Happy to share the files and add support for 32 bit ARM once I get it working.
Thanks!

@circuitry2
Copy link

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:
-lwebrtc-linux-arm-magic \

No matter what I do whenever I build I get the same error:
../../../github.com/keroserene/go-webrtc/ctestenums.cc:2:48: fatal error: webrtc/api/peerconnectioninterface.h: No such file or directory #include "webrtc/api/peerconnectioninterface.h"

Are my binaries bad, or am I just doing something wrong?
I'm trying to build the project using a vagrant VM that I have that is specifically set up to cross compile go projects for ARM. I haven't had any problems with the VM to date. The VM just says various CGO and GO cross compiling environment variables.

@circuitry2
Copy link

@tomtom5152 do you have any ideas for how I can use my WebRTC binary with this project?

@tomtom5152
Copy link
Contributor Author

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.
If this is the case, then it may have something to do with the .pc files in the package root, see my webrtc-linux-arm.pc for more details. You will also need to inform CGO to use this file as per datachannel.go#12, although I can't help you with the exact syntax as I have never used ARM64.

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.
I would also advise that you ensure you are compiling the correct version of WebRTC, this library is a tad behind these days L12 has the version that I used successfully, although I'm sure a bump the lates WebRTC wouldn't go a miss with this repo, but baby steps.

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.

@uumaro
Copy link
Collaborator

uumaro commented Mar 26, 2018

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
../../../github.com/keroserene/go-webrtc/ctestenums.cc:2:48: fatal error: webrtc/api/peerconnectioninterface.h: No such file or directory #include "webrtc/api/peerconnectioninterface.h"

@circuitry2 This error message is the same as at #76. It probably means that your build is missing the necessary -Iinclude/ option to refer to the correct header files.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants