Skip to content

Issue#77 #191

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.

Sign up for GitHub

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

Closed
sarthakNITT wants to merge 7 commits into from
Closed

Issue#77 #191

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c3979d2
Fix camera state handling when users click Next #77
Sarthak114123082 Oct 4, 2025
2f5158a
working smoothly
Sarthak114123082 Oct 5, 2025
8f314c2
removed testing logs
Sarthak114123082 Oct 5, 2025
efecea9
small bug fixed
Sarthak114123082 Oct 5, 2025
ed58d52
updated to latest commit
sarthakNITT Oct 25, 2025
f496fef
fixed: working smoothly
sarthakNITT Oct 25, 2025
1018c49
fixed: addressed required changes
sarthakNITT Nov 5, 2025
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
Binary file modified backend/bun.lockb
View file
Open in desktop
Binary file not shown.
21 changes: 9 additions & 12 deletions backend/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ const app = express();
const server = http.createServer(app);

const io = new Server(server, { cors: { origin: "*" } });
// store latest media state per-socket so late joiners can be informed
const lastMediaState = new Map<string, { micOn?: boolean; camOn?: boolean }>();
Comment on lines +18 to +19

@coderabbitai coderabbitai Bot Oct 25, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

lastMediaState is never written/read — late‑joiner hydration won’t work

You introduce the cache but don’t persist to it or read from it. As-is, late joiners can’t receive existing peers’ media states; this misses the issue #77 requirement.

Hook writes in media events and hydrate on join (see fixes on Lines 78-82 and 118-120).

// io.adapter(createAdapter(pubClient, subClient));

