Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@
import kotlinx.coroutines.flow.flow
import kotlinx.coroutines.flow.flowOn
import kotlinx.coroutines.flow.retryWhen
import kotlinx.coroutines.flow.update
import kotlinx.coroutines.launch
import kotlinx.coroutines.sync.Mutex
import kotlinx.coroutines.sync.withLock
Expand Down Expand Up @@ -172,6 +173,7 @@
import java.io.InterruptedIOException
import java.net.SocketTimeoutException
import java.util.Collections
import java.util.concurrent.atomic.AtomicBoolean
import java.util.concurrent.atomic.AtomicInteger
/**
* Keeps track of which track is being rendered at what resolution.
Expand Down Expand Up @@ -326,7 +328,14 @@
)
},
) {
private var muteStateSyncJob: Job? = null
private val muteStateSyncJobs = TrackKeyedJobs()

/**
* When false, [setMuteState] still records the latest desired mute bits locally but does
* not POST [UpdateMuteStatesRequest] — used while this session's SFU is being torn down
* for reconnect/migration so a media collector cannot target the old connection.
*/
private val muteSyncEnabled = AtomicBoolean(true)
private val oneBasedSessionCounter = sessionCounter + 1

/**
Expand Down Expand Up @@ -574,8 +583,8 @@
private val connectionConfiguration: PeerConnection.RTCConfiguration
get() = buildConnectionConfiguration(iceServers)

internal val subscriber: MutableStateFlow<Subscriber?> = MutableStateFlow(null)

Check warning on line 586 in stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/call/RtcSession.kt

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Don't expose mutable flow types.

See more on https://sonarcloud.io/project/issues?id=GetStream_stream-video-android&issues=AaBdcmJuXrJvphm2MnE4&open=AaBdcmJuXrJvphm2MnE4&pullRequest=1796
internal val publisher: MutableStateFlow<Publisher?> = MutableStateFlow(null)

Check warning on line 587 in stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/call/RtcSession.kt

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Don't expose mutable flow types.

See more on https://sonarcloud.io/project/issues?id=GetStream_stream-video-android&issues=AaBdcmJuXrJvphm2MnE5&open=AaBdcmJuXrJvphm2MnE5&pullRequest=1796

internal lateinit var sfuConnectionModule: SfuConnectionModule

Expand Down Expand Up @@ -908,7 +917,7 @@
message = "Use connectInternal() which returns SfuConnectionResult instead of throwing.",
replaceWith = ReplaceWith("connectInternal(reconnectDetails, options)"),
)
suspend fun connect(

Check warning on line 920 in stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/call/RtcSession.kt

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Do not forget to remove this deprecated code someday.

See more on https://sonarcloud.io/project/issues?id=GetStream_stream-video-android&issues=AaBdcmJuXrJvphm2MnE6&open=AaBdcmJuXrJvphm2MnE6&pullRequest=1796
reconnectDetails: ReconnectDetails? = null,
options: List<PublishOption>? = null,
) {
Expand Down Expand Up @@ -1195,6 +1204,7 @@

private suspend fun connectRtc() {
logger.d { "[connectRtc] #sfu; #track; no args" }
resumeMuteSync()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resumeMuteSync() launches one UpdateMuteStates per recorded track, then listenToMediaChanges() two lines down re-collects the camera/mic/screenshare StateFlows — collectLatest replays on re-subscription, so each replay reaches syncMuteStateToSfu for the same TrackType and cancels the flush job launched microseconds earlier.

So every reconnect gets a redundant in-flight RPC per track, cancelled mid-flight, logging the IOException: Canceled quoted in the Goal section. Not a correctness bug — the winning job carries the same value — but it puts the line back in customer logs.

Reordering won't help; the collectors launch on mediaScope and the interleaving is nondeterministic either way. Recording which tracks were actually deferred while paused and flushing only those avoids it, and also stops the flush posting for tracks that were never published when publisher.value == null.

// step 6 - onNegotiationNeeded will trigger and complete the setup using SetPublisherRequest
publisher.value?.let {
listenToMediaChanges()
Expand Down Expand Up @@ -1289,6 +1299,8 @@
}

mediaScope.cancel()
muteSyncEnabled.set(false)
muteStateSyncJobs.cancelAll()

// cleanup all non-local tracks
supervisorJob.cancel()
Expand Down Expand Up @@ -1327,23 +1339,37 @@
* matches the web SDK, which only signals the track(s) that actually changed. On SFU
* (re)connect/migration each enabled track is re-signalled individually via
* [listenToMediaChanges], so the full state is still restored.
*
* Sync jobs are keyed by [trackType]. Cancelling a shared job used to drop an in-flight
* audio unmute when video (or screen-share) published a moment later — reconnect restarts
* [listenToMediaChanges] and fires those collectors together.
*
* The local mute map is updated with [MutableStateFlow.update] so concurrent collectors
* cannot lose another track's bit via a stale read–copy–write.
*
* During reconnect/migration, [cancelActiveWork] pauses SFU sync so collectors can keep
* recording the latest desired bits without posting to a stale connection. Sync resumes
* from [connectRtc] once the active SFU is ready.
*/
private fun setMuteState(isEnabled: Boolean, trackType: TrackType) {
logger.d { "[setPublishState] #sfu; $trackType isEnabled: $isEnabled" }

// update the local copy
val copy = muteState.value.toMutableMap()
copy[trackType] = isEnabled
val new = copy.toMap()
muteState.value = new
muteState.update { it + (trackType to isEnabled) }
syncMuteStateToSfu(trackType, isEnabled)
}

private fun syncMuteStateToSfu(trackType: TrackType, isEnabled: Boolean) {
if (!muteSyncEnabled.get()) {
logger.d {
"[syncMuteStateToSfu] deferred until SFU is ready; $trackType isEnabled: $isEnabled"
}
return
}

val currentSfu = sfuUrl
// prevent running multiple of these at the same time
// if there's already a job active. cancel it
muteStateSyncJob?.cancel()
// start a new job
// this code is a bit more complicated due to the retry behaviour
muteStateSyncJob = coroutineScope.launch {
// Coalesce retries for this track only. Other tracks keep their in-flight RPCs.
muteStateSyncJobs.launch(coroutineScope, trackType) {
Comment thread
coderabbitai[bot] marked this conversation as resolved.
if (!muteSyncEnabled.get()) return@launch
flow {
val request = UpdateMuteStatesRequest(
session_id = sessionId,
Expand All @@ -1358,7 +1384,8 @@
emit(response)
}.flowOn(DispatcherProvider.IO).retryWhen { cause, attempt ->
if (cause is SessionFatalException) return@retryWhen false
val sameValue = new == muteState.value
if (!muteSyncEnabled.get()) return@retryWhen false
val sameValue = muteState.value[trackType] == isEnabled
val sameSfu = currentSfu == sfuUrl
val isPermanent = isPermanentError(cause)
val willRetry = !isPermanent && sameValue && sameSfu && attempt < 30
Expand All @@ -1372,6 +1399,14 @@
}
}

private fun resumeMuteSync() {
if (!muteSyncEnabled.compareAndSet(false, true)) return
logger.d { "[resumeMuteSync] flushing latest mute state to the active SFU" }
muteState.value.forEach { (trackType, isEnabled) ->
syncMuteStateToSfu(trackType, isEnabled)
}
}

private fun isPermanentError(cause: Throwable): Boolean {
return false
}
Expand Down Expand Up @@ -1606,7 +1641,7 @@
paused = false,
)

if (event.trackType == TrackType.TRACK_TYPE_AUDIO) {

Check warning on line 1644 in stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/call/RtcSession.kt

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Merge this "if" statement with the nested one.

See more on https://sonarcloud.io/project/issues?id=GetStream_stream-video-android&issues=AaBdcmJuXrJvphm2MnE8&open=AaBdcmJuXrJvphm2MnE8&pullRequest=1796
if (event.sessionId == sessionId) {
val isMicDisabled = !call.mediaManager.microphone.isEnabled.value
if (isMicDisabled) {
Expand Down Expand Up @@ -2107,8 +2142,8 @@
if (cancelEventJob) eventJob?.cancel()
iceMonitoringJob?.cancel()
iceMonitoringJob = null
muteStateSyncJob?.cancel()
muteStateSyncJob = null
muteSyncEnabled.set(false)
muteStateSyncJobs.cancelAll()
Comment thread
PratimMallick marked this conversation as resolved.
participantsMonitoringJob?.cancel()
participantsMonitoringJob = null
serialProcessor.stop()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/*
* Copyright (c) 2014-2026 Stream.io Inc. All rights reserved.
*
* Licensed under the Stream License;
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://github.com/GetStream/stream-video-android/blob/main/LICENSE
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package io.getstream.video.android.core.call

import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.CoroutineStart
import kotlinx.coroutines.Job
import kotlinx.coroutines.launch
import stream.video.sfu.models.TrackType
import java.util.concurrent.ConcurrentHashMap

/**
* One job per [TrackType]. Replacing a job cancels only that track's previous work, so an
* in-flight audio mute RPC is not dropped when video or screen-share sync starts.
*/
internal class TrackKeyedJobs {
private val jobs = ConcurrentHashMap<TrackType, Job>()

fun launch(
scope: CoroutineScope,
trackType: TrackType,
block: suspend CoroutineScope.() -> Unit,
) {
val job = scope.launch(start = CoroutineStart.LAZY, block = block)
jobs.put(trackType, job)?.cancel()
job.start()
}

fun cancelAll() {
jobs.values.forEach { it.cancel() }
jobs.clear()
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
/*
* Copyright (c) 2014-2026 Stream.io Inc. All rights reserved.
*
* Licensed under the Stream License;
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://github.com/GetStream/stream-video-android/blob/main/LICENSE
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package io.getstream.video.android.core.call

import com.google.common.truth.Truth.assertThat
import kotlinx.coroutines.CompletableDeferred
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Job
import kotlinx.coroutines.test.UnconfinedTestDispatcher
import kotlinx.coroutines.test.runTest
import org.junit.Test
import stream.video.sfu.models.TrackType
import java.util.concurrent.atomic.AtomicBoolean

class TrackKeyedJobsTest {

@Test
fun `launching a second track does not cancel the first track's job`() = runTest {
val scope = CoroutineScope(UnconfinedTestDispatcher(testScheduler) + Job())
val jobs = TrackKeyedJobs()
val release = CompletableDeferred<Unit>()
val audioDone = AtomicBoolean(false)
val videoDone = AtomicBoolean(false)

try {
jobs.launch(scope, TrackType.TRACK_TYPE_AUDIO) {
release.await()
audioDone.set(true)
}
jobs.launch(scope, TrackType.TRACK_TYPE_VIDEO) {
release.await()
videoDone.set(true)
}

release.complete(Unit)

assertThat(audioDone.get()).isTrue()
assertThat(videoDone.get()).isTrue()
} finally {
scope.coroutineContext[Job]?.cancel()
}
}

@Test
fun `launching the same track cancels the previous job`() = runTest {
val scope = CoroutineScope(UnconfinedTestDispatcher(testScheduler) + Job())
val jobs = TrackKeyedJobs()
val firstHold = CompletableDeferred<Unit>()
val firstDone = AtomicBoolean(false)
val secondDone = AtomicBoolean(false)

try {
jobs.launch(scope, TrackType.TRACK_TYPE_AUDIO) {
firstHold.await()
firstDone.set(true)
}
jobs.launch(scope, TrackType.TRACK_TYPE_AUDIO) {
secondDone.set(true)
}
firstHold.complete(Unit)

assertThat(firstDone.get()).isFalse()
assertThat(secondDone.get()).isTrue()
} finally {
scope.coroutineContext[Job]?.cancel()
}
}

@Test
fun `cancelAll cancels every track job and clears the map`() = runTest {
val scope = CoroutineScope(UnconfinedTestDispatcher(testScheduler) + Job())
val jobs = TrackKeyedJobs()
val audioHold = CompletableDeferred<Unit>()
val videoHold = CompletableDeferred<Unit>()
val audioDone = AtomicBoolean(false)
val videoDone = AtomicBoolean(false)

try {
jobs.launch(scope, TrackType.TRACK_TYPE_AUDIO) {
audioHold.await()
audioDone.set(true)
}
jobs.launch(scope, TrackType.TRACK_TYPE_VIDEO) {
videoHold.await()
videoDone.set(true)
}

jobs.cancelAll()
audioHold.complete(Unit)
videoHold.complete(Unit)

assertThat(audioDone.get()).isFalse()
assertThat(videoDone.get()).isFalse()
} finally {
scope.coroutineContext[Job]?.cancel()
}
}
}
Loading
Loading