feat: overhaul phone call system — thread safety, CallKit extraction, China compliance#6322
Conversation
…io thread safety Core Audio real-time callbacks access shared state ~100 times/sec. OmiAudioLock uses os_unfair_lock (5-10ns) instead of DispatchQueue.sync (50-200ns) to protect audio device state without blocking real-time threads.
…audio state Moved to PhoneCalls/ subfolder. Applied @OmiAudioLock to audioUnit, renderingContext, capturingContext, and isMicStreamMuted. Pre-allocates capture buffer in startCapturing() instead of allocating on the real-time audio thread. Eliminates active data race and malloc-on-RT-thread bugs.
…t error reporting Provides specific error codes (MIC_PERMISSION_DENIED, TWILIO_ERROR, etc.) with human-readable messages, serializable to Flutter EventChannel.
…tors Protocol enabling CallKit (OmiCallCoordinator) and no-CallKit (OmiDirectCallCoordinator) implementations to be swapped at init time based on region. Decouples the plugin from CallKit entirely.
…eset recovery Extracted CallKit management into dedicated coordinator with: - DispatchQueue-based state isolation - Provider readiness tracking with 3-second timeout - Automatic provider recreation after system reset - Pending completion tracking by UUID - 20ms audio buffer config (5ms corrupts AirPods) - Fallback action.fulfill() to prevent CallKit timeouts
Enables proximity monitoring on call ringing, disables on cleanup. Toggles idle timer based on proximity state changes.
Manages AVAudioSession directly without CallKit for regions where Apple requires CallKit to be deactivated. No system call UI, no Phone.app recents — VoIP functionality preserved.
…iction Checks device locale (CN) and carrier MCC (460) to determine if CallKit should be disabled, as required by Apple/MIIT for the China App Store.
… check, proximity Moved to PhoneCalls/ subfolder as OmiPhoneCallsPlugin. Key changes: - No CallKit import — delegates to OmiCallCoordinatorProtocol - Region-based coordinator selection (CallKit vs Direct for China) - Proximity sensor enable/disable on call lifecycle - Audio route discovery and selection (iPhone/Speaker/AirPods/BT) - Structured error events, mute/speaker confirmation events - Dedicated audio dispatch queue (off main thread)
Adds PhoneCalls PBXGroup, moves existing file refs, adds 7 new Swift files to build sources.
Data model for audio output routes (iPhone, Speaker, AirPods, Bluetooth, headphones, CarPlay) with factory from native map.
Structured error with code/message from native EventChannel. Token expiresAt enables proactive refresh scheduling.
…s, region check Adds onError, onMuteConfirmed, onSpeakerConfirmed event parsing. New methods: getAudioRoutes(), selectAudioRoute(), isCallKitAvailable().
…onStatus - Proactive token refresh (3-min buffer before expiry) - Audio route discovery and selection via native bridge - TranscriptionStatus enum exposed to UI (idle/connecting/active/reconnecting/failed) - Audio buffering during WS reconnect (~2s, 100 frames) - Increased max WS reconnect from 5 to 10 attempts - Confirmation-based mute/speaker (no more optimistic UI update) - Structured PhoneCallError storage
…to call UI - Transcription status dot (yellow/orange/red) above transcript - Audio route picker bottom sheet (long-press speaker button) - Specific error messages in failed state from lastError - Route icons for iPhone/Speaker/AirPods/Bluetooth/headphones
Green when active, orange when reconnecting, red when failed — gives user visual feedback on transcription health without text.
…KitAvailable Basic Speaker/Phone route support on Android. isCallKitAvailable returns false (iOS-only). Maintains MethodChannel contract parity.
… access levels - AVAudioSession.Port has no .carPlay member — removed from route discovery - Changed callCoordinator, callUUID, proximitySensor to fileprivate for access from TwilioCallDelegateHandler in the same file
…r all 34 locales New keys: transcriptionConnecting, transcriptionReconnecting, transcriptionUnavailable, audioOutput — with real translations for all 33 non-English locales.
Replace hardcoded strings with context.l10n.transcriptionConnecting, transcriptionReconnecting, transcriptionUnavailable, and audioOutput.
…ve call iOS temporarily deactivates the audio session during screen lock and reactivates it. Stopping the audio device during this window causes Twilio to detect the device stopped and disconnect the call. Now only stops the device when no active call exists. Fixes: pressing power button on ActiveCallPage ending the call.
…press is intended iOS behavior Pressing the power button during a CallKit VoIP call sends CXEndCallAction by design. Removed action.fail(), audio session reactivation hacks, and debug stack traces. Kept clean CXEndCallAction handling that fulfills and disconnects normally.
…ximity sensor - Android: send muteConfirmed/speakerConfirmed events so Dart UI updates (confirmation-based state requires native to echo back the change) - iOS: switch audio session mode from .voiceChat to .default (fixes Bluetooth earphone audio output routing) - iOS: rewrite OmiProximitySensor with @mainactor pattern
mdmohsin7
force-pushed
the
feat/phone-call-system-improvement
branch
from
1fedefa to
0416c99
Compare
Greptile SummaryThis PR comprehensively overhauled the phone call stack:
Confidence Score: 4/5Safe to merge after fixing the token-refresh retry timing; all other findings are P2 or lower One P1 bug: _scheduleTokenRefresh(30) fires in 15 s instead of the intended 30 s with a misleading log message. All three prior reviewer concerns (expiresAt frozen, _tokenExpiry dead code, silent refresh failure with no retry) have been resolved in this revision. The remaining P2 (DEBUG race in OmiAudioLock) does not affect production builds. app/lib/providers/phone_call_provider.dart — _scheduleTokenRefresh retry argument should be 60, not 30 Important Files Changed
Sequence DiagramsequenceDiagram
participant F as Flutter/Dart
participant P as PhoneCallProvider
participant S as PhoneCallService
participant N as OmiPhoneCallsPlugin
participant C as OmiCallCoordinator
participant T as TwilioVoiceSDK
F->>P: makeCall(phoneNumber)
P->>S: initialize(token)
P->>S: makeCall(phoneNumber, callId)
S->>N: MethodChannel makeCall
N->>C: startCall(uuid, phoneNumber)
C-->>N: onAudioSessionActivated
N->>T: TwilioVoiceSDK.connect()
T-->>N: callDidConnect
N-->>C: reportCallConnected(uuid)
N-->>S: EventChannel {callStateChanged: active}
S-->>P: onCallStateChanged(active)
P-->>F: notifyListeners()
Note over P,N: Proactive token refresh (TTL − 3 min)
P->>P: _scheduleTokenRefresh(ttl)
P->>S: initialize(newToken)
Note over P,N: Confirmation-based mute toggle
F->>P: toggleMute()
P->>S: toggleMute(!_isMuted)
S->>N: MethodChannel toggleMute
N-->>S: EventChannel {muteConfirmed}
S-->>P: onMuteConfirmed(muted)
P-->>F: notifyListeners()
Note over P: WS reconnect with audio buffering
P->>P: buffer audio frames (_audioBuffer)
P->>P: _connectTranscriptionSocket()
P->>P: flush _audioBuffer to new socket
Reviews (2): Last reviewed commit: "fix: address PR review — remove double-d..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| func sendCallStateEvent(_ state: String) { | ||
| sendEvent(["type": "callStateChanged", "state": state]) | ||
| } | ||
|
|
||
| func sendErrorEvent(_ error: OmiCallError) { | ||
| sendEvent(error.toEventData()) | ||
| } | ||
|
|
||
| func sendAudioDataEvent(_ data: Data, channel: Int) { | ||
| audioEventQueue.async { [weak self] in | ||
| guard let self = self else { return } | ||
| DispatchQueue.main.async { |
There was a problem hiding this comment.
audioEventQueue.async immediately re-dispatches to the main thread via DispatchQueue.main.async. The intermediate hop adds a thread context switch on every audio callback (~50 Hz) without doing any work on audioEventQueue. The comment on the queue says it exists "to avoid flooding the main thread", but this implementation doesn't throttle or batch — the sink call still ends up on main. Compare sendEvent(_:) below, which dispatches directly to main in one hop.
| func sendCallStateEvent(_ state: String) { | |
| sendEvent(["type": "callStateChanged", "state": state]) | |
| } | |
| func sendErrorEvent(_ error: OmiCallError) { | |
| sendEvent(error.toEventData()) | |
| } | |
| func sendAudioDataEvent(_ data: Data, channel: Int) { | |
| audioEventQueue.async { [weak self] in | |
| guard let self = self else { return } | |
| DispatchQueue.main.async { | |
| func sendAudioDataEvent(_ data: Data, channel: Int) { | |
| DispatchQueue.main.async { [weak self] in | |
| self?.eventSink?([ | |
| "type": "audioData", | |
| "data": FlutterStandardTypedData(bytes: data), | |
| "channel": channel, | |
| ]) | |
| } | |
| } |
|
|
||
| PhoneCallToken({required this.accessToken, required this.ttl, required this.identity}); | ||
|
|
||
| DateTime get expiresAt => DateTime.now().add(Duration(seconds: ttl)); |
There was a problem hiding this comment.
expiresAt returns a different value on every access
DateTime.now().add(Duration(seconds: ttl)) is evaluated fresh each time the getter is called. If a caller holds a PhoneCallToken and queries expiresAt minutes later, the result will be minutes too far in the future — it no longer represents the actual expiry of the token. Currently _tokenExpiry is assigned right after the HTTP response so drift is minimal, but any future use of this getter for expiry decisions will be subtly wrong.
Fix by computing it once at object construction:
| DateTime get expiresAt => DateTime.now().add(Duration(seconds: ttl)); | |
| late final DateTime expiresAt = DateTime.now().add(Duration(seconds: ttl)); |
This freezes the value at parse time, making it safe to call multiple times.
|
|
||
| // Audio routes | ||
| List<AudioRoute> _availableRoutes = []; |
There was a problem hiding this comment.
_tokenExpiry is stored but never read — dead code
_tokenExpiry is assigned in two places (initial call setup and inside the refresh callback) but is never read for any decision. The refresh logic uses token.ttl directly in _scheduleTokenRefresh(token.ttl), so storing _tokenExpiry has no effect. If the intent was to compute remaining time more accurately on the next cycle (e.g. _tokenExpiry!.difference(DateTime.now()).inSeconds), that calculation is missing. Remove the field until it is actually used, to avoid misleading future readers.
| // Audio routes | |
| List<AudioRoute> _availableRoutes = []; | |
| // Token refresh | |
| Timer? _tokenRefreshTimer; |
|
|
||
| Logger.info('PhoneCallProvider: scheduling token refresh in ${refreshInSeconds}s'); | ||
| _tokenRefreshTimer = Timer(Duration(seconds: refreshInSeconds), () async { | ||
| if (_callState != PhoneCallState.active && _callState != PhoneCallState.ringing) return; | ||
| Logger.info('PhoneCallProvider: refreshing call token'); | ||
| var token = await api.getPhoneCallToken(); | ||
| if (token != null) { | ||
| await _nativeService.initialize(token.accessToken); | ||
| _tokenExpiry = token.expiresAt; | ||
| _scheduleTokenRefresh(token.ttl); | ||
| } |
There was a problem hiding this comment.
If api.getPhoneCallToken() returns null (network error, server error), the refresh silently does nothing. The original token will expire at its original TTL, and the call will fail with no warning to the user. For a feature specifically designed to prevent silent call failures, this is an ironic gap. At minimum, log an error and consider a simple retry:
var token = await api.getPhoneCallToken();
if (token != null) {
await _nativeService.initialize(token.accessToken);
_scheduleTokenRefresh(token.ttl);
} else {
Logger.error('PhoneCallProvider: token refresh failed — retrying in 30s');
// Retry once before the original token expires
_tokenRefreshTimer = Timer(const Duration(seconds: 30), () => _scheduleTokenRefresh(30));
}| "getAudioRoutes" -> { | ||
| // Basic routes for Android — Speaker and Phone | ||
| val routes = listOf( | ||
| mapOf("id" to "phone", "name" to "Phone", "type" to "iPhone"), | ||
| mapOf("id" to "speaker", "name" to "Speaker", "type" to "speaker") |
There was a problem hiding this comment.
"iPhone"
Sending "type" to "iPhone" from Android causes AudioRoute.fromMap to assign AudioRouteType.iPhone to the earpiece. The UI then uses Icons.phone_android for that type, which happens to be correct — but only by coincidence, because the icon lookup for AudioRouteType.iPhone was written with Icons.phone_android. If AudioRouteType.iPhone is ever renamed or its icon updated for iOS, the Android earpiece silently breaks.
Consider adding a shared earpiece type to AudioRouteType on the Dart side and using it from both platforms:
mapOf("id" to "phone", "name" to "Phone", "type" to "earpiece"),…d refresh retry - Remove unnecessary audioEventQueue double-dispatch on audio path - Compute expiresAt once at construction instead of on every access - Remove unused _tokenExpiry field - Retry token refresh in 30s on failure instead of silently giving up
|
@greptile-apps re-review |
_scheduleTokenRefresh halves the input (ttl/2), so passing 30 resulted in a 15s delay. Pass 60 so the actual timer fires at 30s.
… China compliance (BasedHardware#6322) ## Summary - **Audio thread safety**: Added `OmiAudioLock` (`os_unfair_lock`, 5-10ns) to protect audio device state from Core Audio real-time callbacks. Pre-allocates capture buffers instead of malloc on the audio thread. - **Structured native errors**: Errors now propagate from native to Dart with specific codes (`MIC_PERMISSION_DENIED`, `TWILIO_ERROR`, etc.). Mute/speaker toggles use confirmation-based state updates, fixing a race condition where UI could desync from native state. - **CallKit coordinator extraction**: All CallKit management extracted into `OmiCallCoordinator` with provider reset recovery, 3-second readiness timeout, and pending completion tracking. The plugin itself no longer imports CallKit. - **China/CallKit compliance**: `OmiRegionCheck` detects CN locale and carrier MCC 460, swapping to `OmiDirectCallCoordinator` which manages AVAudioSession directly without CallKit. Resolves Apple Guideline 5 Legal rejection. - **Token refresh**: Proactive Twilio token refresh with a 3-minute buffer before expiry, preventing silent call failures on long calls. - **Proximity sensor**: Screen turns off when phone held to ear during calls, prevents accidental touches. - **Audio route selection**: Full discovery of available outputs (iPhone, Speaker, AirPods, Bluetooth). Long-press the speaker button to open the route picker. - **WebSocket resilience**: Audio data buffered during transcription WebSocket reconnects (~2s). Max reconnect attempts increased from 5 to 10. `TranscriptionStatus` enum tracks WS health. - **Transcription status UI**: Color-coded indicator (yellow=connecting, orange=reconnecting, red=failed) in call view and banner. - **File organization**: All phone call Swift files moved into `app/ios/Runner/PhoneCalls/` subfolder (9 files). ## Test plan - [x] Make a call on iOS — verify audio works, CallKit green bar appears (non-China region) - [x] Set device locale to China — verify no CallKit UI, call still works - [ ] Connect AirPods → long-press speaker → verify route picker shows AirPods - [x] Hold phone to ear during call → verify screen turns off - [ ] Toggle mute rapidly 10x → verify UI stays in sync - [ ] Kill backend WebSocket mid-call → verify reconnecting/unavailable indicators - [ ] Start long call → verify token refreshes without interrupting audio - [x] Test on Android → verify speaker toggle and basic routes still work 🤖 Generated with [Claude Code](https://claude.com/claude-code)
1 participant
Summary
OmiAudioLock(os_unfair_lock, 5-10ns) to protect audio device state from Core Audio real-time callbacks. Pre-allocates capture buffers instead of malloc on the audio thread.MIC_PERMISSION_DENIED,TWILIO_ERROR, etc.). Mute/speaker toggles use confirmation-based state updates, fixing a race condition where UI could desync from native state.OmiCallCoordinatorwith provider reset recovery, 3-second readiness timeout, and pending completion tracking. The plugin itself no longer imports CallKit.OmiRegionCheckdetects CN locale and carrier MCC 460, swapping toOmiDirectCallCoordinatorwhich manages AVAudioSession directly without CallKit. Resolves Apple Guideline 5 Legal rejection.TranscriptionStatusenum tracks WS health.app/ios/Runner/PhoneCalls/subfolder (9 files).Test plan
🤖 Generated with Claude Code