-
Notifications
You must be signed in to change notification settings - Fork 140
Expose participant's permission to ffi layer #824
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -153,6 +153,10 @@ pub enum RoomEvent { | |
| participant: Participant, | ||
| is_encrypted: bool, | ||
| }, | ||
| ParticipantPermissionChanged { | ||
| participant: Participant, | ||
| permission: Option<proto::ParticipantPermission>, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same concern here
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had the same concerns from the beginning and have been struggling with this decision. The root cause is that the protocol layer uses proto3 syntax, which makes the generated nested structs optional by default. Even though we know in practice it will never be empty (and we can get the semantic default value even if it's empty in the proto3 world), after careful consideration, I decided to make the permission field optional in the I completely agree that it should be required in the BTW, I made a new commit to initialize the permission field(It should be discovered earlier if choose required instead, LOL) |
||
| }, | ||
| ActiveSpeakersChanged { | ||
| speakers: Vec<Participant>, | ||
| }, | ||
|
|
@@ -505,6 +509,7 @@ impl Room { | |
| pi.metadata, | ||
| pi.attributes, | ||
| e2ee_manager.encryption_type(), | ||
| pi.permission, | ||
| ); | ||
|
|
||
| let dispatcher = Dispatcher::<RoomEvent>::default(); | ||
|
|
@@ -580,6 +585,14 @@ impl Room { | |
| } | ||
| }); | ||
|
|
||
| local_participant.on_permission_changed({ | ||
| let dispatcher = dispatcher.clone(); | ||
| move |participant, permission| { | ||
| let event = RoomEvent::ParticipantPermissionChanged { participant, permission }; | ||
| dispatcher.dispatch(&event); | ||
| } | ||
| }); | ||
|
|
||
| let (incoming_stream_manager, open_rx) = IncomingStreamManager::new(); | ||
| let (outgoing_stream_manager, packet_rx) = OutgoingStreamManager::new(); | ||
|
|
||
|
|
@@ -649,6 +662,7 @@ impl Room { | |
| pi.name, | ||
| pi.metadata, | ||
| pi.attributes, | ||
| pi.permission, | ||
| ) | ||
| }; | ||
| participant.update_info(pi.clone()); | ||
|
|
@@ -1011,6 +1025,7 @@ impl RoomSession { | |
| pi.name, | ||
| pi.metadata, | ||
| pi.attributes, | ||
| pi.permission, | ||
| ) | ||
| }; | ||
|
|
||
|
|
@@ -1616,6 +1631,7 @@ impl RoomSession { | |
| name: String, | ||
| metadata: String, | ||
| attributes: HashMap<String, String>, | ||
| permission: Option<proto::ParticipantPermission>, | ||
| ) -> RemoteParticipant { | ||
| let participant = RemoteParticipant::new( | ||
| self.rtc_engine.clone(), | ||
|
|
@@ -1627,6 +1643,7 @@ impl RoomSession { | |
| metadata, | ||
| attributes, | ||
| self.options.auto_subscribe, | ||
| permission, | ||
| ); | ||
|
|
||
| participant.on_track_published({ | ||
|
|
@@ -1724,6 +1741,14 @@ impl RoomSession { | |
| } | ||
| }); | ||
|
|
||
| participant.on_permission_changed({ | ||
| let dispatcher = self.dispatcher.clone(); | ||
| move |participant, permission| { | ||
| let event = RoomEvent::ParticipantPermissionChanged { participant, permission }; | ||
| dispatcher.dispatch(&event); | ||
| } | ||
| }); | ||
|
|
||
| participant.on_encryption_status_changed({ | ||
| let dispatcher = self.dispatcher.clone(); | ||
| move |participant, is_encrypted| { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be optional? seems like it should always be there