Handle binary data

[simbleau]

Currently there's no handler for binary data, but it could be added quite trivially with bincode. I think this is an easy-win.

matchbox/matchbox_socket/src/webrtc_socket/native/signalling_loop.rs

Line 42 in c9d10cb

warn!("ignoring unexpected non-text message from signalling server: {:?}", message)

[johanhelsing]

Yeah, agree. I also thought I'd look into using msgpack.

This is my (very rough impression, and perhaps also wrong (?), list of pros and cons).

json

Pros

• very easy to inspect/debug
• no extra dependencies
• supported everywhere. If you need a javascript implementation for some reason, you can do that.

cons

• requires more bandwidth
• slower serialization and deserialization?

bincode

Pros

• very fast serialization and deserialization
• library is quite small
• widely used
  • will be well-maintained
  • may already be in your dependency tree

Cons

• rust only
• need to make sure client and server is using the same version of bincode
• (minor) need to be configured to prevent against attacks

msgpack

Pros

• smaller messages (?)
• works well with other languages and tooling

Cons

• slightly slower to serialize and deserialize than bincode(?)
• more dependencies

[garryod]

It may be worth either having a set of pick-one-of feature flags for serialisation (e..g., 'json', 'bincode' or 'msgpack') or operating a bring your own serialiser model and supplying a few common ones. The trade off between serialisation speed, serialisation speed & message size is going to depend on the network topology and data size, whilst json may be nice for debugging

[johanhelsing]

Yes, assuming it won't add too much complexity, I think feature flags would be a good idea.

One downside is that you now have to take care that the server and peers is configured in a compatible way (i.e. potential footgun if you only configure the socket side), or have the server support multiple formats (more bloated protocol and implementation). We should at least take care that we produce some meaningful error that will put people on the right track.

Perhaps it would make sense to default to msgpack in the cli and socket, and then leave the other formats as feature flags for more advanced users?

Also, I think I like the bring-your-own serializer idea, as long as we can keep it optional for less advanced users... So I guess we could probably do both? A default-signalling-msgpack feature flag enabled by default?

[simbleau]

re: binary data -

bincode-org/bincode#629 (comment)

Seems like bincode is wasm compat.

Does that make it the top contender?

Also, re "need to make sure client and server is using the same version of bincode" - i'm not sure that's an issue? Since we have the same problem for serde_json. I'd assume matching those versions is up to the user and how they want their clients to update.

[garryod]

Seems like a solid contender, though kind of wondering if this is premature optimisation, how much does serialising to json cost us when it's just for signalling?

[simbleau]

I guess it's a good topic to discuses. Do we want to do this at all?

I am fine with closing this as a "won't do." or as a very low priority task to eventually do.

[garryod]

Feel like we should first write some benchmarks? And see what we determine from there. I've noticed that establishing a connection can take quite a while on Wasm, but don't know why

[simbleau]

Noticed that too, but I figured that was just JavaScript being JavaScript.

[johanhelsing]

iirc, even though we implement ice trickle, there is a theoretically unnecessary wait for ice completion in there. However, in practice nat hole punching doesn't work if we remove it. I'm not sure what that's about, though.

Made an issue to track it: #192

[johanhelsing]

re: binary data -

bincode-org/bincode#629 (comment)

Seems like bincode is wasm compat.

Does that make it the top contender?

Also, re "need to make sure client and server is using the same version of bincode" - i'm not sure that's an issue? Since we have the same problem for serde_json. I'd assume matching those versions is up to the user and how they want their clients to update.

Not the same thing i think. Json will always be json, so it doesn't matter if the client is on another version or not, could even be another language. Unless I'm misunderstanding something.

Bincode needs to be on the same major version and "use the same configuration" or you might not be able to deserialize. Not sure what configuration means exactly. See faq in bincode readme.