Control Ringing→Active transition using ActiveStateGate and PeerConnection strategies

[rahul-lohra]

Goal

Improve the logic to render RingingState.Active when both of the PeerConnections Publisher's & Subscriber's are connected

Previously

In ringing calls, we transitioned to RingingState.Active based on signaling events—either when the call was accepted by the other participant or when call.join() completed.

JoinAndRing	User	Logic to set RingingState.Active
Yes	Caller	After CallAcceptedEvent
Yes	Callee	After join()
No	Caller	After CallAcceptedEvent
No	Callee	After join()

---

Problem

With the current approach, we transition to RingingState.Active before the publisher and subscriber RTCPeerConnections are fully connected to the SFU.

Since these connections are established asynchronously, this results in a gap where the UI is active but no media packets are flowing yet. Internally, the publisher and subscriber are still in the process of establishing their transport (ICE + DTLS), leading to a few seconds of silence.
Solution: Transition to RingingState.Active will include one more logic that either one of the Publisher or Subscriber should get connected to SFU first with a custimizable internal timeout

---

Proposed Solution

Update the transition logic for RingingState.Active to include an additional condition:

• Transition to RingingState.Active when both the publisher or subscriber reaches PeerConnectionState.CONNECTED, instead of relying solely on signaling events.
• Introduce a configurable internal timeout for 5 seconds to ensure we don’t block indefinitely if neither connection reaches CONNECTED.

JoinAndRing	User	Logic to set RingingState.Active
Yes	Caller	After CallAcceptedEvent & both one publisher & subscriber is connected with SFU
Yes	Callee	After join() & both publisher & subscriber is connected with SFU
No	Caller	After CallAcceptedEvent & both one publisher and subscriber is connected with SFU
No	Callee	After join() & both publisher and subscriber is connected with SFU

private enum class TransitionToRingingStateStrategy {
        NONE,
        ANY_PEER_CONNECTED,
        BOTH_PEER_CONNECTED, // default used
        PUBLISHER_CONNECTED,  
        SUBSCRIBER_CONNECTED, 
        WAIT_FOR_FIRST_PACKET_RECEIVED 
    }

We can use any strategy to experiment, by default we will select BOTH_PEER_CONNECTED

Implementation

Core Changes	Reason
Make Call.session observable	Need to observe publisher's and subscriber's connection state
Make Publisher & Subscriber both observable	Need to observe their connection state
Add new logic transitionToActiveState logic	Need to observe PeerConnection state with timeout before transition to RingingState.Active

internal enum class TransitionToRingingStateStrategy {
    NONE,
    ANY_PEER_CONNECTED,
    BOTH_PEER_CONNECTED,
    PUBLISHER_CONNECTED,
    SUBSCRIBER_CONNECTED,
    FIST_PACKET_RECEIVED,
}

private const val PEER_CONNECTION_OBSERVER_TIMEOUT = 5_000L

