Handling many participant joins #170
Conversation
Generated by 🚫 Danger |
| if count < 16 { | ||
| return 0 | ||
| } else if count < 50 { | ||
| return 250_000_000 |
There was a problem hiding this comment.
nit: WDYT about making those configurable via a config? Or at least move them as constants somewhere more prominent as here they feel like magic numbers
There was a problem hiding this comment.
we shouldn't have them configurable, this is an internal thing. Moving them to constants makes sense, will do that.
| @@ -105,6 +105,8 @@ public class VideoRenderer: RTCMTLVideoView { | |||
| self.track?.trackId | |||
| } | |||
|
|
|||
| private var viewSize: CGSize? | |||
There was a problem hiding this comment.
| private var viewSize: CGSize? | |
| private var trackSize: CGSize = .zero | |
| private lazy var scale: CGFloat = UIScreen.main.scale |
There was a problem hiding this comment.
It's actually the view size, not the track size, so the naming is correct. Good point about the scale, will move it here.
Btw, should be optional, because we have a default track size - we should change it only when we know the view size.
| public override func layoutSubviews() { | ||
| super.layoutSubviews() | ||
| let scale = UIScreen.main.scale | ||
| self.viewSize = CGSize( | ||
| width: self.bounds.size.width * scale, | ||
| height: self.bounds.size.height * scale | ||
| ) | ||
| } | ||
|
|
There was a problem hiding this comment.
| public override func layoutSubviews() { | |
| super.layoutSubviews() | |
| let scale = UIScreen.main.scale | |
| self.viewSize = CGSize( | |
| width: self.bounds.size.width * scale, | |
| height: self.bounds.size.height * scale | |
| ) | |
| } | |
| public override func layoutSubviews() { | |
| super.layoutSubviews() | |
| trackSize = CGSize( | |
| width: self.bounds.size.width * scale, | |
| height: self.bounds.size.height * scale | |
| ) | |
| } | |
| if let viewSize, prev != viewSize { | ||
| onTrackSizeUpdate(viewSize, participant) |
There was a problem hiding this comment.
| if let viewSize, prev != viewSize { | |
| onTrackSizeUpdate(viewSize, participant) | |
| if let trackSize, prev != viewSize { | |
| onTrackSizeUpdate(trackSize, participant) |
| @@ -171,15 +182,11 @@ extension VideoRenderer { | |||
| if let track = participant.track { | |||
| log.debug("adding track to a view \(self)") | |||
| self.add(track: track) | |||
| DispatchQueue.main.asyncAfter(deadline: .now() + 0.01) { | |||
| DispatchQueue.global(qos: .userInteractive).asyncAfter(deadline: .now() + 0.01) { [weak self] in | |||
There was a problem hiding this comment.
Why not update async without a delay?
There was a problem hiding this comment.
I can't remember now, but there were issues when it was done straight away. Should've put a comment back then. 😞
| @@ -586,6 +586,10 @@ open class CallViewModel: ObservableObject { | |||
| } | |||
| } | |||
| } else if let participantEvent = callEventsHandler.checkForParticipantEvents(from: event) { | |||
| guard participants.count < 25 else { | |||
There was a problem hiding this comment.
Are we completely skipping them? Would that have any unwanted effects on UI? (e.g. participants list count not updating)
There was a problem hiding this comment.
For now yes, because it is actually not usable nor correct. We need to re-think this (sth like "User and 10 more joined"), but not for the scope of this PR.
|
Kudos, SonarCloud Quality Gate passed! |
🔗 Issue Links
Provide all JIRA tickets and/or GitHub issues related to this PR, if applicable.
🎯 Goal
Describe why we are making this change.
📝 Summary
Provide bullet points with the most important changes in the codebase.
🛠 Implementation
Provide a detailed description of the implementation and explain your decisions if you find them relevant.
🎨 Showcase
Add relevant screenshots and/or videos/gifs to easily see what this PR changes, if applicable.
imgimg🧪 Manual Testing Notes
Explain how this change can be tested manually, if applicable.
☑️ Contributor Checklist
🎁 Meme
Provide a funny gif or image that relates to your work on this pull request. (Optional)