davidliu
Committed by GitHub

Fix ConcurrentModificationException in RemoteVideoTrack (#413)

@@ -23,7 +23,11 @@ import io.livekit.android.events.TrackEvent @@ -23,7 +23,11 @@ import io.livekit.android.events.TrackEvent
23 import io.livekit.android.room.track.video.VideoSinkVisibility 23 import io.livekit.android.room.track.video.VideoSinkVisibility
24 import io.livekit.android.room.track.video.ViewVisibility 24 import io.livekit.android.room.track.video.ViewVisibility
25 import io.livekit.android.util.LKLog 25 import io.livekit.android.util.LKLog
26 -import kotlinx.coroutines.* 26 +import kotlinx.coroutines.CoroutineDispatcher
  27 +import kotlinx.coroutines.CoroutineScope
  28 +import kotlinx.coroutines.SupervisorJob
  29 +import kotlinx.coroutines.cancel
  30 +import kotlinx.coroutines.launch
27 import livekit.org.webrtc.RtpReceiver 31 import livekit.org.webrtc.RtpReceiver
28 import livekit.org.webrtc.VideoSink 32 import livekit.org.webrtc.VideoSink
29 import javax.inject.Named 33 import javax.inject.Named
@@ -40,7 +44,8 @@ class RemoteVideoTrack( @@ -40,7 +44,8 @@ class RemoteVideoTrack(
40 44
41 private var coroutineScope = CoroutineScope(dispatcher + SupervisorJob()) 45 private var coroutineScope = CoroutineScope(dispatcher + SupervisorJob())
42 private val sinkVisibilityMap = mutableMapOf<VideoSink, VideoSinkVisibility>() 46 private val sinkVisibilityMap = mutableMapOf<VideoSink, VideoSinkVisibility>()
43 - private val visibilities = sinkVisibilityMap.values 47 + private val visibilities
  48 + get() = sinkVisibilityMap.values
44 49
45 /** 50 /**
46 * @suppress 51 * @suppress
@@ -80,7 +85,9 @@ class RemoteVideoTrack( @@ -80,7 +85,9 @@ class RemoteVideoTrack(
80 fun addRenderer(renderer: VideoSink, visibility: VideoSinkVisibility) { 85 fun addRenderer(renderer: VideoSink, visibility: VideoSinkVisibility) {
81 super.addRenderer(renderer) 86 super.addRenderer(renderer)
82 if (autoManageVideo) { 87 if (autoManageVideo) {
83 - sinkVisibilityMap[renderer] = visibility 88 + synchronized(sinkVisibilityMap) {
  89 + sinkVisibilityMap[renderer] = visibility
  90 + }
84 visibility.addObserver { _, _ -> recalculateVisibility() } 91 visibility.addObserver { _, _ -> recalculateVisibility() }
85 recalculateVisibility() 92 recalculateVisibility()
86 } else { 93 } else {
@@ -90,7 +97,11 @@ class RemoteVideoTrack( @@ -90,7 +97,11 @@ class RemoteVideoTrack(
90 97
91 override fun removeRenderer(renderer: VideoSink) { 98 override fun removeRenderer(renderer: VideoSink) {
92 super.removeRenderer(renderer) 99 super.removeRenderer(renderer)
93 - val visibility = sinkVisibilityMap.remove(renderer) 100 +
  101 + val visibility = synchronized(sinkVisibilityMap) {
  102 + sinkVisibilityMap.remove(renderer)
  103 + }
  104 +
94 visibility?.close() 105 visibility?.close()
95 if (autoManageVideo && visibility != null) { 106 if (autoManageVideo && visibility != null) {
96 recalculateVisibility() 107 recalculateVisibility()
@@ -99,29 +110,40 @@ class RemoteVideoTrack( @@ -99,29 +110,40 @@ class RemoteVideoTrack(
99 110
100 override fun stop() { 111 override fun stop() {
101 super.stop() 112 super.stop()
102 - sinkVisibilityMap.values.forEach { it.close() }  
103 - sinkVisibilityMap.clear() 113 + synchronized(sinkVisibilityMap) {
  114 + sinkVisibilityMap.values.forEach { it.close() }
  115 + sinkVisibilityMap.clear()
  116 + }
104 } 117 }
105 118
106 private fun hasVisibleSinks(): Boolean { 119 private fun hasVisibleSinks(): Boolean {
107 - return visibilities.any { it.isVisible() } 120 + synchronized(sinkVisibilityMap) {
  121 + return visibilities.any { it.isVisible() }
  122 + }
108 } 123 }
109 124
110 private fun largestVideoViewSize(): Dimensions { 125 private fun largestVideoViewSize(): Dimensions {
111 var maxWidth = 0 126 var maxWidth = 0
112 var maxHeight = 0 127 var maxHeight = 0
113 - visibilities.forEach { visibility ->  
114 - val size = visibility.size()  
115 - maxWidth = max(maxWidth, size.width)  
116 - maxHeight = max(maxHeight, size.height)  
117 - }  
118 128
  129 + synchronized(sinkVisibilityMap) {
  130 + visibilities.forEach { visibility ->
  131 + val size = visibility.size()
  132 + maxWidth = max(maxWidth, size.width)
  133 + maxHeight = max(maxHeight, size.height)
  134 + }
  135 + }
119 return Dimensions(maxWidth, maxHeight) 136 return Dimensions(maxWidth, maxHeight)
120 } 137 }
121 138
122 private fun recalculateVisibility() { 139 private fun recalculateVisibility() {
123 - val isVisible = hasVisibleSinks()  
124 - val newDimensions = largestVideoViewSize() 140 + var isVisible: Boolean
  141 + var newDimensions: Dimensions
  142 +
  143 + synchronized(sinkVisibilityMap) {
  144 + isVisible = hasVisibleSinks()
  145 + newDimensions = largestVideoViewSize()
  146 + }
125 147
126 val eventsToPost = mutableListOf<TrackEvent>() 148 val eventsToPost = mutableListOf<TrackEvent>()
127 if (isVisible != lastVisibility) { 149 if (isVisible != lastVisibility) {