const userManager = new UserManager();
Expand Down Expand Up @@ -73,16 +75,10 @@ io.on("connection", (socket: Socket) => {
userManager.setRoom(socket.id, initialRoomId);
}

// Keep UserManager in sync when client explicitly joins later
socket.on("chat:join", ({ roomId }: ChatJoinPayload) => {
try {
if (!roomId || typeof roomId !== "string") return;
const namespaced = normalizeRoom(roomId.trim());
// Keep UserManager in sync only; actual join + announcements are handled in chat.ts
userManager.setRoom(socket.id, namespaced);
} catch (err) {
console.warn("[chat:join] error", err);
}
// ⬇️ Keep UserManager in sync when client explicitly joins later
socket.on("chat:join", ({ roomId }: { roomId: string; name?: string }) => {
if (roomId) userManager.setRoom(socket.id, roomId);
if (roomId) socket.join(roomId);
});
Comment on lines +78 to 82

@coderabbitai coderabbitai Bot Oct 25, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Normalize room names and hydrate late joiners on chat:join

The handler joins the raw roomId while most emits use the chat: namespace. This mismatch will drop events. Also, use this hook to send existing peers’ media states to the joiner.

Apply this diff:

-  // ⬇️ Keep UserManager in sync when client explicitly joins later
-  socket.on("chat:join", ({ roomId }: { roomId: string; name?: string }) => {
-    if (roomId) userManager.setRoom(socket.id, roomId);
-    if (roomId) socket.join(roomId);
-  });
+  // ⬇️ Keep UserManager in sync when client explicitly joins later
+  socket.on("chat:join", async ({ roomId }: { roomId: string; name?: string }) => {
+    const r = normalizeRoom(roomId);
+    if (!r) return;
+    userManager.setRoom(socket.id, r);
+    socket.join(r);
+    // Hydrate the joiner with current peers' media states
+    const socketsInRoom = await io.in(r).fetchSockets();
+    const peers = socketsInRoom
+      .filter((s) => s.id !== socket.id)
+      .map((s) => ({ id: s.id, state: lastMediaState.get(s.id) ?? {} }));
+    socket.emit("room:peers-media-state", { peers });
+  });

Optional hardening (outside this range): make normalizeRoom idempotent to avoid double prefixing.

// replace existing normalizeRoom
const normalizeRoom = (r: string) => (r ? (r.startsWith("chat:") ? r : `chat:${r}`) : "");
🤖 Prompt for AI Agents 
In backend/src/index.ts around lines 78 to 82, the chat:join handler currently
uses the raw roomId which causes a mismatch with events that use the "chat:"
prefix and doesn't hydrate late joiners; update the handler to call
normalizeRoom(roomId) and use that normalizedRoom for userManager.setRoom and
socket.join, and after joining emit the current peers' media states to the
joining socket (e.g., iterate known peers in the room and send their media state
events to the joiner); also replace or ensure normalizeRoom is idempotent so it
prefixes "chat:" only once (optionally use the provided implementation).


// Screen share + media + renegotiation handlers (same behavior, use namespaced rooms)
Expand Down Expand Up @@ -120,8 +116,7 @@ io.on("connection", (socket: Socket) => {

// Media state
socket.on("media:state", ({ roomId, state }: { roomId: string; state: { micOn?: boolean; camOn?: boolean } }) => {
const r = toRoom(roomId);
if (r) socket.to(r).emit("peer:media-state", { state, from: socket.id });
socket.to(roomId).emit("peer:media-state", { state });
});
Comment on lines 118 to 120

@coderabbitai coderabbitai Bot Oct 25, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

media:state drops from and uses raw roomId; also not updating cache

Without from, clients can’t attribute state changes; using raw roomId bypasses the chat: namespace. Also, not persisting to lastMediaState blocks late‑joiner sync.

Apply this diff:

-  socket.on("media:state", ({ roomId, state }: { roomId: string; state: { micOn?: boolean; camOn?: boolean } }) => {
-    socket.to(roomId).emit("peer:media-state", { state });
-  });
+  socket.on("media:state", ({ roomId, state }: { roomId: string; state: { micOn?: boolean; camOn?: boolean } }) => {
+    const r = normalizeRoom(roomId);
+    // persist latest media state for late joiners
+    lastMediaState.set(socket.id, { ...(lastMediaState.get(socket.id) ?? {}), ...state });
+    if (r) socket.to(r).emit("peer:media-state", { state, from: socket.id });
+  });

Keep the cache in sync for individual toggles too (outside-range snippet):

// media:cam
socket.on("media:cam", ({ roomId, on }: { roomId: string; on: boolean }) => {
  const r = normalizeRoom(roomId);
  lastMediaState.set(socket.id, { ...(lastMediaState.get(socket.id) ?? {}), camOn: on });
  if (r) socket.to(r).emit("media:cam", { on, from: socket.id });
});

// media:mic
socket.on("media:mic", ({ roomId, on }: { roomId: string; on: boolean }) => {
  const r = normalizeRoom(roomId);
  lastMediaState.set(socket.id, { ...(lastMediaState.get(socket.id) ?? {}), micOn: on });
  if (r) socket.to(r).emit("media:mic", { on, from: socket.id });
});

Verify frontend expectations for peer:media-state:

🏁 Script executed:

#!/bin/bash
# Find handlers/usages of 'peer:media-state' and check if 'from' is referenced.
rg -n -C2 --type ts --type tsx "peer:media-state|from" frontend || rg -n -C2 "peer:media-state|from"

Length of output: 30928

Now let me search for the actual functions used in the codebase:

🏁 Script executed:

#!/bin/bash
# Find toRoom function and lastMediaState usage
rg -n -B5 -A10 'toRoom|lastMediaState' backend/src/index.ts | head -100

Length of output: 60

🏁 Script executed:

#!/bin/bash
# Find toRoom function definition and lastMediaState usage in backend
rg -n -B2 -A8 'const toRoom|function toRoom|toRoom\s*=' backend/src/index.ts

echo "---"

# Find lastMediaState declaration and usage
rg -n 'lastMediaState' backend/src/index.ts | head -20

echo "---"

# Show context around media:state handler and nearby handlers
sed -n '115,135p' backend/src/index.ts

Length of output: 1388

Add from, use toRoom(), and update cache consistently across all media handlers

The media:state handler is indeed inconsistent with media:cam and media:mic—it bypasses the chat: namespace normalization and omits the sender ID. However, the cache persistence gap is wider: none of the media handlers update lastMediaState.

Apply the diff as provided, but also update media:cam and media:mic to persist state:

  socket.on("media:cam", ({ roomId, on }: { roomId: string; on: boolean }) => {
    const r = toRoom(roomId);
+   lastMediaState.set(socket.id, { ...(lastMediaState.get(socket.id) ?? {}), camOn: on });
    if (r) socket.to(r).emit("media:cam", { on, from: socket.id });
  });

  socket.on("media:mic", ({ roomId, on }: { roomId: string; on: boolean }) => {
    const r = toRoom(roomId);
+   lastMediaState.set(socket.id, { ...(lastMediaState.get(socket.id) ?? {}), micOn: on });
    if (r) socket.to(r).emit("media:mic", { on, from: socket.id });
  });

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In backend/src/index.ts around lines 118 to 120, the media handlers are
inconsistent: update the "media:state" handler to include the sender id ("from")
and emit using toRoom(chatNamespace, roomId) (i.e., normalize the chat:
namespace) instead of socket.to(roomId).emit, and persist the new state into
lastMediaState cache; likewise, modify the "media:cam" and "media:mic" handlers
to normalize the namespace via toRoom(chatNamespace, roomId), include the sender
id in emitted payloads, and update lastMediaState with the combined per-user
state (micOn/camOn) so the cache is consistently persisted across all three
handlers.


socket.on("media:cam", ({ roomId, on }: { roomId: string; on: boolean }) => {
Expand Down Expand Up @@ -164,6 +159,8 @@ io.on("connection", (socket: Socket) => {

// chat.ts handles leave announcements in its disconnecting handler

// cleanup stored media state
lastMediaState.delete(socket.id);
userManager.removeUser(socket.id);
});

Expand Down
Binary file modified frontend/bun.lockb
View file
Open in desktop
Binary file not shown.
61 changes: 41 additions & 20 deletions frontend/components/RTC/Room.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ export default function Room({
const currentScreenShareTrackRef = useRef<MediaStreamTrack | null>(null);
const localScreenShareStreamRef = useRef<MediaStream | null>(null);
const remoteStreamRef = useRef<MediaStream | null>(null);
const peerCamOnRef = useRef<boolean>(false);

// ICE candidate queues for handling candidates before remote description is set
const senderIceCandidatesQueue = useRef<RTCIceCandidate[]>([]);
Expand Down Expand Up @@ -115,33 +116,35 @@ export default function Room({
pc.addTrack(localAudioTrack);
}

let videoTrack = currentVideoTrackRef.current;
if (!videoTrack || videoTrack.readyState === "ended") {
try {
const stream = await navigator.mediaDevices.getUserMedia({ video: true });
videoTrack = stream.getVideoTracks()[0];
currentVideoTrackRef.current = videoTrack;
} catch (err) {
console.error("Error creating video track:", err);
videoTrack = null;
if (camOn) {
let videoTrack = currentVideoTrackRef.current;
if (!videoTrack || videoTrack.readyState === "ended") {
try {
const stream = await navigator.mediaDevices.getUserMedia({ video: true });
videoTrack = stream.getVideoTracks()[0];
currentVideoTrackRef.current = videoTrack;
} catch (err) {
console.error("Error creating video track:", err);
videoTrack = null;
}
}

if (videoTrack && videoTrack.readyState === "live") {
const vs = pc.addTrack(videoTrack);
videoSenderRef.current = vs;
}
}

if (videoTrack && videoTrack.readyState === "live") {
const vs = pc.addTrack(videoTrack);
videoSenderRef.current = vs;
}

ensureRemoteStreamLocal();
pc.ontrack = (e) => {
if (!remoteStreamRef.current) remoteStreamRef.current = new MediaStream();
if (e.track.kind === 'video') {
remoteStreamRef.current.getVideoTracks().forEach(track =>
remoteStreamRef.current?.removeTrack(track)
);
remoteStreamRef.current.getVideoTracks().forEach(t => { try { remoteStreamRef.current?.removeTrack(t); } catch {} });
remoteStreamRef.current.addTrack(e.track);
} else {
remoteStreamRef.current.addTrack(e.track);
}
remoteStreamRef.current.addTrack(e.track);
ensureRemoteStreamLocal();
ensureRemoteStreamLocal();
};

pc.onicecandidate = (e) => {
Expand Down Expand Up @@ -387,6 +390,8 @@ export default function Room({

teardownPeers(
"teardown",
camOn,
micOn,
sendingPcRef,
receivingPcRef,
remoteStreamRef,
Expand Down Expand Up @@ -456,6 +461,8 @@ export default function Room({

teardownPeers(
reason,
camOn,
micOn,
sendingPcRef,
Comment on lines +464 to 466

@coderabbitai coderabbitai Bot Oct 25, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Pass actual media liveness to teardownPeers during Next.

You compute currentCamState/currentMicState but pass camOn/micOn, which can be stale (e.g., track ended).

-      camOn,
-      micOn,
+      currentCamState,
+      currentMicState,
📝 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.

Suggested change
camOn,
micOn,
sendingPcRef,
currentCamState,
currentMicState,
sendingPcRef,
🤖 Prompt for AI Agents 
In frontend/components/RTC/Room.tsx around lines 472 to 474, the teardownPeers
call is passing camOn and micOn which may be stale; instead pass the computed
currentCamState and currentMicState that reflect actual media liveness (e.g.,
derived from track.enabled/readyState or your currentCamState/currentMicState
variables). Update the teardownPeers invocation to use currentCamState and
currentMicState (ensure those variables are in scope at this call site) so peer
teardown uses the real-time media state.

receivingPcRef,
remoteStreamRef,
Expand Down Expand Up @@ -541,6 +548,10 @@ export default function Room({
socketRef.current.emit("media:state", { roomId, state: { micOn, camOn } });
}, [micOn, camOn, roomId]);

useEffect(() => {
peerCamOnRef.current = peerCamOn;
}, [peerCamOn]);

// Main socket connection effect - simplified, actual WebRTC logic would be here
useEffect(() => {
if (socketRef.current) return;
Expand All @@ -566,6 +577,7 @@ export default function Room({
// ----- CALLER -----
s.on("send-offer", async ({ roomId: rid }) => {
setRoomId(rid);
s.emit("chat:join", { roomId: rid, name });
setLobby(false);
setStatus("Connecting…");

Expand All @@ -580,7 +592,11 @@ export default function Room({
s.emit("chat:join", { roomId: rid, name });
}, 100);
}, 100);

try { remoteStreamRef.current?.getTracks().forEach(t => t.stop()); } catch {}
remoteStreamRef.current = new MediaStream();
if (remoteVideoRef.current) { remoteVideoRef.current.srcObject = null; try { remoteVideoRef.current.load(); } catch {} }
if (remoteAudioRef.current) remoteAudioRef.current.srcObject = null;

const pc = new RTCPeerConnection();
sendingPcRef.current = pc;
peerIdRef.current = rid;
Expand All @@ -595,6 +611,7 @@ export default function Room({
// ----- ANSWERER -----
s.on("offer", async ({ roomId: rid, sdp: remoteSdp }) => {
setRoomId(rid);
s.emit("chat:join", { roomId: rid, name });
setLobby(false);
setStatus("Connecting…");

Expand All @@ -609,6 +626,10 @@ export default function Room({
s.emit("chat:join", { roomId: rid, name });
}, 100);
}, 100);
try { remoteStreamRef.current?.getTracks().forEach(t => t.stop()); } catch {}
remoteStreamRef.current = new MediaStream();
if (remoteVideoRef.current) { remoteVideoRef.current.srcObject = null; try { remoteVideoRef.current.load(); } catch {} }
if (remoteAudioRef.current) remoteAudioRef.current.srcObject = null;

const pc = new RTCPeerConnection();
receivingPcRef.current = pc;
Expand Down
37 changes: 25 additions & 12 deletions frontend/components/RTC/webrtc-utils.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,8 @@ export function stopProvidedTracks(

export function teardownPeers(
reason: string,
camOn: boolean,
micOn: boolean,
sendingPcRef: React.RefObject<RTCPeerConnection | null>,
receivingPcRef: React.RefObject<RTCPeerConnection | null>,
remoteStreamRef: React.RefObject<MediaStream | null>,
Expand Down Expand Up @@ -206,8 +208,8 @@ export function teardownPeers(

// Reset UI states
setters.setShowChat(false);
setters.setPeerMicOn(true);
setters.setPeerCamOn(true);
setters.setPeerMicOn(micOn);
setters.setPeerCamOn(camOn);
setters.setScreenShareOn(false);
Comment on lines +211 to 213

@coderabbitai coderabbitai Bot Oct 25, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Peer state reset uses local flags; likely incorrect UI semantics.

Setting setPeerMicOn(micOn) / setPeerCamOn(camOn) mirrors self into “peer” state and can mislead UI during lobby/transition.

Apply neutral defaults until the peer’s state arrives:

-  setters.setPeerMicOn(micOn);
-  setters.setPeerCamOn(camOn);
+  // Reset to a neutral/unknown state; will be updated by peer events
+  setters.setPeerMicOn(false);
+  setters.setPeerCamOn(false);
📝 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.

Suggested change
setters.setPeerMicOn(micOn);
setters.setPeerCamOn(camOn);
setters.setScreenShareOn(false);
// Reset to a neutral/unknown state; will be updated by peer events
setters.setPeerMicOn(false);
setters.setPeerCamOn(false);
setters.setScreenShareOn(false);
🤖 Prompt for AI Agents 
In frontend/components/RTC/webrtc-utils.tsx around lines 211 to 213, the code
sets peer state using local micOn/camOn flags which mirrors the local user into
the peer UI; change these to neutral defaults so the peer UI doesn’t falsely
reflect local state while waiting for the peer’s actual state — e.g.,
setPeerMicOn(false) and setPeerCamOn(false) (or null/undefined if your state
model supports an "unknown" value) and keep setScreenShareOn(false), so the peer
controls remain neutral until the peer’s presence/state update arrives.

setters.setPeerScreenShareOn(false);

Expand Down Expand Up @@ -270,23 +272,34 @@ export async function toggleCameraTrack(
}

if (videoSenderRef.current) {
await videoSenderRef.current.replaceTrack(track);
try {
await videoSenderRef.current.replaceTrack(track);
} catch (err) {
try {
pc?.getSenders().forEach(s => { if (s.track?.kind === 'video') try { pc.removeTrack(s); } catch {} });
} catch {}
const sender = pc?.addTrack(track) || null;
if (sender) videoSenderRef.current = sender;
}
} else if (pc) {
const sender = pc.addTrack(track);
videoSenderRef.current = sender;
// console.log("Added new video track to existing connection");

if (sendingPcRef.current === pc) {
}

try { socketRef.current?.emit("media:cam", { roomId, on: true }); } catch {}
try {
if (pc) {
const offer = await pc.createOffer();
await pc.setLocalDescription(offer);
socketRef.current?.emit("renegotiate-offer", {
roomId,
sdp: offer,
role: "caller"
socketRef.current?.emit("renegotiate-offer", {
roomId,
sdp: offer,
role: sendingPcRef.current === pc ? "caller" : "answerer"
});
// console.log("📤 Sent renegotiation offer for camera turn on");
}
}
} catch (err) {
console.warn("renegotiation failed:", err);
}
} else {
if (videoSenderRef.current) {
await videoSenderRef.current.replaceTrack(null);
Expand Down