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

WASM Detach #565

Merged
merged 2 commits into from
Apr 1, 2019
Merged

WASM Detach #565

merged 2 commits into from
Apr 1, 2019

Conversation

backkem
Copy link
Member

@backkem backkem commented Mar 29, 2019

Support for Detach when targeting JS/WASM. The goal is to enable the same idiomatic API in both Native and WASM. This is a POC of the shim to turn the JS OnMessage Callback into the ReadWriteCloser interface used by the DataChannel.Detach. I tried to avoid leaking JS code into pions/datachannel by introducing an interface. Therefore this depends on pion/datachannel#11.

TODO

  • The WASM example deadlocks. Have to look into it further.
  • In order to use the same code in Native and WASM we would have to find a common solution for the way Detach is enabled in the SettingEngine in Native ATM.
  • Performance improvements. Maybe you can port your POC over @albrow?

@coveralls
Copy link

coveralls commented Mar 29, 2019

Pull Request Test Coverage Report for Build 2779

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+71.6%) to 80.594%

Totals Coverage Status
Change from base Build 2775: 71.6%
Covered Lines: 3177
Relevant Lines: 3942

💛 - Coveralls

@backkem backkem changed the title [WIP] WASM Detach WASM Detach Apr 1, 2019
@backkem
Copy link
Member Author

backkem commented Apr 1, 2019

Fixed some small bugs in the way Blob messages are handled. This fixes the deadlock issue.

Port the API object to the JS/WASM target. This allows more
code to work for both targets.
JS/WASM shim for Detach API
// valueToDataChannelMessage may block when handling 'Blob' data
// so we need to call it from a new routine. See:
// https://godoc.org/syscall/js#FuncOf
msg := valueToDataChannelMessage(data)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Nice catch!

Copy link
Contributor

@albrow albrow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! I just suggested a comment in one place.

datachannel_js.go Show resolved Hide resolved
@backkem backkem merged commit d906c2b into master Apr 1, 2019
@backkem backkem removed the review label Apr 1, 2019
@backkem backkem deleted the wasm-detach branch April 1, 2019 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants