Conversation
This comment has been minimized.
This comment has been minimized.
|
Warning Review limit reached
Next review available in: 43 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR migrates audio playback from ChangesMedia API migration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant AudioStreamStore
participant ExpoAudio
participant AudioPlayer
AudioStreamStore->>ExpoAudio: setAudioModeAsync(...)
AudioStreamStore->>ExpoAudio: createAudioPlayer(...)
ExpoAudio-->>AudioStreamStore: return AudioPlayer
AudioStreamStore->>AudioPlayer: add playback status listener
AudioPlayer-->>AudioStreamStore: emit playback status
AudioStreamStore->>AudioPlayer: seekTo(0) and play()
AudioStreamStore->>AudioPlayer: pause() and remove()
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (4)
src/services/__tests__/audio.service.test.ts (1)
3-10: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace
anywith a typedAudioPlayertest double.These casts disable type checking where the migration contract is tested. Define a narrow
Pick<AudioPlayer, ...>for the mocked methods and properties. Usejest.MockedFunction<typeof createAudioPlayer>withoutany.As per coding guidelines, “Never use
anytype; use precise types and interfaces with TypeScript strict mode enabled.”Also applies to: 82-87, 291-292
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/__tests__/audio.service.test.ts` around lines 3 - 10, Replace the any casts in the mockSound test double and the additional referenced locations with a narrow Pick<AudioPlayer, ...> type covering only the mocked properties and methods. Type the createAudioPlayer mock as jest.MockedFunction<typeof createAudioPlayer>, preserving the existing test behavior without disabling TypeScript checking.Source: Coding guidelines
src/services/__tests__/notification-sound.service.test.ts (1)
63-84: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the migrated player behavior.
The current assertions pass when the audio-mode options or player options are incorrect. Capture the created player and assert the exact
setAudioModeAsyncarguments,createAudioPlayerarguments,seekTo(0),play(), andremove()calls. Add an error case that verifies logging when seeking fails.As per coding guidelines, “Create and use Jest to test to validate all generated components, services and logic generated.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/__tests__/notification-sound.service.test.ts` around lines 63 - 84, Strengthen the notificationSoundService tests around initialize() to validate migrated player behavior, not just call counts: capture created players and assert the exact setAudioModeAsync and createAudioPlayer options, plus seekTo(0), play(), and remove() calls. Add a seeking failure test that verifies the expected logging, while preserving the existing concurrent-initialization coverage.Source: Coding guidelines
src/stores/app/__tests__/livekit-store.test.ts (2)
106-106: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove the native mock before imports.
The
expo-audiomock is declared after imports. Move the mock block above imports so test setup follows the repository convention.As per coding guidelines, “Mock native modules at the top of test files before imports.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/stores/app/__tests__/livekit-store.test.ts` at line 106, Move the expo-audio native mock containing setAudioModeAsync above all imports in the test file, preserving its existing mock behavior and following the repository’s test setup convention.Source: Coding guidelines
106-106: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPlace both native mocks before imports.
The changed
expo-audiomocks are declared after imports in both test files. Move each mock block above the first import.
src/stores/app/__tests__/livekit-store.test.ts#L106-L106: move theexpo-audiomock block before imports.src/stores/app/__tests__/livekit-store-room-switch.test.ts#L37-L37: move theexpo-audiomock block before imports.As per coding guidelines, “Mock native modules at the top of test files before imports.” (raw.githubusercontent.com)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/stores/app/__tests__/livekit-store.test.ts` at line 106, Move the expo-audio mock block before the first import in both src/stores/app/__tests__/livekit-store.test.ts (lines 106-106) and src/stores/app/__tests__/livekit-store-room-switch.test.ts (lines 37-37), keeping the mock definitions otherwise unchanged.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/audio-stream-refactoring.md`:
- Around line 48-56: Update the Android background-playback guidance in the
Configuration or Troubleshooting section to document activating lock-screen
controls with setActiveForLockScreen(true, metadata) and configuring the audio
mode with interruptionMode: 'doNotMix'. Clarify that shouldPlayInBackground and
keepAudioSessionActive alone do not ensure sustained playback and that
lock-screen activation prevents playback from stopping after extended background
operation.
In `@jest-setup.ts`:
- Around line 243-249: Update the useVideoPlayer mock in jest.mock('expo-video')
to accept the setup callback and invoke it with the mocked player before
returning that player. Preserve the existing play, pause, and addListener mock
behavior so tests verify automatic playback through the same setup path as
NativeVideoPlayer.
In `@src/components/call-video-feeds/video-player-modal.tsx`:
- Line 56: Update the NativeVideoPlayer rendering branch in VideoPlayerModal to
check isAndroid from `@/lib/platform.ts` before passing contentType 'dash'; on
non-Android platforms, render the supported fallback or translated unsupported
state instead of attempting DASH playback, while preserving HLS and Android DASH
behavior.
- Around line 29-34: Update NativeVideoPlayer to subscribe to the video player's
status changes using the Expo SDK 56 event API, detect statusChange.error, and
render an appropriate translated error state instead of leaving the failed
player active. Show a useToastStore toast with retry or copy assistance as
supported by existing patterns, and log the playback failure through logger when
appropriate while preserving normal playback behavior.
In `@src/services/audio.service.ts`:
- Around line 108-113: Guard initialization in initializeAudio/initialize with
an initializationPromise, reusing the in-flight promise when initialization is
already running and clearing it appropriately after completion. Preserve the
existing isInitialized behavior and ensure concurrent callers create only one
set of five players; add a concurrent initialize() test asserting exactly five
players are created, following NotificationSoundService’s pattern.
In `@src/services/notification-sound.service.ts`:
- Around line 58-63: Centralize ownership of global audio-mode configuration
through a shared coordinator: remove the direct persistent setAudioModeAsync
application in src/services/notification-sound.service.ts at lines 58-63, and
update the audio initialization in src/services/audio.service.ts at lines 46-51
and stream setup in src/stores/app/audio-stream-store.ts at lines 102-108 to
request mode changes through that coordinator. Have the coordinator derive
shouldPlayInBackground, interruptionMode, and related flags from the active
audio operation, and add an integration test covering stream startup followed by
audio/notification sound initialization while preserving the stream session.
In `@src/stores/app/audio-stream-store.ts`:
- Around line 102-176: Update audio playback setup in
src/stores/app/audio-stream-store.ts around createAudioPlayer to use
interruptionMode 'doNotMix', configure lock-screen controls with the stream
metadata before sound.play(), and deactivate those controls before every
sound.remove() path, including playback errors. Document this
lock-screen-control requirement in docs/audio-stream-refactoring.md at lines
48-56.
In `@src/stores/app/livekit-store.ts`:
- Around line 64-68: Update the setAudioModeAsync configuration in the LiveKit
audio setup to use a PTT-appropriate Android interruption mode instead of
mixWithOthers: choose doNotMix for exclusive call audio or duckOthers when
competing audio should be reduced, and document the rationale if mixing remains
intentional.
- Around line 90-97: Update setupAudioRouting’s iOS setAudioModeAsync
configuration so shouldRouteThroughEarpiece reflects the selected route: use
true for earpiece and false for speaker, preserving the existing audio-mode
settings.
---
Nitpick comments:
In `@src/services/__tests__/audio.service.test.ts`:
- Around line 3-10: Replace the any casts in the mockSound test double and the
additional referenced locations with a narrow Pick<AudioPlayer, ...> type
covering only the mocked properties and methods. Type the createAudioPlayer mock
as jest.MockedFunction<typeof createAudioPlayer>, preserving the existing test
behavior without disabling TypeScript checking.
In `@src/services/__tests__/notification-sound.service.test.ts`:
- Around line 63-84: Strengthen the notificationSoundService tests around
initialize() to validate migrated player behavior, not just call counts: capture
created players and assert the exact setAudioModeAsync and createAudioPlayer
options, plus seekTo(0), play(), and remove() calls. Add a seeking failure test
that verifies the expected logging, while preserving the existing
concurrent-initialization coverage.
In `@src/stores/app/__tests__/livekit-store.test.ts`:
- Line 106: Move the expo-audio native mock containing setAudioModeAsync above
all imports in the test file, preserving its existing mock behavior and
following the repository’s test setup convention.
- Line 106: Move the expo-audio mock block before the first import in both
src/stores/app/__tests__/livekit-store.test.ts (lines 106-106) and
src/stores/app/__tests__/livekit-store-room-switch.test.ts (lines 37-37),
keeping the mock definitions otherwise unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b06efafc-47f2-4a02-bcfc-889a63070758
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (16)
__mocks__/expo-audio.ts__mocks__/expo-av.tsapp.config.tsdocs/audio-stream-refactoring.mdjest-setup.tspackage.jsonsrc/components/call-video-feeds/video-player-modal.tsxsrc/services/__tests__/audio.service.test.tssrc/services/__tests__/notification-sound.service.test.tssrc/services/audio.service.tssrc/services/notification-sound.service.tssrc/stores/app/__tests__/audio-stream-store.test.tssrc/stores/app/__tests__/livekit-store-room-switch.test.tssrc/stores/app/__tests__/livekit-store.test.tssrc/stores/app/audio-stream-store.tssrc/stores/app/livekit-store.ts
💤 Files with no reviewable changes (1)
- mocks/expo-av.ts
| ## Configuration | ||
|
|
||
| ## Benefits | ||
|
|
||
| 1. **Better Remote Streaming Support**: `expo-av` provides more robust support for remote MP3 streams | ||
| 2. **Improved Audio Configuration**: Proper audio mode settings for background playback and silent mode | ||
| 3. **Enhanced Error Handling**: Better error recovery and cleanup | ||
| 4. **Loading States**: More granular loading and buffering states for better UX | ||
| 5. **Memory Management**: Proper cleanup of audio resources | ||
|
|
||
| ## Migration Notes | ||
|
|
||
| If you were using the previous audio stream store: | ||
|
|
||
| 1. Replace any direct `audioPlayer` references with `soundObject` | ||
| 2. Update any custom audio handling code to use `expo-av` APIs | ||
| 3. The store API remains largely the same, so most usage code should work without changes | ||
| The app config enables background audio and declares microphone permissions for PTT and LiveKit calls. Runtime microphone permissions are checked without activating a competing audio session so permission handling does not race LiveKit or CallKeep. | ||
|
|
||
| ## Troubleshooting | ||
|
|
||
| ### Common Issues | ||
|
|
||
| 1. **Audio not playing on iOS in silent mode**: Make sure `playsInSilentModeIOS: true` is set | ||
| 2. **Buffering issues**: The store now properly tracks buffering state - use `isBuffering` to show loading indicators | ||
| 3. **Background playback**: Ensure your app has proper background audio permissions configured | ||
|
|
||
| ### Audio Permissions | ||
|
|
||
| Make sure your app's configuration includes proper audio permissions: | ||
|
|
||
| **app.json/app.config.js:** | ||
| ```json | ||
| { | ||
| "expo": { | ||
| "ios": { | ||
| "infoPlist": { | ||
| "UIBackgroundModes": ["audio"] | ||
| } | ||
| }, | ||
| "android": { | ||
| "permissions": [ | ||
| "android.permission.RECORD_AUDIO" | ||
| ] | ||
| } | ||
| } | ||
| } | ||
| ``` | ||
| 1. For silent-mode playback, verify `playsInSilentMode: true`. | ||
| 2. For remote stream stalls, inspect the `isBuffering` and `error` fields delivered by `playbackStatusUpdate`. | ||
| 3. For background playback, verify the platform background-audio configuration and keep the player audio session active. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file context =="
if [ -f docs/audio-stream-refactoring.md ]; then
nl -ba docs/audio-stream-refactoring.md | sed -n '1,120p'
else
echo "docs/audio-stream-refactoring.md not found"
fi
echo
echo "== mentions of audio APIs and background config =="
rg -n "shouldPlayInBackground|keepAudioSessionActive|setActiveForLockScreen|interruptionMode|doNotMix|playsInSilentMode|background audio|playbackStatusUpdate|isBuffering" -S .Repository: Resgrid/Unit
Length of output: 209
🌐 Web query:
Expo SDK 56 AudioPlayer setActiveForLockScreen shouldPlayInBackground keepAudioSessionActive interruptionMode doNotMix Android playback stops after three minutes
💡 Result:
The issue where audio playback stops after approximately three minutes on Android when the app is in the background is a known Android OS limitation that is addressed by enabling lock screen controls in Expo [1][2]. To achieve sustained background audio playback in Expo SDK 56 and later, you must explicitly call the setActiveForLockScreen method [1][3]. According to the official Expo documentation, if this is not configured, the Android operating system will terminate the audio playback after roughly three minutes to optimize battery usage [1][4]. Required configuration steps: 1. Enable Lock Screen Controls: You must use the setActiveForLockScreen method provided by the expo-audio library to enable media controls on the lock screen [1][2]. This signals to the Android OS that the app is actively performing media playback, preventing the system from killing the background task [1]. 2. Configure the Config Plugin: Ensure that the Expo audio config plugin is correctly set up in your app configuration (e.g., app.json or app.config.js) [1][4]. This plugin is responsible for adding the necessary permissions, such as FOREGROUND_SERVICE and FOREGROUND_SERVICE_MEDIA_PLAYBACK, and declaring the media playback foreground service (AudioControlsService) in your AndroidManifest.xml [2]. 3. Background Playback Settings: Ensure that shouldPlayInBackground is set to true in your audio mode configuration [1][2]. While setting shouldPlayInBackground and using the correct interruptionMode (such as mixWithOthers) are important for overall audio behavior, they are insufficient on their own to prevent the three-minute timeout on Android [1][5]. The foreground service enabled via the config plugin, combined with the activation of lock screen controls, is mandatory for indefinite background playback [1][2].
Citations:
- 1: https://docs.expo.dev/versions/v56.0.0/sdk/audio/
- 2: https://docs.expo.dev/versions/unversioned/sdk/audio
- 3: https://docs.expo.dev/versions/v57.0.0/sdk/audio/
- 4: https://docs.expo.dev/versions/latest/sdk/audio/
- 5: [expo-audio][android] On Android only, Expo Audio stops playing in background after about ~3 minutes, even when
shouldPlayInBackgroundandsupportsBackgroundPlaybackare all set totrueexpo/expo#38317
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== docs file lines =="
if [ -f docs/audio-stream-refactoring.md ]; then
awk '{printf "%6d %s\n", NR, $0}' docs/audio-stream-refactoring.md | sed -n '1,120p'
else
echo "docs/audio-stream-refactoring.md not found"
fi
echo
echo "== audio API mentions =="
rg -n "shouldPlayInBackground|keepAudioSessionActive|setActiveForLockScreen|interruptionMode|doNotMix|playsInSilentMode|background audio|playbackStatusUpdate|isBuffering" -S . || trueRepository: Resgrid/Unit
Length of output: 10133
🌐 Web query:
Expo SDK 56 setActiveForLockScreen metadata interruptionMode doNotMix
💡 Result:
When using setActiveForLockScreen in Expo SDK 56 (and other versions using expo-audio), you must configure your audio mode to use interruptionMode: 'doNotMix' via setAudioModeAsync [1][2][3]. The 'doNotMix' mode is a strict requirement for lock screen controls to function correctly because the operating system requires an exclusive audio focus to associate lock screen metadata and controls with your player [1][4][5]. If this mode is not set, the OS may fail to display your metadata or controls [1][2][4]. Key implementation details: 1. Configuration: Always set the audio mode before or when initializing your player [1][6]. setAudioModeAsync({ playsInSilentMode: true, shouldPlayInBackground: true, interruptionMode: 'doNotMix', }); 2. Activation: Use setActiveForLockScreen(true, metadata, options) to enable the controls [1][7]. 3. Android Background Playback: Enabling lock screen controls is essential for sustained background playback on Android; otherwise, the OS may terminate playback after approximately 3 minutes due to background limitations [1][8][7]. For more information, refer to the official Expo Audio documentation [1][2].
Citations:
- 1: https://docs.expo.dev/versions/v56.0.0/sdk/audio/
- 2: https://docs.expo.dev/versions/latest/sdk/audio/
- 3: https://docs.expo.dev/versions/v55.0.0/sdk/audio
- 4: https://github.com/expo/expo/blob/073ace8d/packages/expo-audio/build/AudioModule.types.d.ts
- 5: https://github.com/expo/expo/blob/5cbac55c/packages/expo-audio/src/Audio.types.ts
- 6: Expo lockscreen controls expo/expo#28068
- 7: https://docs.expo.dev/versions/v57.0.0/sdk/audio/
- 8: https://docs.expo.dev/versions/unversioned/sdk/audio
Document the Android lock-screen requirement.
shouldPlayInBackground and keepAudioSessionActive are not sufficient for sustained Android playback. Add guidance for activating lock-screen controls with setActiveForLockScreen(true, metadata) and using interruptionMode: 'doNotMix' in the audio mode. Without lock-screen activation, background playback can stop after about three minutes.
🧰 Tools
🪛 LanguageTool
[style] ~56-~56: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...delivered by playbackStatusUpdate. 3. For background playback, verify the platfor...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/audio-stream-refactoring.md` around lines 48 - 56, Update the Android
background-playback guidance in the Configuration or Troubleshooting section to
document activating lock-screen controls with setActiveForLockScreen(true,
metadata) and configuring the audio mode with interruptionMode: 'doNotMix'.
Clarify that shouldPlayInBackground and keepAudioSessionActive alone do not
ensure sustained playback and that lock-screen activation prevents playback from
stopping after extended background operation.
| jest.mock('expo-video', () => ({ | ||
| VideoView: 'VideoView', | ||
| useVideoPlayer: jest.fn(() => ({ | ||
| play: jest.fn(), | ||
| pause: jest.fn(), | ||
| addListener: jest.fn(() => ({ remove: jest.fn() })), | ||
| })), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
curl -fsSL 'https://docs.expo.dev/versions/v56.0.0/sdk/video/' \
| rg -n 'useVideoPlayer\(source, setup\)|setup.*created'
rg -n -U -C 4 'useVideoPlayer\(' src --glob '*.{ts,tsx}'Repository: Resgrid/Unit
Length of output: 50369
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== jest-setup useVideoPlayer mock =="
sed -n '230,255p' jest-setup.ts
echo
echo "== useVideoPlayer calls =="
rg -n -U -C 3 'useVideoPlayer\(' src --glob '*.{ts,tsx}' || true
echo
echo "== NativeVideoPlayer references =="
rg -n -U -C 3 'NativeVideoPlayer|play\(\)' src --glob '*.{ts,tsx}' || true
echo
echo "== package expo-video version =="
if [ -f package.json ]; then
node - <<'JS'
const fs = require('fs');
const pkg = JSON.parse(fs.readFileSync('package.json', 'utf8'));
console.log(JSON.stringify({ dependencies: pkg.dependencies?.['expo-video'], devDependencies: pkg.devDependencies?.['expo-video'] }, null, 2));
JS
fi
echo
echo "== focused setup callback text in curated docs source if available =="
curl -fsSL 'https://docs.expo.dev/versions/v56.0.0/sdk/video/' \
| rg -n 'useVideoPlayer\(source, setup\)|callback after|created player|automatically play|play()' \
| head -n 80Repository: Resgrid/Unit
Length of output: 50369
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== repo files around setup =="
wc -l jest-setup.ts
sed -n '238,252p' jest-setup.ts
echo
echo "== useVideoPlayer calls in tracked src files only =="
git ls-files 'src/*' 'src/**/*' \
| xargs rg -n -U -C 3 'useVideoPlayer\(' --glob '*.{ts,tsx}' || true
echo
echo "== focused setup callback/signature in docs source only =="
curl -fsSL 'https://docs.expo.dev/versions/v56.0.0/sdk/video/' \
| python3 - <<'PY'
import sys, re
data=sys.stdin.read()
for pat in [
r'useVideoPlayer\s*\([^)]*\)\s*:',
r'\\bsetup\\b.*callback',
r'called.*after.*create',
r'play\\(\\)',
]:
hits=re.findall(pat, data, re.I)
if hits:
print(f"== pattern {pat!r} ==")
print("\n".join(set(hits))[:200])
PY
echo
echo "== package expo-video metadata =="
python3 - <<'JS' || true
const fs = require('fs');
const pkg = JSON.parse(fs.readFileSync('package.json', 'utf8'));
console.log(JSON.stringify({ dependencies: pkg.dependencies?.['expo-video'], devDependencies: pkg.devDependencies?.['expo-video'] }, null, 2));
JSRepository: Resgrid/Unit
Length of output: 1357
🌐 Web query:
Expo SDK 56 video useVideoPlayer setup callback documentation
💡 Result:
In Expo SDK 56, the useVideoPlayer hook is the recommended way to create and manage a VideoPlayer instance, ensuring the player is automatically cleaned up when the component unmounts [1][2]. The setup callback is the second argument of the useVideoPlayer hook [2]. It is an optional function that allows you to configure the player immediately after it is created [2]. The signature of the hook is: useVideoPlayer(source, setup, playerBuilderOptions) [2] Parameters: - source (VideoSource): The video source used to initialize the player [2]. - setup (optional, (player: VideoPlayer) => void): A function that executes after the player is created, allowing you to set properties like player.loop = true or trigger player.play() [1][2]. - playerBuilderOptions (optional, PlayerBuilderOptions): Configuration options applied to the Android player builder before the native constructor is invoked [2]. Example usage: const player = useVideoPlayer(videoSource, player => { player.loop = true; player.play; }); For advanced use cases where you need a player that persists beyond the component's lifecycle, the createVideoPlayer function is available, but it requires you to manually call release() to prevent memory leaks [1][2].
Citations:
- 1: https://docs.expo.dev/versions/v56.0.0/sdk/video/
- 2: https://docs.expo.dev/versions/v56.0.0/sdk/video
Invoke the useVideoPlayer setup callback in the mock.
NativeVideoPlayer calls useVideoPlayer(source, (player) => player.play()), but the current mock returns a stale player and never runs the optional setup function. This can mask a regression where automatic playback is missing from rendered tests.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@jest-setup.ts` around lines 243 - 249, Update the useVideoPlayer mock in
jest.mock('expo-video') to accept the setup callback and invoke it with the
mocked player before returning that player. Preserve the existing play, pause,
and addListener mock behavior so tests verify automatic playback through the
same setup path as NativeVideoPlayer.
Source: Coding guidelines
| const NativeVideoPlayer: React.FC<NativeVideoPlayerProps> = ({ uri, contentType }) => { | ||
| const player = useVideoPlayer({ uri, contentType }, (videoPlayer) => { | ||
| videoPlayer.play(); | ||
| }); | ||
|
|
||
| return <VideoView player={player} style={styles.video} nativeControls contentFit="contain" />; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
curl -fsSL 'https://docs.expo.dev/versions/v56.0.0/sdk/video/' \
| rg -n 'statusChange|error|useEventListener'
rg -n -C 4 \
'useVideoPlayer|statusChange|useEvent|useEventListener|useToastStore|logger' \
src/components/call-video-feeds --glob '*.{ts,tsx}'Repository: Resgrid/Unit
Length of output: 50369
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '--- target file outline ---\n'
ast-grep outline src/components/call-video-feeds/video-player-modal.tsx --view expanded || true
printf '\n--- target file relevant content ---\n'
cat -n src/components/call-video-feeds/video-player-modal.tsx | sed -n '1,220p'
printf '\n--- focused repository search ---\n'
rg -n -C 3 'useVideoPlayer|statusChange|useEvent|useEventListener|useToastStore|logger|NativeVideoPlayer|VideoView' src/components/call-video-feeds --glob '*.{ts,tsx}' || trueRepository: Resgrid/Unit
Length of output: 241
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- target file outline ---'
ast-grep outline src/components/call-video-feeds/video-player-modal.tsx --view expanded || true
printf '%s\n' ''
printf '%s\n' '--- target file relevant content ---'
cat -n src/components/call-video-feeds/video-player-modal.tsx | sed -n '1,220p'
printf '%s\n' ''
printf '%s\n' '--- focused repository search ---'
rg -n -C 3 'useVideoPlayer|statusChange|useEvent|useEventListener|useToastStore|logger|NativeVideoPlayer|VideoView' src/components/call-video-feeds --glob '*.{ts,tsx}' || trueRepository: Resgrid/Unit
Length of output: 13177
🌐 Web query:
Expo SDK 56 expo-video useVideoPlayer statusChange error event useEventListener
💡 Result:
In Expo SDK 56, the expo-video library uses an event-based system for the VideoPlayer, as changes to player properties do not automatically trigger React state updates [1][2]. To handle status changes and potential errors, you can use the statusChange event emitted by the VideoPlayer. The payload for this event, StatusChangeEventPayload, includes the following properties [2]: - status: The new VideoPlayerStatus. - error: An optional PlayerError object, which contains information if an error occurred during the status change [2]. - oldStatus: The previous VideoPlayerStatus [2]. You can listen for this event using the useEventListener hook from the expo package, which is built around the player's addListener and removeListener methods and handles cleanup automatically [1][3]. Example usage: import { useEventListener } from 'expo'; // Inside your component useEventListener(player, 'statusChange', ({ status, error }) => { if (error) { console.error('Player error:', error); } console.log('Player status changed to:', status); }); Alternatively, you can use the useEvent hook if you need a stateful value that updates automatically, or use player.addListener directly with a useEffect hook for more manual control [1][3].
Citations:
- 1: https://docs.expo.dev/versions/unversioned/sdk/video/
- 2: https://docs.expo.dev/versions/v56.0.0/sdk/video
- 3: https://github.com/expo/expo/blob/main/docs/pages/versions/unversioned/sdk/video.mdx
Handle native playback errors in NativeVideoPlayer.
NativeVideoPlayer starts playback but ignores statusChange.error. If the HLS/DASH manifest or request fails, the modal stays open without translated feedback, retry, or copy assistance. Subscribe with useEventListener or useEvent, render an error state, show a useToastStore toast, and log the failure through logger when appropriate. This follows the Expo video event flow documented for SDK 56.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/call-video-feeds/video-player-modal.tsx` around lines 29 - 34,
Update NativeVideoPlayer to subscribe to the video player's status changes using
the Expo SDK 56 event API, detect statusChange.error, and render an appropriate
translated error state instead of leaving the failed player active. Show a
useToastStore toast with retry or copy assistance as supported by existing
patterns, and log the playback failure through logger when appropriate while
preserving normal playback behavior.
Source: Coding guidelines
| case FeedFormat.HLS: | ||
| case FeedFormat.DASH: | ||
| return <Video source={{ uri: feed.Url }} style={styles.video} useNativeControls resizeMode={ResizeMode.CONTAIN} shouldPlay />; | ||
| return <NativeVideoPlayer key={`${feed.FeedFormat}:${feed.Url}`} uri={feed.Url} contentType={feed.FeedFormat === FeedFormat.HLS ? 'hls' : 'dash'} />; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
curl -fsSL 'https://docs.expo.dev/versions/v56.0.0/sdk/video/' \
| rg -n '`dash`|Android-only'
rg -n -C 5 \
'FeedFormat\.DASH|contentType.*dash|VideoAssetTransportProvider|contentTypeHint|isAndroid' \
. --glob '*.{ts,tsx,js}'Repository: Resgrid/Unit
Length of output: 50369
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate files =="
fd -a 'video-player-modal|platform|FeedFormat|feedResult|call' . | sed 's#^\./##' | head -200
echo
echo "== exact DASH/transport/platform references in tracked files =="
rg -n -C 3 \
'FeedFormat\.DASH|contentType:\s*["'\'']dash["'\'']|VideoAssetTransportProvider|contentTypeHint|isAndroid|FeedFormat' \
--glob '*.{ts,tsx,js}' . || true
echo
echo "== video player modal lines =="
file="$(fd 'video-player-modal.tsx' . | head -1)"
if [ -n "$file" ]; then
echo -- "$file"
wc -l "$file"
sed -n '1,140p' "$file" | cat -n
fi
echo
echo "== platform module =="
file="$(fd 'platform.ts' src/lib | head -1)"
if [ -n "$file" ]; then
echo -- "$file"
sed -n '1,160p' "$file" | cat -n
fi
echo
echo "== FeedFormat definitions/usages =="
rg -n -C 3 'enum FeedFormat|FeedFormat;' --glob '*.{ts,tsx,js}' . || true
echo
echo "== deterministic content-type mapping probe from source =="
python3 - <<'PY'
from pathlib import Path
p = next(Path('.').glob('src/components/call-video-feeds/video-player-modal.tsx'), None)
if p:
src = p.read_text()
print("modal_path", str(p))
print("has_hls_branch", "FeedFormat.HLS ? 'hls'" in src)
print("has_dash_branch", "FeedFormat.DASH ? 'dash'" in src)
print("has_isAndroid_in_modal", "isAndroid" in src)
else:
print("modal_path", None)
PYRepository: Resgrid/Unit
Length of output: 37570
Guard FeedFormat.DASH on non-Android platforms.
contentType: 'dash' is Android-only in Expo video, so an .mpd feed sent to VideoPlayerModal will also fail on iOS if no custom transport is registered. Branch with isAndroid from @/lib/platform.ts and render a supported fallback or translated unsupported state.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/call-video-feeds/video-player-modal.tsx` at line 56, Update
the NativeVideoPlayer rendering branch in VideoPlayerModal to check isAndroid
from `@/lib/platform.ts` before passing contentType 'dash'; on non-Android
platforms, render the supported fallback or translated unsupported state instead
of attempting DASH playback, while preserving HLS and Android DASH behavior.
Source: Coding guidelines
| private async loadSound(module: number): Promise<AudioPlayer | null> { | ||
| try { | ||
| const { sound } = await Audio.Sound.createAsync(module); | ||
| return sound; | ||
| const player = createAudioPlayer(module, { keepAudioSessionActive: true }); | ||
| player.loop = false; | ||
| player.volume = 1.0; | ||
| return player; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Prevent concurrent initialization from orphaning players.
The constructor starts initializeAudio(), and initialize() can start it again before isInitialized becomes true. Each call creates five players. Later calls overwrite the player fields, so cleanup() cannot remove the earlier players.
Add an initializationPromise guard, as used by NotificationSoundService. Add a concurrent initialize() test that verifies exactly five players are created.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/services/audio.service.ts` around lines 108 - 113, Guard initialization
in initializeAudio/initialize with an initializationPromise, reusing the
in-flight promise when initialization is already running and clearing it
appropriately after completion. Preserve the existing isInitialized behavior and
ensure concurrent callers create only one set of five players; add a concurrent
initialize() test asserting exactly five players are created, following
NotificationSoundService’s pattern.
| await setAudioModeAsync({ | ||
| allowsRecording: false, | ||
| shouldPlayInBackground: false, | ||
| playsInSilentMode: true, | ||
| interruptionMode: 'duckOthers', | ||
| shouldRouteThroughEarpiece: false, |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)notification-sound\.service\.ts$|(^|/)audio\.service\.ts$|(^|/)audio-stream-store\.ts$|(^|/)audio-stream-refactoring\.md$|expo|audio' | sed -n '1,200p'
echo "== target snippets with line numbers =="
for f in src/services/notification-sound.service.ts src/services/audio.service.ts src/stores/app/audio-stream-store.ts; do
if [ -f "$f" ]; then
echo "--- $f ---"
wc -l "$f"
sed -n '1,130p' "$f" | nl -ba
fi
done
echo "== setAudioModeAsync usages =="
rg -n "setAudioModeAsync|configureAsync|stopAsync|AudioMode|shouldPlayInBackground|interruptionMode|allowsRecording|playsInSilentMode|shouldRouteThroughEarpiece" src docs -S || true
echo "== package expo audio version/context =="
for f in package.json package-lock.json yarn.lock bun.lockb pnpm-lock.yaml; do
[ -f "$f" ] && { echo "--- $f ---"; rg -n "expo/audio|expo-audio|`@expo/config`" "$f" -S | sed -n '1,120p' || true; }
done
echo "== structural outline of target TS files =="
for f in src/services/notification-sound.service.ts src/services/audio.service.ts src/stores/app/audio-stream-store.ts; do
[ -f "$f" ] && { echo "--- outline $f ---"; ast-grep outline "$f" --view compact 2>/dev/null || true; }
doneRepository: Resgrid/Unit
Length of output: 4229
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== parse target TypeScript files for Audio calls and dependencies =="
python3 - <<'PY'
import ast, pathlib, re
files = [
pathlib.Path('src/services/notification-sound.service.ts'),
pathlib.Path('src/services/audio.service.ts'),
pathlib.Path('src/stores/app/audio-stream-store.ts'),
]
for p in files:
if not p.exists():
continue
text = p.read_text()
print(f'--- {p} ---')
print('imports:')
for m in re.finditer(r"import\s+([^;\n]+);", text):
line = text.count('\n', 0, m.start()) + 1
print(f'{line}: {m.group(1)}')
print('setAudioModeAsync calls:')
for m in re.finditer(r"setAudioModeAsync\s*\(", text):
line = text.count('\n', 0, m.start()) + 1
print(f'{line}: call starts at {m.start()}')
# print 300 chars
s = text[m.start(): m.start()+500].split('\n', 10)[:-1]
print(' '.join([''.join(s)]))
PYRepository: Resgrid/Unit
Length of output: 2204
🌐 Web query:
Expo Audio setAudioModeAsync shouldPlayInBackground interruptionMode documentation
💡 Result:
In the expo-audio library, setAudioModeAsync is the function used to configure global audio behavior, including background playback and interruption handling [1][2]. shouldPlayInBackground The shouldPlayInBackground property (boolean) determines whether the audio session remains active when the application moves to the background [1][3]. By default, this is set to false [1][4]. Note that on Android, simply setting this to true is often insufficient for sustained playback; you must also enable lockscreen controls using setActiveForLockScreen, or the OS may terminate audio playback after approximately 3 minutes [1][5][4]. Additionally, ensure that the appropriate config-plugin is configured for your project [1][3]. interruptionMode The interruptionMode property defines how your app's audio session interacts with other audio sessions on the device [1][4]. For scenarios involving background playback, it is often recommended to set this to 'doNotMix' to ensure proper OS-level management of audio focus and to allow lockscreen controls to associate correctly with your player [4]. Example configuration: import { setAudioModeAsync } from 'expo-audio'; await setAudioModeAsync({ playsInSilentMode: true, shouldPlayInBackground: true, interruptionMode: 'doNotMix' // Often required for correct lockscreen behavior });
Citations:
- 1: https://docs.expo.dev/versions/latest/sdk/audio/
- 2: https://docs.expo.dev/versions/v57.0.0/sdk/audio/
- 3: https://docs.expo.dev/versions/unversioned/sdk/audio
- 4: https://docs.expo.dev/versions/v55.0.0/sdk/audio
- 5: https://docs.expo.dev/versions/v56.0.0/sdk/audio/
🌐 Web query:
Expo AudioMode configuration Android duckOthers mixWithOthers shouldPlayInBackground lockscreen
💡 Result:
To configure audio behavior on Android using the modern expo-audio library (or the older expo-av), you must address both the audio session settings and the requirements for background execution [1][2][3]. 1. Audio Session Configuration (setAudioModeAsync) Use Audio.setAudioModeAsync to define how your app interacts with other audio on the device [1][2]. - interruptionMode: This controls audio focus [1][2]. Options include 'doNotMix' (exclusive focus, pauses others), 'duckOthers' (lowers others' volume), and 'mixWithOthers' (plays alongside, no focus request) [1][2]. - shouldPlayInBackground: Set this to true to allow the session to stay active when the app moves to the background [1][2]. 2. Background Playback & Lock Screen (Android) On Android, simply setting shouldPlayInBackground to true is insufficient for long-term playback [1][2]. - Sustained Background Playback: To prevent the Android OS from killing your audio after approximately 3 minutes, you must enable lock screen controls using the player's setActiveForLockScreen method [1][2][3]. - Lock Screen Requirement: When using setActiveForLockScreen, you should set the interruptionMode to 'doNotMix' in setAudioModeAsync to ensure the OS correctly associates the lock screen controls with your player [4]. - Config Plugin: You must configure the expo-audio config plugin in your app.json (or app.config.js) to enable background audio [1][2][3]. This plugin automatically adds necessary permissions (FOREGROUND_SERVICE, FOREGROUND_SERVICE_MEDIA_PLAYBACK) and declares the required media playback service (AudioControlsService) in your AndroidManifest.xml [5][3]. Summary of Key Properties: - shouldPlayInBackground (boolean): Must be true for background audio [1][2]. - interruptionMode (string): Use 'doNotMix' for best compatibility with lock screen controls [4]. - setActiveForLockScreen (method): Required for sustained Android background playback [1][2][3].
Citations:
- 1: https://docs.expo.dev/versions/latest/sdk/audio/
- 2: https://docs.expo.dev/versions/v57.0.0/sdk/audio/
- 3: https://github.com/expo/expo/blob/main/docs/pages/versions/unversioned/sdk/audio.mdx
- 4: https://github.com/expo/expo/blob/073ace8d/packages/expo-audio/build/AudioModule.types.d.ts
- 5: https://docs.expo.dev/versions/v55.0.0/sdk/audio
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== target section contents =="
python3 - <<'PY'
from pathlib import Path
for path, start, end in [
("src/services/notification-sound.service.ts", 45, 70),
("src/services/audio.service.ts", 38, 60),
("src/stores/app/audio-stream-store.ts", 95, 120),
]:
p = Path(path)
lines = p.read_text().splitlines()
print(f"--- {path} lines {start}-{end} ---")
for i in range(start - 1, min(end, len(lines))):
print(f"{i+1}: {lines[i]}")
print()
PY
echo "== expo-audio call sites and setActiveAudioDevice / setActiveForLockScreen usage =="
rg -n "setAudioModeAsync|setActiveAudioDevice|setActiveForLockScreen|createAudioPlayer|AudioMode|shouldPlayInBackground|interruptionMode|duckOthers|mixWithOthers|doNotMix|playsInSilentMode" src --glob "*.ts" --glob "*.tsx" -S || true
echo "== relevant docs excerpts =="
for f in docs/audio-stream-refactoring.md docs/audio-service-direct-playback.md; do
if [ -f "$f" ]; then
echo "--- $f ---"
wc -l "$f"
sed -n '1,120p' "$f"
fi
done
echo "== audio store lifecycle =="
python3 - <<'PY'
from pathlib import Path
p = Path("src/stores/app/audio-stream-store.ts")
text = p.read_text()
for token in ["playAudioStream", "stopAudioStream", "pauseAudioStream", "resumeAudioStream", "initializeAudioStream", "audioPlayer"]:
idx = text.find(token)
print(f"--- {token} at {text.count(chr(10), 0, idx)+1 if idx!=-1 else 'not found'} ---")
if idx != -1:
ctx = text[max(0, idx-400): idx+1200]
for i, line in enumerate(ctx.splitlines(), start=max(1, text[:idx].count("\n")+1)):
if i <= 60:
print(f"{i}: {line}")
PY
echo "== service lifecycle snippets =="
python3 - <<'PY'
from pathlib import Path
for path in ["src/services/notification-sound.service.ts", "src/services/audio.service.ts"]:
p=Path(path)
text=p.read_text()
for token in ["getInstance", "init", "initialize", "loadAudioFiles", "stopAudio", "stopAllAudio"]:
idx=text.find(token)
print(f"--- {token} at {text.count(chr(10), 0, idx)+1 if idx!=-1 else 'not found'} ---")
if idx!=-1:
ctx=text[max(0, idx-400): idx+800]
for i,line in enumerate(ctx.splitlines(), start=max(1,text[:idx].count("\n")+1)):
if i<=40:
print(f"{i}: {line}")
PYRepository: Resgrid/Unit
Length of output: 19369
Centralize setAudioModeAsync ownership.
These components call the same global audio-mode API during independent initialization paths, so one can replace the other's active session settings. Route all audio-mode requests through shared coordinator logic that derives shouldPlayInBackground, interruptionMode, and related flags from the current active audio operation. Add an integration test that starts a stream and then initializes audio/notification sound players before asserting the stream session remains active.
src/services/notification-sound.service.ts: remove direct application of the persistent audio mode.src/services/audio.service.ts: request audio-mode ownership through the shared coordinator.src/stores/app/audio-stream-store.ts: request the stream audio mode through the shared coordinator.
📍 Affects 3 files
src/services/notification-sound.service.ts#L58-L63(this comment)src/services/audio.service.ts#L46-L51src/stores/app/audio-stream-store.ts#L102-L108
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/services/notification-sound.service.ts` around lines 58 - 63, Centralize
ownership of global audio-mode configuration through a shared coordinator:
remove the direct persistent setAudioModeAsync application in
src/services/notification-sound.service.ts at lines 58-63, and update the audio
initialization in src/services/audio.service.ts at lines 46-51 and stream setup
in src/stores/app/audio-stream-store.ts at lines 102-108 to request mode changes
through that coordinator. Have the coordinator derive shouldPlayInBackground,
interruptionMode, and related flags from the active audio operation, and add an
integration test covering stream startup followed by audio/notification sound
initialization while preserving the stream session.
| await setAudioModeAsync({ | ||
| allowsRecording: false, | ||
| shouldPlayInBackground: true, | ||
| playsInSilentMode: true, | ||
| interruptionMode: 'duckOthers', | ||
| shouldRouteThroughEarpiece: false, | ||
| }); | ||
|
|
||
| // Create new sound object | ||
| const { sound } = await Audio.Sound.createAsync( | ||
| { uri: stream.Url } as AVPlaybackSource, | ||
| { | ||
| shouldPlay: false, | ||
| isLooping: false, | ||
| volume: 1.0, | ||
| isMuted: false, | ||
| progressUpdateIntervalMillis: 1000, | ||
| }, | ||
| (status: AVPlaybackStatus) => { | ||
| if (status.isLoaded) { | ||
| const { isPlaying, isBuffering } = get(); | ||
|
|
||
| if (status.isPlaying !== isPlaying) { | ||
| set({ isPlaying: status.isPlaying }); | ||
| } | ||
| const sound = createAudioPlayer(stream.Url, { | ||
| updateInterval: 1000, | ||
| keepAudioSessionActive: true, | ||
| preferredForwardBufferDuration: 5, | ||
| }); | ||
| sound.loop = false; | ||
| sound.volume = 1.0; | ||
| sound.muted = false; | ||
|
|
||
| set({ soundObject: sound, currentStream: stream }); | ||
|
|
||
| sound.addListener('playbackStatusUpdate', (status: AudioStatus) => { | ||
| if (get().soundObject !== sound) { | ||
| return; | ||
| } | ||
|
|
||
| if (status.isBuffering !== isBuffering) { | ||
| set({ isBuffering: status.isBuffering }); | ||
| if (status.error) { | ||
| logger.error({ | ||
| message: 'Audio playback error', | ||
| context: { error: status.error, streamName: stream.Name }, | ||
| }); | ||
| sound.remove(); | ||
| set({ | ||
| soundObject: null, | ||
| currentStream: null, | ||
| isPlaying: false, | ||
| isLoading: false, | ||
| isBuffering: false, | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| const { isPlaying, isBuffering } = get(); | ||
| if (status.playing !== isPlaying) { | ||
| set({ isPlaying: status.playing }); | ||
| } | ||
| if (status.isBuffering !== isBuffering) { | ||
| set({ isBuffering: status.isBuffering }); | ||
| } | ||
|
|
||
| if (status.didJustFinish) { | ||
| logger.info({ | ||
| message: 'Audio stream finished', | ||
| context: { streamName: stream.Name }, | ||
| }); | ||
|
|
||
| setTimeout(async () => { | ||
| const { currentStream, soundObject } = get(); | ||
| if (currentStream?.Id !== stream.Id || soundObject !== sound) { | ||
| return; | ||
| } | ||
|
|
||
| // Handle stream ended/error scenarios | ||
| if (status.didJustFinish) { | ||
| logger.info({ | ||
| message: 'Audio stream finished', | ||
| context: { streamName: stream.Name }, | ||
| try { | ||
| await sound.seekTo(0); | ||
| sound.play(); | ||
| } catch (replayError) { | ||
| logger.error({ | ||
| message: 'Failed to restart audio stream', | ||
| context: { error: replayError, streamName: stream.Name }, | ||
| }); | ||
|
|
||
| // For live streams, try to reconnect | ||
| const { currentStream } = get(); | ||
| if (currentStream?.Id === stream.Id) { | ||
| setTimeout(async () => { | ||
| try { | ||
| await sound.replayAsync(); | ||
| } catch (replayError) { | ||
| logger.error({ | ||
| message: 'Failed to restart audio stream', | ||
| context: { error: replayError, streamName: stream.Name }, | ||
| }); | ||
| } | ||
| }, 1000); | ||
| } | ||
| } | ||
| } else { | ||
| // Handle error state | ||
| logger.error({ | ||
| message: 'Audio playback error', | ||
| context: { error: 'Failed to load audio', streamName: stream.Name }, | ||
| }); | ||
| set({ | ||
| soundObject: null, | ||
| currentStream: null, | ||
| isPlaying: false, | ||
| isLoading: false, | ||
| isBuffering: false, | ||
| }); | ||
| } | ||
| }, 1000); | ||
| } | ||
| ); | ||
| }); | ||
|
|
||
| // Start playing | ||
| await sound.playAsync(); | ||
| sound.play(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate files =="
git ls-files | rg '(^src/stores/app/audio-stream-store\.ts$|docs/audio-stream-refactoring\.md$|package\.json$|app\.json$.*)' || true
echo
echo "== package expo version =="
if [ -f package.json ]; then
node -e 'const p=require("./package.json"); console.log(JSON.stringify({dependencies:p.dependencies, devDependencies:p.devDependencies}, null, 2))' | rg -n '"expo"|"expo-router"|"expo-audio"|"unimodules"|"react-native-keep-awake"' || true
fi
echo
echo "== app config audio permissions =="
fd -a 'app.json$|app.config.js$|app.config.ts$' . | while read -r f; do
echo "--- $f"
sed -n '1,220p' "$f" | rg -n 'audio|microphone|backgroundMode|background_audio|ios|android|plugins|EXPO|expo' -C 2 || true
done
echo
echo "== audio stream store outline/state =="
if [ -f src/stores/app/audio-stream-store.ts ]; then
wc -l src/stores/app/audio-stream-store.ts
ast-grep outline src/stores/app/audio-stream-store.ts --view compact || true
echo "--- lines 1-240"
sed -n '1,240p' src/stores/app/audio-stream-store.ts | nl -ba
fi
echo
echo "== docs audio stream refactoring relevant lines =="
if [ -f docs/audio-stream-refactoring.md ]; then
sed -n '1,160p' docs/audio-stream-refactoring.md | nl -ba
fi
echo
echo "== search audio/lockscreen/android docs references in repo =="
rg -n "lock.*screen|lock screen|doNotMix|sustained|background.*play|background audio|setAudioModeAsync|createAudioPlayer|playbackStatusUpdate|BackgroundAudioManager|updateMetadata|setMetadata|Android|android" . --glob '!node_modules' --glob '!dist' --glob '!build' | head -200Repository: Resgrid/Unit
Length of output: 4968
🌐 Web query:
Expo SDK 56 audio background audio Android continuous playback lock screen controls setAudioModeAsync doNotMix documentation
💡 Result:
To enable continuous background audio playback with lock screen controls on Android using Expo SDK 56 and the expo-audio library, you must follow these requirements: 1. Configure the Config Plugin: Ensure your app config is set up to include the required native permissions and foreground service declarations. The expo-audio config plugin automatically handles this when configured correctly [1][2]. It adds the necessary FOREGROUND_SERVICE and FOREGROUND_SERVICE_MEDIA_PLAYBACK permissions and declares the AudioControlsService in your AndroidManifest.xml [1][3]. 2. Configure Audio Mode: You must use setAudioModeAsync to set the interruptionMode to "doNotMix" [1][4]. This setting is mandatory for the OS to correctly associate lock screen controls with your audio player [4][5][6]. Example configuration: setAudioModeAsync({ playsInSilentMode: true, shouldPlayInBackground: true, interruptionMode: 'doNotMix', }); 3. Enable Lock Screen Controls: On Android, you must explicitly enable lock screen controls using the setActiveForLockScreen method on your audio player instance [1][2]. This is required for sustained background playback; otherwise, the Android OS will stop audio playback after approximately 3 minutes [1][7]. Example usage: player.setActiveForLockScreen(true, { title: 'My Audio Title', artist: 'Artist Name', artworkUrl: 'https://example.com/artwork.jpg', }); If interruptionMode is not set to "doNotMix", the lock screen controls may fail to appear or function, and background playback may not be sustained correctly [4][5][6].
Citations:
- 1: https://docs.expo.dev/versions/v56.0.0/sdk/audio/
- 2: https://docs.expo.dev/versions/latest/sdk/audio/
- 3: https://github.com/expo/expo/blob/main/docs/pages/versions/unversioned/sdk/audio.mdx
- 4: https://docs.expo.dev/versions/v55.0.0/sdk/audio
- 5: https://github.com/expo/expo/blob/073ace8d/packages/expo-audio/build/AudioModule.types.d.ts
- 6: Expo lockscreen controls expo/expo#28068
- 7: https://docs.expo.dev/versions/v57.0.0/sdk/audio/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== audio stream store exact contents =="
python3 - <<'PY'
from pathlib import Path
p = Path('src/stores/app/audio-stream-store.ts')
lines = p.read_text().splitlines()
print(f'total-lines={len(lines)}')
for i in range(1, min(270, len(lines)+1)):
print(f'{i:4}: {lines[i-1]}')
PY
echo
echo "== docs audio stream refactoring relevant contents =="
python3 - <<'PY'
from pathlib import Path
p = Path('docs/audio-stream-refactoring.md')
lines = p.read_text().splitlines()
for i in range(1, min(180, len(lines)+1)):
print(f'{i:4}: {lines[i-1]}')
PY
echo
echo "== app.config.ts android/audio related range =="
python3 - <<'PY'
from pathlib import Path
p = Path('app.config.ts')
lines = p.read_text().splitlines()
for i in range(1, len(lines)+1):
if any(term in lines[i-1].lower() for term in ['audio','android','permissions','plugin','expo']):
start=max(1,i-3); end=min(len(lines), i+3)
print(f'--- vicinity line {i}')
for j in range(start,end+1):
print(f'{j:4}: {lines[j-1]}')
PYRepository: Resgrid/Unit
Length of output: 30077
Activate Android lock-screen controls for sustained audio playback.
On SDK 56, background audio with lock-screen controls requires setAudioModeAsync({ interruptionMode: 'doNotMix' }) and player metadata. This stream uses duckOthers and never calls the lock-screen metadata API, so the Android player is still at risk of stopping after the background audio timeout. Activate controls with stream metadata before playback, use doNotMix, deactivate controls before removal, and add this requirement to docs/audio-stream-refactoring.md.
📍 Affects 2 files
src/stores/app/audio-stream-store.ts#L102-L176(this comment)docs/audio-stream-refactoring.md#L48-L56
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/stores/app/audio-stream-store.ts` around lines 102 - 176, Update audio
playback setup in src/stores/app/audio-stream-store.ts around createAudioPlayer
to use interruptionMode 'doNotMix', configure lock-screen controls with the
stream metadata before sound.play(), and deactivate those controls before every
sound.remove() path, including playback errors. Document this
lock-screen-control requirement in docs/audio-stream-refactoring.md at lines
48-56.
| await setAudioModeAsync({ | ||
| allowsRecording: true, | ||
| shouldPlayInBackground: true, | ||
| playsInSilentMode: true, | ||
| interruptionMode: 'mixWithOthers', |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 4 'interruptionMode|AudioSession\.startAudioSession|applyAudioRouting' \
src/stores/app/livekit-store.ts src/stores/app/__tests__Repository: Resgrid/Unit
Length of output: 4192
🌐 Web query:
Expo Audio 56 setAudioModeAsync interruptionMode mixWithOthers duckOthers noDuckOthers Android audio focus
💡 Result:
In the expo-audio library (the successor to expo-av), managing audio focus and interruption modes on Android has been a known area of technical evolution and active development throughout 2025 and 2026 [1][2][3]. As of the latest documentation, the interruptionMode property (which replaces the deprecated interruptionModeAndroid) is the intended way to configure how your audio session interacts with others on both Android and iOS [4][5]. However, users have encountered significant challenges regarding the reliability of these modes on Android [2][3]. Summary of Behavior and Focus Modes: 1. doNotMix: Requests exclusive audio focus, causing other apps to pause their audio [5][6]. 2. duckOthers: Requests audio focus with ducking, intended to lower the volume of other playing apps rather than pausing them [5][6]. 3. mixWithOthers: Audio plays alongside other apps without requesting audio focus [5][6]. Key Implementation Context for Android: - Configuration: Use setAudioModeAsync to define the interruptionMode. While documentation indicates it works across platforms, early versions of expo-audio faced issues where the interruptionMode string was not properly cast or recognized on Android [7]. Always ensure you are using the most recent version of expo-audio to benefit from ongoing stability fixes regarding focus management [8][9]. - Known Issues: Developers have reported inconsistent behavior with "ducking" on Android, specifically regarding the failure to restore the original volume of background apps ("unducking") after the app finishes playing audio [2][3]. This is a distinct behavior from the older expo-av library, and fixes have been released incrementally to address focus management and "unducking" reliability [8]. - Troubleshooting: If you experience issues where modes are ignored or behave unexpectedly (e.g., permanent ducking), ensure that you are not experiencing shadowing of focus request objects in your custom implementations [8]. Some community workarounds have involved checking for playback status and explicitly managing focus, but relying on the built-in library methods is recommended as the module matures [8][3]. For critical background playback, remember that on Android, you must enable lockscreen controls using player.setActiveForLockScreen(true, ...) to prevent the OS from terminating your audio session after approximately 3 minutes of background activity [4][10].
Citations:
- 1: expo-audio does not allow to set interruptionMode on Android expo/expo#34473
- 2: Ducking modes in expo-audio on Android does not work as expected (as in expo-av) expo/expo#37694
- 3: [expo-audio] There is no way to "Unduck" a background song on Android expo/expo#37776
- 4: https://docs.expo.dev/versions/latest/sdk/audio/
- 5: https://docs.expo.dev/versions/v57.0.0/sdk/audio/
- 6: https://docs.expo.dev/versions/v55.0.0/sdk/audio
- 7: [expo-audio][android] Unable to use interruptionMode option expo/expo#34025
- 8: [android][audio] Fix focus management issues expo/expo#37698
- 9: [audio] Add support for mixWithOthers to interruptionModeAndroid expo/expo#38415
- 10: https://docs.expo.dev/versions/v56.0.0/sdk/audio/
🌐 Web query:
site:docs.expo.dev versions/v56.0.0/sdk/audio setAudioModeAsync interruptionMode "mixWithOthers"
💡 Result:
In Expo SDK 56.0.0, the interruptionMode property in Audio.setAudioModeAsync() determines how your application's audio session interacts with other audio sessions on the device [1][2]. The mixWithOthers value is one of the available options for this property [1][2]. The mixWithOthers setting allows your app's audio to play simultaneously alongside audio from other applications without interrupting them [1][2]. On Android specifically, this setting means no audio focus is requested [1][2]. It is generally recommended for use cases such as sound effects, UI feedback, or short audio clips [1][2]. Important implementation notes for interruptionMode in SDK 56.0.0 include: - interruptionMode is the unified, preferred property that works across both Android and iOS [1][2]. - Older properties like interruptionModeAndroid and interruptionModeIOS are considered deprecated in favor of this single, platform-agnostic property [1][2]. - If you require lock screen controls to function correctly for your audio player, you must use doNotMix instead of mixWithOthers, as the operating system may fail to associate lock screen controls with your player otherwise [1][2]. Other available interruptionMode options include: - doNotMix: Requests exclusive audio focus, causing other apps to pause their audio [1][2]. - duckOthers: Requests audio focus with ducking, causing other apps to lower their volume while continuing to play [1][2].
Citations:
- 1: https://docs.expo.dev/versions/v56.0.0/sdk/audio/
- 2: https://docs.expo.dev/versions/v56.0.0/sdk/audio
Use a PTT-specific Android interruption mode.
interruptionMode: 'mixWithOthers' applies on Android and requests no audio focus, so other audio can continue during the LiveKit call without focus-loss callbacks. Use doNotMix for exclusive audio, or duckOthers if the call must coexist but duck competing audio, and document why mixing is intended for PTT.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/stores/app/livekit-store.ts` around lines 64 - 68, Update the
setAudioModeAsync configuration in the LiveKit audio setup to use a
PTT-appropriate Android interruption mode instead of mixWithOthers: choose
doNotMix for exclusive call audio or duckOthers when competing audio should be
reduced, and document the rationale if mixing remains intentional.
| // iOS handling (expo-audio configures the mode while CallKeep handles the session) | ||
| // Just ensure the mode is correct | ||
| await Audio.setAudioModeAsync({ | ||
| allowsRecordingIOS: true, | ||
| staysActiveInBackground: true, | ||
| playsInSilentModeIOS: true, | ||
| shouldDuckAndroid: false, | ||
| playThroughEarpieceAndroid: true, | ||
| interruptionModeIOS: InterruptionModeIOS.MixWithOthers, | ||
| await setAudioModeAsync({ | ||
| allowsRecording: true, | ||
| shouldPlayInBackground: true, | ||
| playsInSilentMode: true, | ||
| interruptionMode: 'mixWithOthers', | ||
| shouldRouteThroughEarpiece: true, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 5 'applyAudioRouting|selectedAudioDevices\.speaker|shouldRouteThroughEarpiece' \
src/stores/app/livekit-store.ts src/stores/app/__tests__Repository: Resgrid/Unit
Length of output: 4734
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,180p' src/stores/app/livekit-store.ts | cat -nRepository: Resgrid/Unit
Length of output: 8546
🌐 Web query:
Expo 56 setAudioModeAsync shouldRouteThroughEarpiece true false speaker earpiece
💡 Result:
In Expo, the shouldRouteThroughEarpiece property is used within the setAudioModeAsync function to control audio output routing [1][2]. When set to true, audio is routed through the device's earpiece (the speaker used for phone calls); when set to false, it is routed through the main speaker [1][2]. Key details regarding this property include: - Default Behavior: The default value is false, meaning audio is routed through the main speaker [1][2]. - iOS Limitations: On iOS, this setting only has an effect when allowsRecording is true (which puts the audio session into the.playAndRecord category) [1][2]. - API Evolution: In newer versions of Expo (using the expo-audio library), this property is documented as part of the audio mode configuration [1][2]. In older versions of Expo (using the deprecated expo-av library), similar behavior was often managed via the allowsRecordingIOS property, which could cause audio to route to the earpiece during recording sessions [3]. To route audio to the earpiece, you would call: await setAudioModeAsync({ shouldRouteThroughEarpiece: true, //... other configuration options }); To return audio to the main speaker, simply set it back to false: await setAudioModeAsync({ shouldRouteThroughEarpiece: false, });
Citations:
- 1: https://docs.expo.dev/versions/latest/sdk/audio/
- 2: https://docs.expo.dev/versions/v56.0.0/sdk/audio/
- 3: https://docs.expo.dev/versions/v54.0.0/sdk/audio-av/
Honor the selected iOS route.
setupAudioRouting selects 'speaker', but the iOS setAudioModeAsync call always passes shouldRouteThroughEarpiece: true. Expo routes true to the earpiece and false to the speaker, so speaker selection is ignored.
Proposed fix
interruptionMode: 'mix WithOthers',
- shouldRouteThroughEarpiece: true,
+ shouldRouteThroughEarpiece: normalizedDeviceType !== 'speaker',🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/stores/app/livekit-store.ts` around lines 90 - 97, Update
setupAudioRouting’s iOS setAudioModeAsync configuration so
shouldRouteThroughEarpiece reflects the selected route: use true for earpiece
and false for speaker, preserving the existing audio-mode settings.
| ### 3. Enhanced Audio Configuration | ||
| import { createAudioPlayer, setAudioModeAsync, type AudioPlayer } from 'expo-audio'; | ||
|
|
||
| await setAudioModeAsync({ |
There was a problem hiding this comment.
Unhandled promise rejection risk identified where the awaited setAudioModeAsync call lacks a try/catch guard. Wrap the operation in a try/catch block to comply with Rule [1] and degrade gracefully.
Kody rule violation: Handle async operations with proper error handling
Prompt for LLM
File docs/audio-stream-refactoring.md:
Line 12:
Unhandled promise rejection risk identified where the awaited `setAudioModeAsync` call lacks a try/catch guard. Wrap the operation in a try/catch block to comply with Rule [1] and degrade gracefully.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| ### 3. Enhanced Audio Configuration | ||
| import { createAudioPlayer, setAudioModeAsync, type AudioPlayer } from 'expo-audio'; | ||
|
|
||
| await setAudioModeAsync({ |
There was a problem hiding this comment.
OS-level external call setAudioModeAsync lacks error handling and mapping to application-level errors. Wrap the call in a try/catch block to comply with Rule [27], include the operation name and context in structured logging, and surface a typed error.
Kody rule violation: Add try-catch blocks for external calls
Prompt for LLM
File docs/audio-stream-refactoring.md:
Line 12:
OS-level external call `setAudioModeAsync` lacks error handling and mapping to application-level errors. Wrap the call in a try/catch block to comply with Rule [27], include the operation name and context in structured logging, and surface a typed error.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| playsInSilentModeIOS: true, | ||
| shouldDuckAndroid: true, | ||
| playThroughEarpieceAndroid: false, | ||
| player.addListener('playbackStatusUpdate', (status) => { |
There was a problem hiding this comment.
Missing error handler and indeterminate cleanup path identified for the player.addListener subscription. Implement explicit status.error handling inside the callback and store the subscription object for removal during teardown to comply with Rule [4].
Kody rule violation: Provide error handlers to subscription/listener APIs
Prompt for LLM
File docs/audio-stream-refactoring.md:
Line 26:
Missing error handler and indeterminate cleanup path identified for the `player.addListener` subscription. Implement explicit `status.error` handling inside the callback and store the subscription object for removal during teardown to comply with Rule [4].
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| Added proper audio mode configuration for streaming: | ||
| const player: AudioPlayer = createAudioPlayer(stream.Url, { | ||
| updateInterval: 1000, |
There was a problem hiding this comment.
Magic number identified where the numeric literal 1000 is used inline for the updateInterval configuration. Extract this domain-specific value into a named constant like PLAYBACK_UPDATE_INTERVAL_MS to comply with Rule [9].
Kody rule violation: Replace magic numbers with named constants
Prompt for LLM
File docs/audio-stream-refactoring.md:
Line 21:
Magic number identified where the numeric literal `1000` is used inline for the `updateInterval` configuration. Extract this domain-specific value into a named constant like `PLAYBACK_UPDATE_INTERVAL_MS` to comply with Rule [9].
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| playsInSilentModeIOS: true, | ||
| shouldDuckAndroid: true, | ||
| playThroughEarpieceAndroid: false, | ||
| player.addListener('playbackStatusUpdate', (status) => { |
There was a problem hiding this comment.
Memory leak risk identified where the event listener registered via player.addListener lacks removal during cleanup. Capture the returned subscription and call remove() on it to comply with Rule [53].
Kody rule violation: Proper memory management in event listeners
Prompt for LLM
File docs/audio-stream-refactoring.md:
Line 26:
Memory leak risk identified where the event listener registered via `player.addListener` lacks removal during cleanup. Capture the returned subscription and call `remove()` on it to comply with Rule [53].
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| const player = useVideoPlayer({ uri, contentType }, (videoPlayer) => { | ||
| videoPlayer.play(); | ||
| }); |
There was a problem hiding this comment.
The useVideoPlayer source passes an unrecognized contentType field, preventing the native player from identifying adaptive streams. Construct the source using the discriminated shapes { hls: uri } or { dash: uri } based on the format.
const source = contentType === 'hls' ? { hls: uri } : { dash: uri };
const player = useVideoPlayer(source, (videoPlayer) => {
videoPlayer.play();
});Prompt for LLM
File src/components/call-video-feeds/video-player-modal.tsx:
Line 30 to 32:
The `useVideoPlayer` source passes an unrecognized `contentType` field, preventing the native player from identifying adaptive streams. Construct the source using the discriminated shapes `{ hls: uri }` or `{ dash: uri }` based on the format.
Suggested Code:
const source = contentType === 'hls' ? { hls: uri } : { dash: uri };
const player = useVideoPlayer(source, (videoPlayer) => {
videoPlayer.play();
});
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| case FeedFormat.HLS: | ||
| case FeedFormat.DASH: | ||
| return <Video source={{ uri: feed.Url }} style={styles.video} useNativeControls resizeMode={ResizeMode.CONTAIN} shouldPlay />; | ||
| return <NativeVideoPlayer key={`${feed.FeedFormat}:${feed.Url}`} uri={feed.Url} contentType={feed.FeedFormat === FeedFormat.HLS ? 'hls' : 'dash'} />; |
There was a problem hiding this comment.
Code duplication identified where shared string literals 'hls' and 'dash' are inlined at the call site and within the NativeVideoPlayerProps type (line 26). Extract these strings into a single source of truth to comply with Rule [6] and prevent drift.
Kody rule violation: Centralize string constants
Prompt for LLM
File src/components/call-video-feeds/video-player-modal.tsx:
Line 56:
Code duplication identified where shared string literals `'hls'` and `'dash'` are inlined at the call site and within the `NativeVideoPlayerProps` type (line 26). Extract these strings into a single source of truth to comply with Rule [6] and prevent drift.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| shouldPlayInBackground: true, | ||
| playsInSilentMode: true, | ||
| shouldRouteThroughEarpiece: false, | ||
| interruptionMode: 'mixWithOthers', |
There was a problem hiding this comment.
Type regression identified where the interruptionMode field uses a raw string literal 'mixWithOthers' instead of the previous InterruptionModeIOS enum. Import an interruption-mode enum or define a shared typed constant to prevent brittle, typo-prone code.
Kody rule violation: Use enums instead of magic strings
Prompt for LLM
File src/services/__tests__/audio.service.test.ts:
Line 120:
Type regression identified where the `interruptionMode` field uses a raw string literal `'mixWithOthers'` instead of the previous `InterruptionModeIOS` enum. Import an interruption-mode enum or define a shared typed constant to prevent brittle, typo-prone code.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| context: { streamName: stream.Name }, | ||
| }); | ||
|
|
||
| setTimeout(async () => { |
There was a problem hiding this comment.
Indeterminate cleanup path identified where a setTimeout return value is never captured. Store the timer ID and call clearTimeout(timerId) in the stop()/teardown path or when sound.remove() is called to prevent lingering timers.
Kody rule violation: Clear timers on teardown/unmount
Prompt for LLM
File src/stores/app/audio-stream-store.ts:
Line 156:
Indeterminate cleanup path identified where a `setTimeout` return value is never captured. Store the timer ID and call `clearTimeout(timerId)` in the `stop()`/teardown path or when `sound.remove()` is called to prevent lingering timers.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| } catch { | ||
| // The player may already have been released by an error event. | ||
| } |
There was a problem hiding this comment.
Swallowed exception identified in the catch block at lines 200–202. Replace the explanatory comment with a logger.debug or logger.warn call containing the error context to comply with Rule 28 and ensure proper diagnostics.
Kody rule violation: Avoid empty catch blocks
Prompt for LLM
File src/stores/app/audio-stream-store.ts:
Line 200 to 202:
Swallowed exception identified in the catch block at lines 200–202. Replace the explanatory comment with a `logger.debug` or `logger.warn` call containing the error context to comply with Rule 28 and ensure proper diagnostics.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| await setAudioModeAsync({ | ||
| allowsRecording: true, | ||
| shouldPlayInBackground: true, | ||
| playsInSilentMode: true, | ||
| interruptionMode: 'mixWithOthers', | ||
| shouldRouteThroughEarpiece: true, | ||
| }); |
There was a problem hiding this comment.
The iOS branch of applyAudioRouting hardcodes shouldRouteThroughEarpiece: true, ignoring the deviceType argument and forcing audio to the earpiece even when 'speaker' is requested. Set shouldRouteThroughEarpiece: normalizedDeviceType !== 'speaker' to honor the request.
await setAudioModeAsync({
allowsRecording: true,
shouldPlayInBackground: true,
playsInSilentMode: true,
interruptionMode: 'mixWithOthers',
shouldRouteThroughEarpiece: normalizedDeviceType !== 'speaker',
});Prompt for LLM
File src/stores/app/livekit-store.ts:
Line 92 to 98:
The iOS branch of `applyAudioRouting` hardcodes `shouldRouteThroughEarpiece: true`, ignoring the `deviceType` argument and forcing audio to the earpiece even when `'speaker'` is requested. Set `shouldRouteThroughEarpiece: normalizedDeviceType !== 'speaker'` to honor the request.
Suggested Code:
await setAudioModeAsync({
allowsRecording: true,
shouldPlayInBackground: true,
playsInSilentMode: true,
interruptionMode: 'mixWithOthers',
shouldRouteThroughEarpiece: normalizedDeviceType !== 'speaker',
});
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| await setAudioModeAsync({ | ||
| allowsRecording: true, | ||
| shouldPlayInBackground: true, | ||
| playsInSilentMode: true, | ||
| interruptionMode: 'mixWithOthers', | ||
| shouldRouteThroughEarpiece: true, | ||
| }); |
There was a problem hiding this comment.
Code duplication identified where the setAudioModeAsync configuration block repeats logic from lines 64-71. Extract a shared helper like configureAudioMode(shouldRouteThroughEarpiece: boolean) to prevent drift.
Kody rule violation: Extract duplicated logic into functions
Prompt for LLM
File src/stores/app/livekit-store.ts:
Line 92 to 98:
Code duplication identified where the `setAudioModeAsync` configuration block repeats logic from lines 64-71. Extract a shared helper like `configureAudioMode(shouldRouteThroughEarpiece: boolean)` to prevent drift.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/stores/app/audio-stream-store.ts (1)
123-132: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPrevent an older request from replacing a newer player.
requestIdonly prevents stale error handling. If an older request resumes aftersetAudioModeAsyncand a newer request has already completed, this code creates the older player and overwritessoundObjectandcurrentStream. Both players can then play.Return when
requestId !== latestPlayRequestIdimmediately afterawait setAudioModeAsync. Add a test where the older setup resolves after the newer stream becomes active.As per coding guidelines, “Generate tests for all components, services and logic generated.”
Proposed fix
await setAudioModeAsync({ allowsRecording: false, shouldPlayInBackground: true, playsInSilentMode: true, interruptionMode: 'duckOthers', shouldRouteThroughEarpiece: false, }); + if (requestId !== latestPlayRequestId) { + return; + } + const sound = createAudioPlayer(streamUrl, {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/stores/app/audio-stream-store.ts` around lines 123 - 132, In the play-request flow, add a stale-request guard immediately after await setAudioModeAsync: return when requestId !== latestPlayRequestId before createAudioPlayer runs. Keep the existing soundObject/currentStream assignment unchanged for the latest request, and add a test covering an older setup resolving after a newer stream is active without replacing its player.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/stores/app/audio-stream-store.ts`:
- Around line 123-132: In the play-request flow, add a stale-request guard
immediately after await setAudioModeAsync: return when requestId !==
latestPlayRequestId before createAudioPlayer runs. Keep the existing
soundObject/currentStream assignment unchanged for the latest request, and add a
test covering an older setup resolving after a newer stream is active without
replacing its player.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3a32785a-23ba-4822-b390-5186be6dfcc1
📒 Files selected for processing (3)
docs/audio-stream-refactoring.mdsrc/stores/app/__tests__/audio-stream-store.test.tssrc/stores/app/audio-stream-store.ts
| logger.error({ | ||
| message: 'Failed to stop audio stream', | ||
| context: { error }, | ||
| }); |
There was a problem hiding this comment.
Missing structured context in the stop-stream failure log omits the operation name and relevant identifiers, violating Rule [3] and preventing stream correlation. Include structured fields in the logger context, such as op: 'stopStream', streamId: currentStream?.Id, and streamName: currentStream?.Name.
Kody rule violation: Include error context in structured logs
Prompt for LLM
File src/stores/app/audio-stream-store.ts:
Line 249 to 252:
Missing structured context in the stop-stream failure log omits the operation name and relevant identifiers, violating Rule [3] and preventing stream correlation. Include structured fields in the logger context, such as `op: 'stopStream'`, `streamId: currentStream?.Id`, and `streamName: currentStream?.Name`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| logger.debug({ | ||
| message: 'Starting audio stream', | ||
| context: { streamName: stream.Name, streamUrl: stream.Url }, | ||
| context: { streamName: stream.Name, streamUrl }, | ||
| }); | ||
|
|
||
| // Configure audio mode for streaming |
There was a problem hiding this comment.
Race condition in playStream allows overlapping calls from audio-stream-bottom-sheet.tsx to bypass the requestId staleness guard on the success path, orphaning the previous AudioPlayer when a newer request overwrites soundObject/currentStream. Add the if (requestId !== latestPlayRequestId) return; guard after await setAudioModeAsync(...) and before createAudioPlayer.
const requestId = ++latestPlayRequestId;
try {
const { soundObject: currentSound, stopStream } = get();
// Stop current stream if playing
if (currentSound) {
await stopStream();
}
set({ isLoading: true, isBuffering: true });
// Configure audio mode for streaming
await setAudioModeAsync({
allowsRecording: false,
shouldPlayInBackground: true,
playsInSilentMode: true,
interruptionMode: 'duckOthers',
shouldRouteThroughEarpiece: false,
});
// Bail out if a newer play request superseded this one
if (requestId !== latestPlayRequestId) {
return;
}
const sound = createAudioPlayer(streamUrl, {Prompt for LLM
File src/stores/app/audio-stream-store.ts:
Line 108 to 114:
Race condition in `playStream` allows overlapping calls from `audio-stream-bottom-sheet.tsx` to bypass the `requestId` staleness guard on the success path, orphaning the previous AudioPlayer when a newer request overwrites `soundObject`/`currentStream`. Add the `if (requestId !== latestPlayRequestId) return;` guard after `await setAudioModeAsync(...)` and before `createAudioPlayer`.
Suggested Code:
const requestId = ++latestPlayRequestId;
try {
const { soundObject: currentSound, stopStream } = get();
// Stop current stream if playing
if (currentSound) {
await stopStream();
}
set({ isLoading: true, isBuffering: true });
// Configure audio mode for streaming
await setAudioModeAsync({
allowsRecording: false,
shouldPlayInBackground: true,
playsInSilentMode: true,
interruptionMode: 'duckOthers',
shouldRouteThroughEarpiece: false,
});
// Bail out if a newer play request superseded this one
if (requestId !== latestPlayRequestId) {
return;
}
const sound = createAudioPlayer(streamUrl, {
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
| } catch { | ||
| // The player may already have been released. | ||
| } |
There was a problem hiding this comment.
Silent exception swallowing occurs in the catch block on lines 137-139, violating Rule [28] which requires explicit logging or handling. Capture the error and log it using logger.warn('sound.remove() failed during superseded cleanup', { requestId, err: e }) to ensure observability.
Kody rule violation: Avoid empty catch blocks
Prompt for LLM
File src/stores/app/audio-stream-store.ts:
Line 137 to 139:
Silent exception swallowing occurs in the catch block on lines 137-139, violating Rule [28] which requires explicit logging or handling. Capture the error and log it using `logger.warn('sound.remove() failed during superseded cleanup', { requestId, err: e })` to ensure observability.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
2 participants
PR Description
This PR migrates the codebase from the deprecated
expo-avpackage to the newerexpo-audioandexpo-videopackages, resolving compatibility issues with Expo SDK 56.Key Changes
Audio migration (
expo-av→expo-audio):AudioService,NotificationSoundService,AudioStreamStore, andLiveKitStoreto usecreateAudioPlayer()andsetAudioModeAsync()fromexpo-audioreplayAsync()→seekTo(0)+play(),unloadAsync()→remove(),pauseAsync()→pause(), etc.allowsRecordingIOS→allowsRecording,staysActiveInBackground→shouldPlayInBackground)playbackStatusUpdateevent listeners instead of callback-based status updates, and includes improved error handling and stream restart logickeepAudioSessionActive: trueto avoid conflicts with active call audio sessionsVideo migration (
expo-av→expo-video):Videocomponent fromexpo-avwithVideoViewanduseVideoPlayerfrom the newexpo-videopackage inVideoPlayerModalexpo-videoto the Expo plugins configurationTests & mocks:
expo-audioAPIsexpo-videoin the global Jest setup and theexpo-audiomock fileDocumentation:
expo-audio-based approachSummary by CodeRabbit
New Features
Documentation
Tests