FBS: WIP, scope notifications #1162
Conversation
# Conflicts: # worker/Makefile
# Conflicts: # worker/src/RTC/RtpStream.cpp
# Conflicts: # .github/workflows/mediasoup-node.yaml # .gitignore # CHANGELOG.md # npm-scripts.js # worker/src/lib.cpp
# Conflicts: # CHANGELOG.md # npm-scripts.mjs # package.json
# Conflicts: # npm-scripts.mjs
Already thown by flatbuffers.
# Conflicts: # CHANGELOG.md
# Conflicts: # CHANGELOG.md
# Conflicts: # CHANGELOG.md
# Conflicts: # worker/src/RTC/RtpStreamRecv.cpp
And enable 'cargo test' workflow in GH actions.
As explained in #1064 (comment) and messages below it
# Conflicts: # CHANGELOG.md # node/src/Router.ts
# Conflicts: # CHANGELOG.md
# Conflicts: # CHANGELOG.md
# Conflicts: # CHANGELOG.md
As Nazar said, for now the process is manual: 1. In `rust` directory run this: `planus rust -o src/fbs.rs ../worker/fbs/*` 2. Then revert some of the changes at the top of the file to make sure it re-exports things the same way and suppresses clippy warnings in there. 3. For ^ to work you might need to install it first with `cargo install planus-cli` (this will download, compile and install binary in Cargo's directory).
fbs: FBS WebRtcTransport --------- Co-authored-by: Iñaki Baz Castillo <ibc@aliax.net>
Rust: port transport methods to FBS
Rust: port transport notifications to FBS
* FBS: normalize FBS types * FBS: format schemas * FBS: type for RtpHeaderExtensionUri (#1161)
Scope notifications to the class to which they are being received. This adds a level of indirection to the overall message but enhances the design of the flatbuffer types and allows exhausing the 'switch' statements in the processing blocks.
jmillan
force-pushed
the
wip-notification-scoping
branch
from
07b43d5
to
62615a4
Compare
| let tuple_fbs = transport::Tuple::try_from(body.tuple().unwrap()).unwrap(); | ||
| let tuple = TransportTuple::from_fbs(&tuple_fbs); | ||
| match notification { | ||
| plain_transport::Notification::Tuple(notification) => { |
There was a problem hiding this comment.
Switch cases only contain the specific PlainTransport Notification.
| }; | ||
|
|
||
| // Get the Notification instance. | ||
| let notification = notification_wrapper |
There was a problem hiding this comment.
Get the PlainTransport Notification out of the wrapper.
| @@ -55,6 +55,7 @@ union Body { | |||
| DataProducer_SendNotification: FBS.DataProducer.SendNotification, | |||
|
|
|||
| // Notifications from worker. | |||
| PlainTransport: FBS.PlainTransport.NotificationWrapper, | |||
There was a problem hiding this comment.
This type (Notification::Body) will only have the respective namespaces (PlainTransport, DirectTransport, Producer, Consumer, etc), not the specific notifications anymore.
| @@ -46,6 +46,17 @@ table GetStatsResponse { | |||
|
|
|||
| // Notifications from Worker. | |||
|
|
|||
| union Notification { | |||
There was a problem hiding this comment.
PlainTransort Notification type.
| Trace: FBS.Transport.TraceNotification, | ||
| } | ||
|
|
||
| table NotificationWrapper { |
There was a problem hiding this comment.
This wrapper is needed since Notification, which is a union cannot be used by itself, it needs to be contained within a table.
| @@ -1188,11 +1188,17 @@ namespace RTC | |||
| auto notification = FBS::PlainTransport::CreateTupleNotification( | |||
| this->shared->channelNotifier->GetBufferBuilder(), tuple); | |||
|
|
|||
| auto notificationWrapper = FBS::PlainTransport::CreateNotificationWrapper( | |||
There was a problem hiding this comment.
Wrap Notification into NotificationWrapper.
|
I like it. |
| .try_into() | ||
| .unwrap(); |
There was a problem hiding this comment.
Why is try_into needed here? Reading just on GitHub this looks a bit confusing.
There was a problem hiding this comment.
This is getting a Notification object out a NotificationRef. Otherwise each field in NotificationRef is accessed via a method with the field name, which returns a result with the value. IMHO it's too cluttered.
There was a problem hiding this comment.
NOTE: We will finally use NotificationRef instead as noted here
Scope notifications to the class to which they are being received.
This adds a level of indirection to the overall message but enhances the design of the flatbuffer types and allows exhausing the 'switch' statements in the processing blocks.