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

Add streaming API for datachannel messages and state changes. #91

Merged
merged 4 commits into from May 15, 2019

Conversation

@FrobtheBuilder
Copy link
Contributor

commented May 14, 2019

This change adds a streaming API to the RTCDataChannel. Adds stateChangeStream and messageStream to the RTCDataChannel class, which are broadcast streams that emit state changes and messages, just as would be passed to the onDataChannelState and onMessage handlers. This allows for easily attaching multiple listeners to these events, whereas previously the class could only support one handler for each.

Adds functionality from my own wrapper utility class:

class StreamingRTCDataChannel {
  RTCDataChannel dataChannel;
  final _messageController = StreamController<RTCDataChannelMessage>.broadcast();
  final _stateChangeController = StreamController<RTCDataChannelState>.broadcast();
  Stream<RTCDataChannelMessage> message;
  Stream<RTCDataChannelState> stateChange;
  StreamingRTCDataChannel(this.dataChannel) {
    message = _messageController.stream;
    stateChange = _stateChangeController.stream;
    dataChannel.onMessage = _messageController.add;
    dataChannel.onDataChannelState = _stateChangeController.add;
  }
}

Which will no longer be necessary after this change.

Also updates the datachannel example code to show usage.

This change has been tested within our own app and will not break the existing API, as it's only adding new properties to the class. It should not cause any performance issues, since broadcast streams don't buffer their events and only deliver them to active listeners on demand. If nothing is listening, events are simply discarded.

@FrobtheBuilder

This comment has been minimized.

Copy link
Contributor Author

commented May 14, 2019

It might be a good idea to use the SynchronousStreamController instead since the events are already asynchronous. It improves latency, but it DOES break the stream contract. That shouldn't be an issue though since add is just being called in an async listener itself. Tell me what you think.

@FrobtheBuilder

This comment has been minimized.

Copy link
Contributor Author

commented May 14, 2019

I decided to just go ahead and do it. Should improve performance. Just tell me if you think there's some reason it shouldn't work that way. It should behave exactly as expected since add is the last thing being done in the async listener, and it works perfectly fine in testing.
https://api.dartlang.org/stable/2.3.0/dart-async/SynchronousStreamController-class.html

@cloudwebrtc cloudwebrtc merged commit d25ce9a into cloudwebrtc:master May 15, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.