internal class ActiveStateGate(
    private val coroutineScope: CoroutineScope,
    private val previousRingingStates: Set<RingingState>,
    private val strategy: TransitionToRingingStateStrategy = TransitionToRingingStateStrategy.BOTH_PEER_CONNECTED,
    private val timeoutMs: Long = PEER_CONNECTION_OBSERVER_TIMEOUT,
) {
    private val logger by taggedLogger("ActiveStateGate")
    private var peerConnectionObserverJob: Job? = null

    internal fun awaitAndTransition(currentRingingState: RingingState, call: Call, onReady: () -> Unit) {
        // check file changes
    }

    private fun observePeerConnection(call: Call, onReady: () -> Unit, strategy: TransitionToRingingStateStrategy) {
        if (peerConnectionObserverJob?.isActive == true) return

        peerConnectionObserverJob = coroutineScope.launch {
            val start = System.currentTimeMillis()

            val result = withTimeoutOrNull(timeoutMs) {
                call.session
                    .filterNotNull()
                    .flatMapLatest { session ->
                        when (strategy) {
                         // check file changes
                         }
                    .first()
            }
            onReady()
        }
    }

    fun cleanup() {...}

🎨 UI Changes

None, just the time to render RingState.Active has increased

Testing

1. Perform audio-call (normal and joinAndRing) and observe that there is almost zero waiting time in audio transfer

Summary by CodeRabbit

Release Notes

• New Features

  • Added peer connection state monitoring during call state transitions with configurable connection strategies (ANY_PEER_CONNECTED or BOTH_PEER_CONNECTED) and automatic 5-second timeout handling.

• Improvements

  • Enhanced call session state management for improved reactivity and reliability during ringing-to-active transitions.
  • Improved session lifecycle handling and state tracking across call operations.

[github-actions]

PR checklist ✅

All required conditions are satisfied:

• Title length is OK (or ignored by label).
• At least one pr: label exists.
• Sections ### Goal, ### Implementation, and ### Testing are filled.

🎉 Great job! This PR is ready for review.

[github-actions]

SDK Size Comparison 📏

SDK	Before	After	Difference	Status
stream-video-android-core	12.00 MB	12.02 MB	0.02 MB	🟢
stream-video-android-ui-xml	5.68 MB	5.68 MB	0.00 MB	🟢
stream-video-android-ui-compose	6.27 MB	6.28 MB	0.02 MB	🟢

[coderabbitai]

Walkthrough

This pull request refactors session and ringing state management by converting RtcSession.subscriber and RtcSession.publisher to StateFlows, updating Call.session to expose a StateFlow, introducing a new ActiveStateTransition class to coordinate ringing-to-active state transitions via WebRTC peer connection observation, and propagating these changes throughout the codebase.

Changes

Cohort / File(s)	Summary
Session State Management Call.kt, call/RtcSession.kt	Converted session and peer components from nullable fields to StateFlows (Call.session → StateFlow<RtcSession?>, RtcSession.subscriber/publisher → MutableStateFlow<...>). Updated all access patterns to use .value dereferencing and adjusted initialization/lifecycle methods accordingly.
Active Ringing State Transition ActiveStateTransition.kt, CallState.kt	Added new ActiveStateTransition class that observes WebRTC peer connection state changes with configurable strategies (ANY\_PEER\_CONNECTED / BOTH\_PEER\_CONNECTED) and 5s timeout. Integrated into CallState to route ringing-to-active transitions through peer connection observation before updating state.
Session Access Path Updates CallStats.kt, StreamVideoClient.kt, notifications/handlers/StreamMediaSessionController.kt	Updated session access throughout the codebase from call.session?.... to call.session.value?.... to accommodate StateFlow-backed session property.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant CallState
    participant ActiveStateTransition
    participant RtcSession
    participant PeerConnection

    Client->>CallState: Update ringing state to Active
    CallState->>ActiveStateTransition: transitionToActiveState(currentRingingState, call)
    activate ActiveStateTransition
    
    alt Is incoming/outgoing call?
        ActiveStateTransition->>RtcSession: Observe subscriber & publisher<br/>state flows
        activate RtcSession
        RtcSession->>PeerConnection: Monitor connection states
        
        alt Strategy: ANY_PEER_CONNECTED
            PeerConnection-->>RtcSession: Any peer reaches CONNECTED
        else Strategy: BOTH_PEER_CONNECTED
            PeerConnection-->>RtcSession: Both reach CONNECTED
        end
        
        alt Within 5s timeout
            RtcSession-->>ActiveStateTransition: State reached
        else Timeout
            RtcSession-->>ActiveStateTransition: Timeout (log & proceed)
        end
        deactivate RtcSession
    end
    
    ActiveStateTransition->>CallState: Invoke transitionToActiveState()
    CallState->>CallState: Set _ringingState to Active
    deactivate ActiveStateTransition

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐰 Hops with glee through state machines,
StateFlows dance in racing scenes,
Peer connections bloom and bend,
From ring to active, journey's end! 🎯✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.40% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description check	✅ Passed	The PR description includes all major template sections (Goal, Implementation, UI Changes, Testing) with detailed explanations of the problem, proposed solution, and implementation strategy.
Title check	✅ Passed	The title accurately describes the main changes: introducing ActiveStateTransition (referenced as 'ActiveStateGate') to manage ringing-to-active transitions using peer connection strategies.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feature/rahullohra/improve-transition-to-ringing-active-state

📝 Coding Plan

• Generate coding plan for human review comments

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/call/RtcSession.kt (2)

1482-1496: ⚠️ Potential issue | 🔴 Critical

Bug: Comparing StateFlow objects to null instead of their values.

Lines 1482 and 1490 check publisher == null and subscriber == null, but these are now MutableStateFlow objects that are never null. The check should be against .value.

🐛 Proposed fix

-    if (event.peerType == PeerType.PEER_TYPE_PUBLISHER_UNSPECIFIED && publisher == null && sfuConnectionModule.socketConnection.state().value is SfuSocketState.Connected) {
+    if (event.peerType == PeerType.PEER_TYPE_PUBLISHER_UNSPECIFIED && publisher.value == null && sfuConnectionModule.socketConnection.state().value is SfuSocketState.Connected) {
         logger.v {
             "[handleIceTrickle] `#sfu`; #${event.peerType.stringify()}; publisher is null, adding to pending"
         }
         publisherPendingEvents.add(event)
         return
     }

-    if (event.peerType == PeerType.PEER_TYPE_SUBSCRIBER && subscriber == null && sfuConnectionModule.socketConnection.state().value is SfuSocketState.Connected) {
+    if (event.peerType == PeerType.PEER_TYPE_SUBSCRIBER && subscriber.value == null && sfuConnectionModule.socketConnection.state().value is SfuSocketState.Connected) {
         logger.v {
             "[handleIceTrickle] `#sfu`; #${event.peerType.stringify()}; subscriber is null, adding to pending"
         }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/call/RtcSession.kt`
around lines 1482 - 1496, The null checks are currently comparing the StateFlow
objects themselves (publisher and subscriber) instead of their contained values;
update the conditions in handleIceTrickle to test publisher.value == null and
subscriber.value == null respectively (while keeping the existing
sfuConnectionModule.socketConnection.state().value is SfuSocketState.Connected
checks) and continue to add the event to publisherPendingEvents or
subscriberPendingEvents when the .value is null; ensure you reference the same
symbols (publisher, subscriber, publisherPendingEvents, subscriberPendingEvents,
sfuConnectionModule.socketConnection.state().value, SfuSocketState.Connected) so
the logic behaves correctly with MutableStateFlow.

---

1521-1526: ⚠️ Potential issue | 🔴 Critical

Similar issue: Line 1521 checks subscriber == null instead of subscriber.value == null.

🐛 Proposed fix

     suspend fun handleSubscriberOffer(offerEvent: SubscriberOfferEvent) {
         logger.d { "[handleSubscriberOffer] `#sfu`; `#subscriber`; event: $offerEvent" }
-        if (subscriber == null) {
+        if (subscriber.value == null) {
             subscriberPendingEvents.add(offerEvent)
             return
         }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/call/RtcSession.kt`
around lines 1521 - 1526, The null check is using the wrapper reference instead
of the held value; change the condition in RtcSession where it currently does if
(subscriber == null) to if (subscriber.value == null) so that you add offerEvent
to subscriberPendingEvents when there is no actual subscriber, then keep calling
subscriber.value?.negotiate(offerEvent.sdp) afterwards; target the block that
references subscriber, subscriberPendingEvents, offerEvent, and negotiate to
make this replacement.

🧹 Nitpick comments (3)

stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/CallState.kt (1)

490-491: Consider using a simpler Set implementation.

ConcurrentHashMap.newKeySet() is thread-safe but the set is only written in updateRingingState() and read in ActiveStateTransition. Since both operations appear to occur on the same scope, a simple mutableSetOf guarded by the existing serialization might suffice. However, if thread safety is intentional for future use, this is acceptable.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/CallState.kt`
around lines 490 - 491, Replace the thread-safe ConcurrentHashMap.newKeySet()
used for previousRingingStates with a simple mutableSetOf<RingingState>() and
rely on the existing serialization that already guards writes in
updateRingingState() and reads in ActiveStateTransition; update any nullability
or type expectations accordingly and add a brief comment on the assumed
synchronization (or keep the ConcurrentHashMap.newKeySet and add a comment if
you want to intentionally preserve thread-safety for future use).

stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/ActiveStateTransition.kt (2)

33-33: Consider making the timeout configurable.

The 5-second timeout is hardcoded. For varying network conditions or testing scenarios, consider accepting this as a constructor parameter with a default value.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/ActiveStateTransition.kt`
at line 33, The hardcoded PEER_CONNECTION_OBSERVER_TIMEOUT should be made
configurable: replace the top-level constant with a configurable parameter
(e.g., timeoutMs) on the ActiveStateTransition class constructor (or factory)
with a default of 5_000L, update any references that used
PEER_CONNECTION_OBSERVER_TIMEOUT to use the instance property (timeoutMs), and
keep the original default so behavior is unchanged unless overridden for testing
or different network conditions.

---

60-63: Unused enum value ANY_PEER_CONNECTED.

The ANY_PEER_CONNECTED strategy is defined but never used—only BOTH_PEER_CONNECTED is passed on line 52. Consider removing it to avoid dead code, or document if it's intended for future use.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/ActiveStateTransition.kt`
around lines 60 - 63, The enum TransitionToRingingStateStrategy declares an
unused member ANY_PEER_CONNECTED (only BOTH_PEER_CONNECTED is actually used);
either remove ANY_PEER_CONNECTED to eliminate dead code or explicitly document
its intended future use (or implement its logic where transition strategy is
chosen); update the enum declaration accordingly and adjust any references to
TransitionToRingingStateStrategy to reflect the reduced set or add a TODO/KDoc
on ANY_PEER_CONNECTED explaining why it remains.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/ActiveStateTransition.kt`:
- Around line 111-124: The callback can run after cleanup() due to a race
between withTimeoutOrNull returning and calling transitionToActiveState(); guard
against this by checking coroutine cancellation before proceeding: after
withTimeoutOrNull (where result is read) and before calling
transitionToActiveState(), verify the coroutine is still active (e.g., check
coroutineContext.isActive or call ensureActive()) and bail out if cancelled;
update the block handling result/null in ActiveStateTransition (around
withTimeoutOrNull and transitionToActiveState) to perform this cancellation
check so transitionToActiveState() never runs post-cleanup().

In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/Call.kt`:
- Around line 235-238: Tests fail because callers try to assign the session flow
directly but _session is private and session is read-only; add a test-only
accessor or setter so tests can update the flow. For example, in the Call class
expose a `@VisibleForTesting` fun setSession(value: RtcSession?) (or mark _session
as internal with `@VisibleForTesting`) that updates the MutableStateFlow
_session.value, so tests that reference session assignments
(ReconnectSessionIdTest and ReconnectAttemptsCountTest) can set the session
without changing the public read-only session: StateFlow<RtcSession?>.

In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/CallState.kt`:
- Line 682: The property peerConnectionObserverJob in CallState is unused dead
code; remove its declaration and any cancel call (currently at line where
cancelled) to avoid confusion and duplicate state since ActiveStateTransition
already manages its own peerConnectionObserverJob; search for
"peerConnectionObserverJob" and delete the private var in CallState plus the
cancel/cleanup call that references it, or alternatively if intended to be
shared, initialize and use it consistently across CallState methods and
ActiveStateTransition (prefer removal unless you have a real shared
responsibility).

---

Outside diff comments:
In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/call/RtcSession.kt`:
- Around line 1482-1496: The null checks are currently comparing the StateFlow
objects themselves (publisher and subscriber) instead of their contained values;
update the conditions in handleIceTrickle to test publisher.value == null and
subscriber.value == null respectively (while keeping the existing
sfuConnectionModule.socketConnection.state().value is SfuSocketState.Connected
checks) and continue to add the event to publisherPendingEvents or
subscriberPendingEvents when the .value is null; ensure you reference the same
symbols (publisher, subscriber, publisherPendingEvents, subscriberPendingEvents,
sfuConnectionModule.socketConnection.state().value, SfuSocketState.Connected) so
the logic behaves correctly with MutableStateFlow.
- Around line 1521-1526: The null check is using the wrapper reference instead
of the held value; change the condition in RtcSession where it currently does if
(subscriber == null) to if (subscriber.value == null) so that you add offerEvent
to subscriberPendingEvents when there is no actual subscriber, then keep calling
subscriber.value?.negotiate(offerEvent.sdp) afterwards; target the block that
references subscriber, subscriberPendingEvents, offerEvent, and negotiate to
make this replacement.

---

Nitpick comments:
In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/ActiveStateTransition.kt`:
- Line 33: The hardcoded PEER_CONNECTION_OBSERVER_TIMEOUT should be made
configurable: replace the top-level constant with a configurable parameter
(e.g., timeoutMs) on the ActiveStateTransition class constructor (or factory)
with a default of 5_000L, update any references that used
PEER_CONNECTION_OBSERVER_TIMEOUT to use the instance property (timeoutMs), and
keep the original default so behavior is unchanged unless overridden for testing
or different network conditions.
- Around line 60-63: The enum TransitionToRingingStateStrategy declares an
unused member ANY_PEER_CONNECTED (only BOTH_PEER_CONNECTED is actually used);
either remove ANY_PEER_CONNECTED to eliminate dead code or explicitly document
its intended future use (or implement its logic where transition strategy is
chosen); update the enum declaration accordingly and adjust any references to
TransitionToRingingStateStrategy to reflect the reduced set or add a TODO/KDoc
on ANY_PEER_CONNECTED explaining why it remains.

In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/CallState.kt`:
- Around line 490-491: Replace the thread-safe ConcurrentHashMap.newKeySet()
used for previousRingingStates with a simple mutableSetOf<RingingState>() and
rely on the existing serialization that already guards writes in
updateRingingState() and reads in ActiveStateTransition; update any nullability
or type expectations accordingly and add a brief comment on the assumed
synchronization (or keep the ConcurrentHashMap.newKeySet and add a comment if
you want to intentionally preserve thread-safety for future use).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: aabc82c3-ef94-452b-8aad-f185e736a6c2

📥 Commits

Reviewing files that changed from the base of the PR and between 9128cd3 and b9db4dc.

📒 Files selected for processing (7)

• stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/ActiveStateTransition.kt
• stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/Call.kt
• stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/CallState.kt
• stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/CallStats.kt
• stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/StreamVideoClient.kt
• stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/call/RtcSession.kt
• stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/handlers/StreamMediaSessionController.kt

[sonarqubecloud]

Quality Gate failed

Failed conditions
32.4% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud