fix: align webapp with updated livekit tracks lifetime#316
Conversation
✅ Deploy Preview for hoppdocs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughFixes YUV 4:2:0 chroma sizing via ceiling division for odd widths; adds Radix-based Select UI and dependency; refactors Room.tsx to use ChangesVideo and Media Handling Improvements
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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: 1
🤖 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 `@web-app/src/pages/Room.tsx`:
- Around line 561-573: RemoteAudio currently grabs the first audio publication
but forces Track.Source.Microphone in the AudioTrack ref, causing a mismatch
when the first publication isn't a microphone (e.g., screen-share audio). Update
RemoteAudio to find the audio publication whose source is actually
Track.Source.Microphone (e.g., iterate
participant.audioTrackPublications.values() and select pub where
pub.track?.source === Track.Source.Microphone or pub.source ===
Track.Source.Microphone), use the same source when calling useIsMuted, and pass
that matching publication and its source into AudioTrack.trackRef so the
component can resolve the correct remote microphone track.
🪄 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
Run ID: 4e6c7a2a-5aec-4d73-901c-54adb2b8b3b9
📒 Files selected for processing (3)
core/src/graphics/yuv_renderer.rscore/src/livekit/video.rsweb-app/src/pages/Room.tsx
| function RemoteAudio({ participant }: { participant: Participant }) { | ||
| const isMicMuted = useIsMuted({ participant, source: Track.Source.Microphone }); | ||
| const audioPub = Array.from(participant.audioTrackPublications.values())[0]; | ||
|
|
||
| if (!audioPub || isMicMuted) return null; | ||
|
|
||
| return ( | ||
| <AudioTrack | ||
| trackRef={{ | ||
| participant, | ||
| source: Track.Source.Microphone, | ||
| publication: audioPub, | ||
| }} |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For livekit-client v2.16.1, can Participant.audioTrackPublicationsinclude non-microphone sources (e.g., screen-share audio)? For@livekit/components-reactv2.9.17, inAudioTrack, what happens if trackRef.sourceisMicrophonebuttrackRef.publication.source differs?
💡 Result:
For LiveKit Client SDK v2.16.1, the Participant.audioTrackPublications property is a collection of all audio track publications for a participant [1]. While it is commonly used for microphone tracks, it is not strictly limited to them; it includes any published audio track, including those from other sources like screen-share audio, provided they are published as audio tracks [1][2]. Regarding @livekit/components-react v2.9.17, the AudioTrack component uses the useMediaTrackBySourceOrName hook to resolve the track to be rendered [3][4]. When you provide a trackRef, the component relies on this hook to find the appropriate publication [3][4]. If the trackRef.source is set to Microphone but the actual publication source differs (e.g., it is a screen-share audio track), the hook will typically fail to find a matching publication for that specific source, resulting in the component not rendering the track or rendering nothing, as it specifically filters for the source provided in the trackRef [3][5][4]. The component does not automatically fallback to other sources if the specified source does not match the publication's source [3][4].
Citations:
- 1: https://docs.livekit.io/reference/client-sdk-js/classes/Participant.html
- 2: https://docs.livekit.io/intro/basics/rooms-participants-tracks/tracks/
- 3: https://cdn.jsdelivr.net/npm/@livekit/components-react@2.9.19/src/components/participant/AudioTrack.tsx
- 4: https://github.com/livekit/components-js/blob/f40ba89c/packages/react/src/components/participant/AudioTrack.tsx
- 5: https://github.com/livekit/components-js/blob/main/packages/react/src/hooks/useTracks.ts
Select microphone publication explicitly in RemoteAudio.
RemoteAudio picks the first audio publication, but then forces source to microphone. Participant.audioTrackPublications can include non-microphone audio sources (e.g., screen-share audio). When trackRef.source (Microphone) does not match the actual publication source, the AudioTrack component fails to resolve the track, resulting in missing remote audio.
Proposed fix
function RemoteAudio({ participant }: { participant: Participant }) {
const isMicMuted = useIsMuted({ participant, source: Track.Source.Microphone });
- const audioPub = Array.from(participant.audioTrackPublications.values())[0];
+ const audioPub = Array.from(participant.audioTrackPublications.values()).find(
+ (pub) => pub.source === Track.Source.Microphone,
+ );
if (!audioPub || isMicMuted) return null;
return (
<AudioTrack
trackRef={{
participant,
source: Track.Source.Microphone,
publication: audioPub,
}}
volume={1.0}
/>
);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function RemoteAudio({ participant }: { participant: Participant }) { | |
| const isMicMuted = useIsMuted({ participant, source: Track.Source.Microphone }); | |
| const audioPub = Array.from(participant.audioTrackPublications.values())[0]; | |
| if (!audioPub || isMicMuted) return null; | |
| return ( | |
| <AudioTrack | |
| trackRef={{ | |
| participant, | |
| source: Track.Source.Microphone, | |
| publication: audioPub, | |
| }} | |
| function RemoteAudio({ participant }: { participant: Participant }) { | |
| const isMicMuted = useIsMuted({ participant, source: Track.Source.Microphone }); | |
| const audioPub = Array.from(participant.audioTrackPublications.values()).find( | |
| (pub) => pub.source === Track.Source.Microphone, | |
| ); | |
| if (!audioPub || isMicMuted) return null; | |
| return ( | |
| <AudioTrack | |
| trackRef={{ | |
| participant, | |
| source: Track.Source.Microphone, | |
| publication: audioPub, | |
| }} | |
| volume={1.0} | |
| /> | |
| ); | |
| } |
🤖 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 `@web-app/src/pages/Room.tsx` around lines 561 - 573, RemoteAudio currently
grabs the first audio publication but forces Track.Source.Microphone in the
AudioTrack ref, causing a mismatch when the first publication isn't a microphone
(e.g., screen-share audio). Update RemoteAudio to find the audio publication
whose source is actually Track.Source.Microphone (e.g., iterate
participant.audioTrackPublications.values() and select pub where
pub.track?.source === Track.Source.Microphone or pub.source ===
Track.Source.Microphone), use the same source when calling useIsMuted, and pass
that matching publication and its source into AudioTrack.trackRef so the
component can resolve the correct remote microphone track.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web-app/src/components/ui/select.tsx (1)
27-27: ⚡ Quick winPrefer
cnutility for consistency.The rest of the file uses the
cnutility for className composition, but this line usesclsxdirectly. Usingcnwould maintain consistency across the component.♻️ Proposed fix for consistency
- <ChevronDownIcon className={clsx("size-4 opacity-70", iconClassName)} /> + <ChevronDownIcon className={cn("size-4 opacity-70", iconClassName)} />🤖 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 `@web-app/src/components/ui/select.tsx` at line 27, Replace the direct use of clsx on the ChevronDownIcon with the project's cn utility for class composition to keep consistency; update the JSX inside the Select component where ChevronDownIcon is rendered (the className currently built with clsx("size-4 opacity-70", iconClassName)) to use cn("size-4 opacity-70", iconClassName) and ensure the cn import is present at top of the file if missing.
🤖 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.
Nitpick comments:
In `@web-app/src/components/ui/select.tsx`:
- Line 27: Replace the direct use of clsx on the ChevronDownIcon with the
project's cn utility for class composition to keep consistency; update the JSX
inside the Select component where ChevronDownIcon is rendered (the className
currently built with clsx("size-4 opacity-70", iconClassName)) to use cn("size-4
opacity-70", iconClassName) and ensure the cn import is present at top of the
file if missing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 268d657f-83d0-4465-a809-8df44f42a2b8
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (3)
web-app/package.jsonweb-app/src/components/ui/select.tsxweb-app/src/pages/Room.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- web-app/src/pages/Room.tsx
2 participants
Summary by CodeRabbit
Bug Fixes
Improvements
New Features