Ensure smoother Ringing to Active transition

[rahul-lohra]

Goal

Ensure a responsive transition to RingingState.Active, where a user is not rendered to others unless their own local media is available.

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()

---

Current Approach

We previously considered transitioning to RingingState.Active only after both Publisher and Subscriber RTCPeerConnections are connected to the SFU.

This approach aimed to ensure media readiness before activating the UI.

Problem

The Both Peers Connected strategy introduces one key issue:
• Privacy Risk: There can still be edge cases where a user is rendered remotely before they have a confirmed local preview, violating a strict privacy expectation.

Additionally, this approach does not align well with established real-world patterns.

Apps like WhatsApp and Telegram:
• Render the local camera preview immediately during the outgoing ringing state
• Show local preview on the callee side right after call acceptance
• Transition to remote media once available

This avoids the “active UI but no media” gap while maintaining privacy.

---

Solution

Adopt the Publisher Connected strategy:
• Transition to RingingState.Active when the Publisher reaches PeerConnectionState.CONNECTED
• Ensure the user’s local media is available before exposing them to others
• Avoid waiting for Subscriber connection to reduce latency

This provides a better balance between:
• Privacy guarantees
• Faster perceived call setup
• Alignment with industry-standard UX patterns

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

internal enum class TransitionToRingingStateStrategy {
    LEGACY_BEHAVIOUR,
    PUBLISHER_CONNECTED    
}

We can use any strategy to experiment, by default we will select PUBLISHER_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

//We will remove all the unused strategy once validation is done and will keep the one which is getting used 

internal enum class TransitionToRingingStateStrategy {
    LEGACY_BEHAVIOUR,
    PUBLISHER_CONNECTED    
}

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.PUBLISHER_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 (LEGACY_BEHAVIOUR or PUBLISHER_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.66 MB	-0.02 MB	🚀
stream-video-android-ui-compose	6.27 MB	6.27 MB	0.00 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 | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

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.
Description check	❓ Inconclusive	PR description provides goal and implementation overview but lacks concrete details in critical sections. UI Changes section contains only placeholders, Testing section provides no actual test procedures or validation steps, and contributor/reviewer checklists are incomplete.	Complete the Testing section with specific step-by-step instructions for reproducing the scenario (e.g., 'Call participant A, observe media timeline'); provide concrete test results; fill Testing details with actual validation approach; and confirm which checklist items have been completed.

✅ Passed checks (1 passed)

Check name	Status	Explanation
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

---

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
30.4% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

[aleksandar-apostolov]

All feedback addressed. LGTM.

[stream-public-bot]

🚀 Available in v1.21.0