-
Notifications
You must be signed in to change notification settings - Fork 349
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
fix: Fix audio/video limits since muted stated was moved to SourceInfo. #1090
Conversation
import org.jitsi.utils.OrderedJsonObject | ||
import org.jitsi.utils.logging2.Logger | ||
import org.jitsi.utils.logging2.createChildLogger | ||
import org.jitsi.xmpp.extensions.jitsimeet.AudioMutedExtension | ||
import org.jitsi.xmpp.extensions.jitsimeet.FeaturesExtension |
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.
Does it matter what jigasi is sending? As It still uses those extensions?
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #1090 +/- ##
============================================
+ Coverage 27.58% 27.68% +0.09%
- Complexity 493 494 +1
============================================
Files 129 129
Lines 7728 7748 +20
Branches 1087 1097 +10
============================================
+ Hits 2132 2145 +13
- Misses 5323 5329 +6
- Partials 273 274 +1
Continue to review full report in Codecov by Sentry.
|
@@ -175,22 +188,6 @@ class ChatRoomMemberImpl( | |||
presence.getExtension(StatsId::class.java)?.let { | |||
statsId = it.statsId | |||
} | |||
|
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 we not leave this in if the endpoint doesn't support source-names ?
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.
Do endpoints with source name support always include SourceInfo
(even when they don't have any tracks)?
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.
Nope, SourceInfo gets added only if there are local tracks.
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.
Sorry, just realized that we send an empty SourceInfo on join if the endpoint doesn't have any local tracks.
"<SourceInfo>{}</SourceInfo>"
/** The information about a source contained in a jitsi-meet SourceInfo extension. */ | ||
data class SourceInfo( | ||
val name: String, | ||
val muted: Boolean, | ||
val videoType: VideoType? | ||
) | ||
) { | ||
val mediaType: MediaType? = name.indexOf('-').let { indexOfDash -> |
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.
I'd rather we signal this explicitly rather than implicitly as here, though that can potentially be a separate PR (and would require client changes).
No description provided.