Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FCE-205 Refactor into smaller files #7

Merged
merged 52 commits into from
Jul 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
fc5ca0a
Extract `createVideoTransceiverConfig` to separate file
kamil-stasiak Jun 26, 2024
5700fe3
Extract `createAudioTransceiverConfig` to separate file
kamil-stasiak Jun 26, 2024
60870c6
Remove `createVideoTransceiverConfig` private function
kamil-stasiak Jun 26, 2024
934e78c
Move `createTransceiverConfig` to separate file
kamil-stasiak Jun 26, 2024
a30b60e
Move `addTrackToConnection` to separate file
kamil-stasiak Jun 26, 2024
34d7517
Extract `setTransceiverDirection` function
kamil-stasiak Jun 26, 2024
eb38b61
Move `addTransceiversIfNeeded` to a separate file
kamil-stasiak Jun 26, 2024
0121d8f
Extract `setTransceiversToReadOnly` function to a separate file
kamil-stasiak Jun 27, 2024
c323afd
Move `getMidToTrackId` function to a separate file
kamil-stasiak Jun 27, 2024
83faa65
Extract `findSenderByTrack` function to a separate file
kamil-stasiak Jun 27, 2024
1d30724
Extract `isTrackInUse` function to a separate file
kamil-stasiak Jun 27, 2024
0f95ae5
Use already existing `findSenderByTrack` function
kamil-stasiak Jun 27, 2024
7d53c21
Use already existing `findSender` function
kamil-stasiak Jun 27, 2024
b887386
Extract `createSdpOfferEvent` to a separte file
kamil-stasiak Jun 27, 2024
6be7d60
Merge main
kamil-stasiak Jul 5, 2024
4eb6603
Merge main
kamil-stasiak Jul 5, 2024
54fe6ea
Merge branch 'main' of github.com:fishjam-cloud/web-client-sdk into r…
kamil-stasiak Jul 5, 2024
28b8871
Merge
kamil-stasiak Jul 5, 2024
f67e25d
Format
kamil-stasiak Jul 5, 2024
fd09728
WIP
kamil-stasiak Jul 8, 2024
6e0a853
Merge branch 'main' of github.com:fishjam-cloud/web-client-sdk into r…
kamil-stasiak Jul 8, 2024
fd42d83
Extract turn to the separate file
kamil-stasiak Jul 8, 2024
1e4893c
Extract `mapMediaEventTracksToTrackContextImpl` to `./internals.ts`
kamil-stasiak Jul 8, 2024
5905851
Extract State manager to the separate file
kamil-stasiak Jul 8, 2024
ee09df5
Extract `onAnswer`, `disableTrackEncoding`, `onTrack`, `checkIfTrackB…
kamil-stasiak Jul 8, 2024
02e5f47
Extract `onTracksAdded` to `StateManager.ts`
kamil-stasiak Jul 8, 2024
007ae8a
Extract `onTracksRemoved` and `eraseTrack` to `StateManager.ts`
kamil-stasiak Jul 8, 2024
7476c29
Extract `onSdpAnswer` to `StateManager.ts`
kamil-stasiak Jul 8, 2024
2c556b0
Extract `onEndpointAdded`, `onEndpointUpdated` and `addEndpoint` to `…
kamil-stasiak Jul 8, 2024
214f2dd
Extract `onEndpointRemoved` and `eraseEndpoint` to `StateManager.ts`
kamil-stasiak Jul 8, 2024
a201025
Extract `onTrackUpdated` to `StateManager.ts`
kamil-stasiak Jul 8, 2024
0e92000
Extract `onTrackEncodingDisabled` to `StateManager.ts`
kamil-stasiak Jul 8, 2024
c0e6b5b
Extract `onTrackEncodingEnabled` to `StateManager.ts`
kamil-stasiak Jul 8, 2024
bc52948
Add comment to `tracksPriority`
kamil-stasiak Jul 8, 2024
203fc80
Extract `onEncodingSwitched` to `StateManager.ts`
kamil-stasiak Jul 8, 2024
ca5533f
Move `onVadNotification` to `StateManager.ts`
kamil-stasiak Jul 8, 2024
96bc569
Extract `onBandwidthEstimation` to `StateManager.ts`
kamil-stasiak Jul 8, 2024
2e1091a
Extract `NegotiationManager` to `StateManager.ts`
kamil-stasiak Jul 8, 2024
42cd9dd
Extract `CommandsQueue` to the separate file
kamil-stasiak Jul 8, 2024
96db590
Move CommandsQueue.ts listener logic to CommandsQueue
kamil-stasiak Jul 8, 2024
aa41395
Fix format
kamil-stasiak Jul 8, 2024
d2d288e
Move `removeTrackHandler` to StateManager.ts
kamil-stasiak Jul 8, 2024
8c2a66d
Move `replaceTrackHandler` to StateManager.ts
kamil-stasiak Jul 8, 2024
e558f79
Move `addTrackHandler` to StateManager.ts
kamil-stasiak Jul 8, 2024
e1408e0
Fix format and lint
kamil-stasiak Jul 9, 2024
f7df3a3
Revert e2e test file
kamil-stasiak Jul 9, 2024
34e58f0
Remove commendType
kamil-stasiak Jul 9, 2024
266f6d0
Apply PR suggestions
kamil-stasiak Jul 9, 2024
a450586
Apply PR suggestions
kamil-stasiak Jul 9, 2024
65d36ea
Add readonly to CommandQueue field
kamil-stasiak Jul 9, 2024
adbab5a
Apply PR suggestions
kamil-stasiak Jul 10, 2024
6f7d020
Fix lint and format
kamil-stasiak Jul 10, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions e2e-tests/ts-client/app/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
"eslint-plugin-react-refresh": "^0.4.5",
"testcontainers": "^10.3.2",
"ts-proto": "^1.176.0",
"typescript": "^5.5.0",
"vite": "^5.1.2",
"vitest": "^1.6.0"
}
Expand Down
4 changes: 2 additions & 2 deletions e2e-tests/ts-client/app/src/MockComponent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ export const MockComponent = ({ webrtc }: Props) => {
const track = stream.getVideoTracks()[0];

const simulcastConfig: SimulcastConfig = {
enabled: false,
activeEncodings: [],
enabled: true,
activeEncodings: ["h", "m", "l"],
disabledEncodings: [],
};
const maxBandwidth: BandwidthLimit = 0;
Expand Down
2 changes: 1 addition & 1 deletion e2e-tests/ts-client/app/src/VideoPlayerWithDetector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export const VideoPlayerWithDetector = ({ stream, id, webrtc }: Props) => {
}, [stream]);

