Skip to content

Commit ff47662

Browse files
authored
fix(client): warn about dangling audio bindings only for published audio tracks (#2183)
### 💡 Overview This PR fixes an issue where the audio bindings watchdog reported dangling audio bindings even after the track had been unpublished. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved accuracy of audio binding detection by aligning warnings with actual published audio tracks rather than the mere presence of audio streams. * **Tests** * Added test coverage for the updated audio binding detection behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent eb0931b commit ff47662

File tree

2 files changed

+41
-3
lines changed

2 files changed

+41
-3
lines changed

packages/client/src/helpers/AudioBindingsWatchdog.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { CallingState, CallState } from '../store';
33
import { createSubscription } from '../store/rxUtils';
44
import { videoLoggerSystem } from '../logger';
55
import { Tracer } from '../stats';
6+
import { TrackType } from '../gen/video/sfu/models/models';
67

78
const toBindingKey = (
89
sessionId: string,
@@ -91,12 +92,23 @@ export class AudioBindingsWatchdog {
9192
const danglingUserIds: string[] = [];
9293
for (const p of this.state.participants) {
9394
if (p.isLocalParticipant) continue;
94-
const { audioStream, screenShareAudioStream, sessionId, userId } = p;
95-
if (audioStream && !this.bindings.has(toBindingKey(sessionId))) {
95+
const {
96+
audioStream,
97+
screenShareAudioStream,
98+
sessionId,
99+
userId,
100+
publishedTracks,
101+
} = p;
102+
if (
103+
audioStream &&
104+
publishedTracks.includes(TrackType.AUDIO) &&
105+
!this.bindings.has(toBindingKey(sessionId))
106+
) {
96107
danglingUserIds.push(userId);
97108
}
98109
if (
99110
screenShareAudioStream &&
111+
publishedTracks.includes(TrackType.SCREEN_SHARE_AUDIO) &&
100112
!this.bindings.has(toBindingKey(sessionId, 'screenShareAudioTrack'))
101113
) {
102114
danglingUserIds.push(userId);

packages/client/src/helpers/__tests__/AudioBindingsWatchdog.test.ts

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { StreamClient } from '../../coordinator/connection/client';
1111
import { CallingState, StreamVideoWriteableStateStore } from '../../store';
1212
import { noopComparator } from '../../sorting';
1313
import { fromPartial } from '@total-typescript/shoehorn';
14+
import { TrackType } from '../../gen/video/sfu/models/models';
1415

1516
describe('AudioBindingsWatchdog', () => {
1617
let watchdog: AudioBindingsWatchdog;
@@ -44,12 +45,17 @@ describe('AudioBindingsWatchdog', () => {
4445
screenShareAudioStream?: MediaStream;
4546
},
4647
) => {
48+
const publishedTracks = [];
49+
if (streams?.audioStream) publishedTracks.push(TrackType.AUDIO);
50+
if (streams?.screenShareAudioStream) {
51+
publishedTracks.push(TrackType.SCREEN_SHARE_AUDIO);
52+
}
4753
call.state.updateOrAddParticipant(
4854
sessionId,
4955
fromPartial({
5056
userId,
5157
sessionId,
52-
publishedTracks: [],
58+
publishedTracks,
5359
...streams,
5460
}),
5561
);
@@ -233,6 +239,26 @@ describe('AudioBindingsWatchdog', () => {
233239
expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('user-1'));
234240
});
235241

242+
it('should not warn when audioStream exists but audio is not published', () => {
243+
// @ts-expect-error private property
244+
const warnSpy = vi.spyOn(watchdog.logger, 'warn');
245+
246+
call.state.updateOrAddParticipant(
247+
'session-1',
248+
fromPartial({
249+
userId: 'user-1',
250+
sessionId: 'session-1',
251+
publishedTracks: [],
252+
audioStream: new MediaStream(),
253+
}),
254+
);
255+
256+
call.state.setCallingState(CallingState.JOINED);
257+
vi.advanceTimersByTime(3000);
258+
259+
expect(warnSpy).not.toHaveBeenCalled();
260+
});
261+
236262
it('should not warn when screenShareAudio element is bound', () => {
237263
// @ts-expect-error private property
238264
const warnSpy = vi.spyOn(watchdog.logger, 'warn');

0 commit comments

Comments
 (0)