WebRTC data channels backend

[ferjm]

This is not entirely spec compliant, but it contains the basic stuff to make data channels work. It allows sending only strings for now.

[sdroege]

This is going to explode sooner or later. The datachannel object is thread-safe, but until https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/-/issues/1168 is solved you'll have to put your own wrapper struct around it on which Send/Sync are implemented.

[sdroege]

While at it, you could also hide the signal emission things in functions inside impl DataChannel :)

[Manishearth]

Overall this seems good, but I'm not quite sure of the callback model.

It kind of would be nice if the actual data channel was completely backend-side, and never exposed to the client as InternalDataChannel, and the backend just kept track of DataChannelIds that it returned to the frontend.

This also means that data channel events can simply be another trait method on WebRTCSignaller!:

trait WebRtcSignaller {
    ....
    fn on_data_channel_event(id: DataChannelId, event: DataChannelEvent);
}

enum DataChannelEvent {
    NewChannel,
    Open,
    Close,
    Error(WebRtcError),
    OnMessage(String),
}

trait WebRtcControllerBackend {
     fn create_data_channel(DataChannelInit) -> DataChannelId;
     fn send_data(id: DataChannelId, msg: String);
}

How does this sound? The signaller/controller distinction exists precisely so that this kind of stuff is easy to do without Arc<Mutex<Callbacks>>. This was quite intentional; I considered quite a few alternative designs when desigining webrtc threading and this seemed to be the most robust. I'd prefer if we use it if at all possible.

[Manishearth]

This feels potentially racy, what if the client sets the callbacks after the channel is created?

Perhaps we should pass in the WebRtcDataChannelCallbacks in at construction? As an added bonus, this means we don't need the arc/mutex/trait at all! We can just have something like

struct DataChannelCallbacks {
   pub open: Box<FnOnce<...>>,
   pub on_message: Box<FnMut<...>,
   // ...
}

and split out each field when setting callbacks.

[Manishearth]

We do something similar in regular webrtc, we pass in a "signaller" object at creation.

[Manishearth]

send() can be called by the controller instead

[Manishearth]

the controller probably can manage ids itself and return an id here.

[Manishearth]

the outer mutex doesn't seem necessary?

[Manishearth]

@sdroege should the on-message-string , on-open, on-close, and on-error` callbacks require Sync here? I recall we fixed some of these cases earlier: sdroege/gstreamer-rs#154

[ferjm]

rustc complains if I don't add the outer mutex

[Manishearth]

Yeah sorry this is the Sync requirement that I think needs to be removed upstream. No worries for now

[Manishearth]

I think these ids should just be numbers and individual webrtc implementations should produce and consume them

We shouldn't have a DataChannelBackend trait at all, all of this should be internal to the webrtc impl

[Manishearth]

probably should be removed or turned into debug!()

[Manishearth]

this looks good, but I don't think we should have a DataChannel trait at all, webrtc implementations should deal with that internally and just surface ids for referring to them

[Manishearth]

@bors-servo r+

[bors-servo]

📌 Commit 512c76f has been approved by Manishearth

[bors-servo]

⌛ Testing commit 512c76f with merge 64e4507...

[bors-servo]

☀️ Test successful - checks-travis
Approved by: Manishearth
Pushing 64e4507 to master...