const getDecodedFrames = useCallback(async () => {
const connection = webrtc["connection"];
const connection = webrtc["stateManager"]["connection"];
if (!connection) return 0;

const inbound = getTrackIdentifierToInboundRtp(await connection.getStats());
Expand Down
4 changes: 2 additions & 2 deletions e2e-tests/ts-client/scenarios/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,14 +137,14 @@ export const assertThatRemoteTracksAreVisible = async (
});
};

type WebrtcClient = { webrtc?: { connection?: RTCPeerConnection } };
type WebrtcClient = { webrtc?: { stateManager: { connection?: RTCPeerConnection } } };
type WindowType = typeof window;

export const assertThatOtherVideoIsPlaying = async (page: Page) => {
await test.step('Assert that media is working', async () => {
const getDecodedFrames: () => Promise<number> = () =>
page.evaluate(async () => {
const connection = (window as WindowType & WebrtcClient)?.webrtc
const connection = (window as WindowType & WebrtcClient)?.webrtc?.stateManager
?.connection;
// connection object is available after first renegotiation (sdpOffer, sdpAnswer)
if (!window || !connection) return -1;
Expand Down
2 changes: 1 addition & 1 deletion packages/react-client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
"typecheck": "tsc"
},
"devDependencies": {
"@playwright/test": "^1.42.1",
"@playwright/test": "^1.45.1",
"@types/events": "^3.0.3",
"@types/lodash.isequal": "^4.5.8",
"@types/node": "^20.14.10",
Expand Down
2 changes: 1 addition & 1 deletion packages/ts-client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
"uuid": "^10.0.0"
},
"devDependencies": {
"@playwright/test": "^1.40.1",
"@playwright/test": "^1.45.1",
"@types/events": "^3.0.3",
"@types/node": "^20.14.10",
"@types/uuid": "^10.0.0",
Expand Down
156 changes: 156 additions & 0 deletions packages/ts-client/src/webrtc/CommandsQueue.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
import type { Command } from './commands';
import type { Deferred } from './deferred';
import type { StateManager } from './StateManager';
import type { NegotiationManager } from './NegotiationManager';

export class CommandsQueue<EndpointMetadata, TrackMetadata> {
private readonly stateManager: StateManager<EndpointMetadata, TrackMetadata>;
private readonly negotiationManager: NegotiationManager;

private clearConnectionCallbacks: (() => void) | null = null;

constructor(
stateManager: StateManager<EndpointMetadata, TrackMetadata>,
negotiationManager: NegotiationManager,
) {
this.stateManager = stateManager;
this.negotiationManager = negotiationManager;
}

public setupEventListeners = (connection: RTCPeerConnection) => {
const onSignalingStateChange = () => {
switch (this.stateManager.connection?.signalingState) {
case 'stable':
this.processNextCommand();
break;
}
};

const onIceGatheringStateChange = () => {
switch (this.stateManager.connection?.iceGatheringState) {
case 'complete':
this.processNextCommand();
break;
}
};

const onConnectionStateChange = () => {
switch (connection.connectionState) {
case 'connected':
this.processNextCommand();
break;
}
};
const onIceConnectionStateChange = () => {
switch (this.stateManager.connection?.iceConnectionState) {
case 'connected':
this.processNextCommand();
break;
}
};

this.clearConnectionCallbacks = () => {
connection.removeEventListener(
'signalingstatechange',
onSignalingStateChange,
);
connection.removeEventListener(
'icegatheringstatechange',
onIceGatheringStateChange,
);
connection.removeEventListener(
'connectionstatechange',
onConnectionStateChange,
);
connection.removeEventListener(
'iceconnectionstatechange',
onIceConnectionStateChange,
);
};

connection.addEventListener(
'icegatheringstatechange',
onIceConnectionStateChange,
);
connection.addEventListener(
'connectionstatechange',
onConnectionStateChange,
);
connection.addEventListener(
'iceconnectionstatechange',
onIceConnectionStateChange,
);
connection.addEventListener('signalingstatechange', onSignalingStateChange);
};

private commandsQueue: Command[] = [];
private commandResolutionNotifier: Deferred<void> | null = null;

public pushCommand = (command: Command) => {
this.commandsQueue.push(command);
this.processNextCommand();
};

private isConnectionUnstable = () => {
const connection = this.stateManager.connection;
if (connection === undefined) return false;

const isSignalingUnstable = connection.signalingState !== 'stable';
const isConnectionNotConnected = connection.connectionState !== 'connected';
const isIceNotConnected = connection.iceConnectionState !== 'connected';

return isSignalingUnstable && isConnectionNotConnected && isIceNotConnected;
};

private isNegotiationInProgress = () => {
return (
this.negotiationManager.ongoingRenegotiation ||
this.stateManager.ongoingTrackReplacement
);
};

public processNextCommand = () => {
if (this.isNegotiationInProgress()) return;
if (this.isConnectionUnstable()) return;

this.resolvePreviousCommand();

const command = this.commandsQueue.shift();

if (!command) return;

this.commandResolutionNotifier = command.resolutionNotifier;
this.handleCommand(command);
};

private handleCommand = (command: Command) => {
const error = command.validate?.();

if (error) {
this.commandResolutionNotifier?.reject(error);
this.commandResolutionNotifier = null;
this.processNextCommand();
} else {
command.handler();

if (command.resolve === 'immediately') {
this.resolvePreviousCommand();
this.processNextCommand();
}
}
};

private resolvePreviousCommand = () => {
if (!this.commandResolutionNotifier) return;

this.commandResolutionNotifier.resolve();
this.commandResolutionNotifier = null;
};

public cleanUp = () => {
this.commandResolutionNotifier?.reject('Disconnected');
this.commandResolutionNotifier = null;
this.commandsQueue = [];
this.clearConnectionCallbacks?.();
};
}
7 changes: 7 additions & 0 deletions packages/ts-client/src/webrtc/NegotiationManager.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export class NegotiationManager {
/**
* Indicates if an ongoing renegotiation is active.
* During renegotiation, both parties are expected to actively exchange events: renegotiateTracks, offerData, sdpOffer, sdpAnswer.
*/
public ongoingRenegotiation: boolean = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WDYT about having two functions renegotationStarted() and renegotationCompleted() with private variable?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. That's why I created this class. I will do it in future PRs

}
11 changes: 11 additions & 0 deletions packages/ts-client/src/webrtc/RTCPeerConnectionUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,14 @@ export const findSender = (
connection!
.getSenders()
.find((sender) => sender.track && sender!.track!.id === trackId)!;

export const findSenderByTrack = (
connection: RTCPeerConnection | undefined,
track: MediaStreamTrack | null | undefined,
): RTCRtpSender | undefined =>
connection?.getSenders().find((sender) => sender.track === track);

export const isTrackInUse = (
connection: RTCPeerConnection | undefined,
track: MediaStreamTrack,
) => connection?.getSenders().some((val) => val.track === track);
Loading