Skip to content

Commit

Permalink
Merge pull request #78 from whereby/havard/pan-302-incoming-video-is-…
Browse files Browse the repository at this point in the history
…broken-in-p2p

Fix incorrect handling of stream added
  • Loading branch information
havardholvik authored Oct 9, 2023
2 parents da8ff9e + 4a269cf commit 8ff9a5a
Show file tree
Hide file tree
Showing 5 changed files with 112 additions and 34 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@whereby.com/browser-sdk",
"version": "2.0.0-alpha21",
"version": "2.0.0-alpha22",
"description": "Modules for integration Whereby video in web apps",
"author": "Whereby AS",
"license": "MIT",
Expand Down
60 changes: 32 additions & 28 deletions src/lib/RoomConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,34 @@ function createSocket() {
return new ServerSocket(SOCKET_HOST, socketConf);
}

export function handleStreamAdded(
remoteParticipants: RemoteParticipant[],
{ clientId, stream, streamId, streamType }: RtcStreamAddedPayload
) {
if (!streamId) {
streamId = stream.id;
}
const remoteParticipant = remoteParticipants.find((p) => p.id === clientId);
if (!remoteParticipant) {
return;
}

const remoteParticipantStream =
remoteParticipant.streams.find((s) => s.id === streamId) || remoteParticipant.streams[0];

if (
(remoteParticipant.stream && remoteParticipant.stream.id === streamId) ||
(!remoteParticipant.stream && streamType === "webcam") ||
(!remoteParticipant.stream && !streamType && remoteParticipant.streams.indexOf(remoteParticipantStream) < 1)
) {
return new CustomEvent("participant_stream_added", { detail: { participantId: clientId, stream, streamId } });
}
// screenshare
return new CustomEvent("screenshare_started", {
detail: { participantId: clientId, stream, id: streamId, isLocal: false },
});
}

/*
* This is the topmost interface when dealing with Whereby.
*
Expand Down Expand Up @@ -717,35 +745,11 @@ export default class RoomConnection extends TypedEventTarget {
});
}

private _handleStreamAdded({ clientId, stream, streamId, streamType }: RtcStreamAddedPayload) {
const remoteParticipant = this.remoteParticipants.find((p) => p.id === clientId);
if (!remoteParticipant) {
this.logger.log("WARN: Could not find participant for incoming stream");
return;
}

const remoteParticipantStream = remoteParticipant.streams.find((s) => s.id === streamId);

if (
(remoteParticipant.stream && remoteParticipant.stream.id === streamId) ||
(!remoteParticipant.stream && streamType === "webcam") ||
(!remoteParticipant.stream &&
!streamType &&
remoteParticipantStream &&
remoteParticipant.streams.indexOf(remoteParticipantStream) < 1)
) {
this.dispatchEvent(
new CustomEvent("participant_stream_added", { detail: { participantId: clientId, stream, streamId } })
);
return;
private _handleStreamAdded(args: RtcStreamAddedPayload) {
const streamAddedEvent = handleStreamAdded(this.remoteParticipants, args);
if (streamAddedEvent) {
this.dispatchEvent(streamAddedEvent);
}

// screenshare
this.dispatchEvent(
new CustomEvent("screenshare_started", {
detail: { participantId: clientId, stream, id: streamId, isLocal: false },
})
);
}

private _joinRoom() {
Expand Down
64 changes: 63 additions & 1 deletion src/lib/__tests__/RoomConnection.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { jest } from "@jest/globals";

import RoomConnection from "../RoomConnection";
import RoomConnection, { handleStreamAdded } from "../RoomConnection";
import { RemoteParticipant } from "../RoomParticipant";
import MockMediaStream from "../__mocks__/MediaStream";

jest.mock("@whereby/jslib-media/src/utils/ServerSocket", () => {
return jest.fn().mockImplementation(() => {
Expand Down Expand Up @@ -40,3 +42,63 @@ describe("RoomConnection", () => {
});
});
});

describe("handleStreamAdded", () => {
let remoteParticipants: RemoteParticipant[];

beforeEach(() => {
remoteParticipants = [
new RemoteParticipant({
displayName: "Participant",
id: "id",
newJoiner: false,
streams: ["0", "screenshare"],
isAudioEnabled: true,
isVideoEnabled: true,
}),
];
});

it("should return undefined if remote participant cannot be found", () => {
const res = handleStreamAdded(remoteParticipants, {
clientId: "zzz",
stream: new MockMediaStream(),
streamId: undefined,
streamType: undefined,
});

expect(res).toEqual(undefined);
});

it("should return `participant_stream_added` when stream id cannot be matched", () => {
const clientId = "id";
const stream = new MockMediaStream();
const streamId = undefined;

const res = handleStreamAdded(remoteParticipants, {
clientId,
stream,
streamId,
streamType: undefined,
});

expect(res?.type).toEqual("participant_stream_added");
expect(res?.detail).toEqual({ participantId: clientId, stream, streamId: stream.id });
});

it("should return `screenshare_started` when stream id is matched", () => {
const clientId = "id";
const stream = new MockMediaStream();
const streamId = "screenshare";

const res = handleStreamAdded(remoteParticipants, {
clientId,
stream,
streamId,
streamType: undefined,
});

expect(res?.type).toEqual("screenshare_started");
expect(res?.detail).toEqual({ participantId: clientId, stream, id: streamId, isLocal: false });
});
});
16 changes: 14 additions & 2 deletions src/stories/components/VideoExperience.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,14 @@ export default function VideoExperience({
logger: console,
});

const { localParticipant, mostRecentChatMessage, remoteParticipants, roomConnectionStatus, waitingParticipants } =
state;
const {
localParticipant,
mostRecentChatMessage,
remoteParticipants,
roomConnectionStatus,
waitingParticipants,
screenshares,
} = state;
const {
knock,
sendChatMessage,
Expand Down Expand Up @@ -108,6 +114,12 @@ export default function VideoExperience({
) : null}
</div>
))}
{screenshares.map(
(s) =>
s.stream && (
<VideoView style={{ width: 200, height: "auto" }} key={s.id} stream={s.stream} />
)
)}
</div>
<div className="controls">
<button onClick={() => toggleCamera()}>Toggle camera</button>
Expand Down
4 changes: 2 additions & 2 deletions src/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ declare module "@whereby/jslib-media/src/webrtc/RtcManagerDispatcher" {
interface RtcStreamAddedPayload {
clientId: string;
stream: MediaStream;
streamId: string;
streamType: "webcam" | "screenshare";
streamId: string | undefined;
streamType: "webcam" | "screenshare" | undefined;
}

type RtcEvents = {
Expand Down

0 comments on commit 8ff9a5a

Please sign in to comment.