SubscribedQualityUpdate message #270
Conversation
qualities for a simulcast video publisher
| "github.com/livekit/protocol/logger" | ||
| "github.com/livekit/protocol/utils" | ||
| "github.com/livekit/protocol/webhook" | ||
| "github.com/pkg/errors" |
| secondServerPort = 8880 | ||
| nodeID1 = "node-1" | ||
| nodeID2 = "node-2" | ||
| secondServerPort = 8880 |
| @@ -1,3 +1,4 @@ | |||
| //go:build tools | |||
| delete(t.maxSubscriberQuality, subscriberID) | ||
| t.maxQualityLock.Unlock() | ||
|
|
||
| t.updateQualityChange() |
There was a problem hiding this comment.
Wanted to call this from here (and from the routine below) in a go routine, but had a couple of issues
- Ordering
- Tests. I tried writing tests with the use of goroutine and sync.WaitGroup, but just could not get it to work.
There was a problem hiding this comment.
Are you wanting to do so for purposes of deadlocks? The test challenges might be related to the need to lock within the goroutines spun up for testing?
There was a problem hiding this comment.
Yeah mostly for locking purposes. We are not locking here while doing the callback. So, I think we should be fine. I was also generally concerned about the callback having a long call chain and causing problems
pkg/rtc/mediatrack.go
Outdated
| allSubscribersMuted = true | ||
| } else { | ||
| for _, maxQuality := range t.maxSubscriberQuality { | ||
| switch maxSubscribedQuality { |
There was a problem hiding this comment.
I think we are trying to get the max quality from the values in the map? how about just doing the following?
for _, subQuality := range t.maxSubscriberQuality {
if subQuality > maxSubscribedQuality {
maxSubscribedQuality = subQuality
}
}
There was a problem hiding this comment.
Oh yeah, they are just integers in the end. Will change. Because it is enum, my natural tendency was to think of them as not integers :-)
There was a problem hiding this comment.
Addressed in this commit - 70d443d
|
Thinking out loud, in future optimizations we might want to limit how frequently we are changing the max subscribed layer to clients. i.e. if a group of 3 participants are speaking in a large meeting, we probably don't want to turn off publishing of HQ layers immediately after the larger, speaker view is displaying someone else. Perhaps some sort of a ramp off time before we turn off the higher layers. |
Yeah, good point. I thought about it too. Could not think of the right balance. Will file an issue and do some more thinking |
SubscribedQualityUpdate message to send list of currently subscribed qualities for a simulcast video publisher