Monitor visitors' codec lists, and aggregate them into a conference property. #1137
Conversation
| { | ||
| visitorCount.adjustValue(-1); | ||
| if (codecs != null) { | ||
| visitorCodecs.removePreference(codecs); |
There was a problem hiding this comment.
We allow codecs to change from null to a non-null, so if a visitor joins with no codecs and then adds some it would mess with the count. It's a bit of a stretch, but one could trick jicofo into upgrading to av1 thus breaking video for some participants.
Maybe make ChatRoomMemberImpl.videoCodecs a lazy val and initialize it with the first presence received?
Unless I misunderstand how the aggregator works
| fun removePreference(prefs: List<String>) { | ||
| synchronized(lock) { | ||
| count-- | ||
| check(count >= 0) { |
There was a problem hiding this comment.
This could also be force to fail by sending initial presence with no codecs, then updating with some codecs, then leaving repeatedly
| } ?: // Older clients sent a single codec in codecType rather than all supported ones in codecList | ||
| presence.getExtensionElement("jitsi_participant_codecType", "jabber:client")?.let { | ||
| if (it is StandardExtensionElement) { | ||
| videoCodecs = if (it.text == "vp8") { |
There was a problem hiding this comment.
In JitsiParticipantCodecList.kt you use lowercase(). Is it also required here?
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1137 +/- ##
============================================
+ Coverage 28.73% 29.41% +0.68%
- Complexity 467 485 +18
============================================
Files 129 130 +1
Lines 7747 7863 +116
Branches 1059 1081 +22
============================================
+ Hits 2226 2313 +87
- Misses 5254 5277 +23
- Partials 267 273 +6
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Depends on jitsi/jitsi-xmpp-extensions#104.