-
Notifications
You must be signed in to change notification settings - Fork 122
fix: emit track-muted event for local tracks #544
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
base: main
Are you sure you want to change the base?
Conversation
|
it seems like you haven't added any nanpa changeset files to this PR. if this pull request includes changes to code, make sure to add a changeset, by writing a file to refer to the manpage for more information. |
36905ef to
42bd9c5
Compare
42bd9c5 to
d8bb6e2
Compare
d8bb6e2 to
abc6ed5
Compare
702f3ff to
bf56220
Compare
|
@davidzhao @typester any progress with this pr? :) |
bf56220 to
c73496b
Compare
|
Running into the same issue. Any chance this can be moved forward? |
|
Hi @sandr01d, I will look into this. |
c73496b to
6dd9315
Compare
6dd9315 to
25ec35c
Compare
25ec35c to
4f85f7a
Compare
|
Thanks @davidzhao for looping me in. Looking at the code, I think it is doing the right thing, the existing Rust code assumes the the local participant does not need the event as the mute happen starting from the UIs, thus they don't need an event to notify them back. Think about it for two use cases, if users update the UI to mute / unmute, then will get the redundant events, but that will be fine, since apps should not nothing if they check the cached value is the same as the mute value in the event, then do nothing. the PR looks good to me in general. That says, I am pretty new to the our Rust SDK, please let me know if I miss anything. @ladvoc, do you have any concern of landing it ? BTW, thanks for the PR and sorry for the length review process. |
|
The code looks good, but I need to confirm whether https://github.com/livekit/rust-sdks/blob/main/livekit/src/room/track/mod.rs#L203-L209 |
ladvoc
left a comment
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.
Looks good to me, see one comment.
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.
This repository no longer uses the nanpa release manager, so this file should be removed before merging.
|
I just created another fix and verified it. |
Currently there's no way to know when a track was muted by a moderator since track changes from the server were ignored.
This updates the TrackInfo of the local participant in a similar way, to how its done for remote participants.
While this seems to work, I'm not sure if this is the correct way to do this, since the comment in https://github.com/livekit/rust-sdks/blob/main/livekit/src/room/publication/remote.rs#L157 seems to imply that this should already work for local publications?