flatbuffers

[jmillan]

WIP to remove JSON in favour of flatbuffers for as the worker communication format.

• JSON parsing is CPU intensive.
• JSON is not type safe.
• flatbuffers does not parse the buffer. Reading the message takes 0ms.
• flatbuffers is type safe since code (C++, Typescript, Rust, etc) is autogenerated out of the schema definitions.

Extras that come along with this changes:

• There is now a single Channel for worker communication.
  • Previously there were 2 (Channel and PayloadChannel)
• Each message sent Node->Worker requires a single write() call to Channel.
  • Previously 2 calls per message were needed for Channel and 4 for PayloadChannel.
• Each message sent Worker->Node previously executed two extra calls to memcpy that are not executed now.

TODO: flatbuffers related

• Try to scope notifications per instance so we don't rely on a global flatbuffers notification event enum but an enum per class (producer, consumer, transport, etc.): FBS: WIP, scope notifications #1162 (non blocker)
• Do not use heap on flatbuffers generation (non blocker)
• Do not use head on flatbuffers processing (non blocker)

TODO: flatbuffers unrealed. Changes made in flatbuffer branch, not present in v3

• Document new TransportListenInfo in the website (PR [flatbuffers] Add ListenInfo in all transports and send/recv buffer size options #1084).
• Document new rtcpListenInfo in PlainTransportOptions once PR PlainTransportOptions: add optional rtcpListenInfo #1099 is merged here.
• Document the new pause/resume feature in DataProducer and DataConsumer (PR [flatbuffers] Add pause() and resume() in DataProducer and DataConsumer #1104):
  • pause()and resume() in DataProducer and DataConsumer.
  • paused getter in DataProducer and DataConsumer.
  • dataproducerpause and dataproducerresume events in DataConsumer.
  • pause and resume events in DataProducer.observer and DataConsumer.observer.
  • paused?: boolean in transport.consumeData() and transport.produceData().
• Document the new subchannels stuff (PR DataChannel subchannels #1152).

[ibc]

@jmillan this branch uses NPM flatbuffers 23.3.3 and there is 23.5.9 already. CHANGELOG here: https://github.com/google/flatbuffers/blob/master/CHANGELOG.md

I'm merging v3 into flatbuffers and will later upgrade this dep.

[ibc]

@jmillan this branch uses NPM flatbuffers 23.3.3 and there is 23.5.9 already. CHANGELOG here: https://github.com/google/flatbuffers/blob/master/CHANGELOG.md

I'm merging v3 into flatbuffers and will later upgrade this dep.

Done and pushed.

[ibc]

Questions. Sometimes I see this:

if (flatbuffers::IsFieldPresent(options->listenInfo(), FBS::Transport::ListenInfo::VT_ANNOUNCEDIP))

and sometimes I see this (which is way nicer):

if (body->port().has_value())
{
    port = body->port().value();
}

I'd love if the latter could be used everywhere, no idea if possible.

[ibc]

I may be crazy but make lint doesn't detect wrong changes in PlainTransport.cpp although it does in other XxxxTransport.cpp classes...

[ibc]

@jmillan, in the TODO items above:

flatbuffers (TS) should throw TypeError instead of Error when there is a missing required field. PR done.

It's been merged. Not sure if there is a new release of flatbuffers for Node, but if so we should change some code, right?

[jmillan]

flatbuffers (TS) should throw TypeError instead of Error when there is a missing required field. google/flatbuffers#7910.

It's been merged. Not sure if there is a new release of flatbuffers for Node, but if so we should change some code, right?

Removed explicit TypeError exceptions.

[ibc]

@jmillan @nazar-pc, the most important missing piece here is:

• Build rust/src/fbs.rs at build time (planus) (blocker)

So I'd like to discuss about this.

Problem

Currently, as soon as we change/add/delete any file in worker/fbs folder, we must perform these steps manually in rust directory:

1. We need to have planus-cli installed (cargo install planus-cli).
2. planus rust -o src/fbs.rs ../worker/fbs/*.fbs
3. Then revert some of the changes at the top of rust/src/fbs.rs file to make sure it re-exports things the same way and suppresses clippy warnings.

Questions

1. Does an external contributor have to install planus-cli manually? Or are we gonna add planus-cli in [dev-dependencies] in rust/Cargo.toml?

• Off-topic: How is the cargo command to install dev deps?

2. So assuming we have planus-cli, how are we gonna automatize steps 2 and 3 above? Is this gonna be a new task in main Makefile? Or can we have a custom cargo run planus command (does such a thing exist?) and use cargo-cli as a lib?

[nazar-pc]

1. Neither, we'll use Planus as a library and compile files programmatically from build.rs instead
2. They are installed automatically when you run tests, etc. It is not the same notion as Node.js. For equivalent stuff to Node.js there are [build-dependencies], those are dependencies needed for build.rs
3. We will programmatically compile files as part of build.rs and emit cargo:rerun-if-changed to instruct Cargo to re-run build.rs only if any of the files change (should also include C++ files)

[ibc]

we'll use Planus as a library and compile files programmatically from build.rs instead

• So we have to run cargo build or cargo test to update fbs.rs, right?
• So we have to add ugly code in build.rs to "revert some of the changes at the top of rust/src/fbs.rs file to make sure it re-exports things the same way and suppresses clippy warnings", right?

[nazar-pc]

So we have to run cargo build or cargo test to update fbs.rs, right?

Neither and both. We will no longer ship fbs.rs, it will be compiled automatically when you run cargo test or use mediasoup as a dependency, but as mentioned before, only if it wasn't compiled already or corresponding files have changed.

So we have to add ugly code in build.rs to "revert some of the changes at the top of rust/src/fbs.rs file to make sure it re-exports things the same way and suppresses clippy warnings", right?

I believe we do not, we can just suppress warnings on the whole module. Basically we will have #[allow(clippy::whatever)] mod fbs {included_code_here} or something like that in lib.rs, no manual search/replace will be necessary in build.rs.

[ibc]

I'm writing all missing docs in versatica/mediasoup-website#50

[ibc]

@jmillan I'm marking this PR as ready.