Skip to content

ix the black camera bubble in Clips desktop - #3369

Open
shomix wants to merge 2 commits into
mainfrom
shomix-p-fix-clips-black-camera
Open

ix the black camera bubble in Clips desktop#3369
shomix wants to merge 2 commits into
mainfrom
shomix-p-fix-clips-black-camera

Conversation

@shomix

@shomix shomix commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

The camera bubble now fixes itself. On some Macs the bubble opened as a black circle with a white play triangle, and only started showing the camera after you clicked it. It came back every time the bubble reopened. The bubble now keeps trying to start the video on its own, and if that keeps failing it switches to the older frame-streaming path, which always works. Either way the user sees their camera without clicking anything.

The bubble no longer claims success when nothing is on screen. It used to report that the camera was live as soon as the connection was made, even when no video was actually playing. That also hid the backup path that would have saved it. It now only reports live video once real frames are on screen, and asks for the backup path when they never arrive.

Better logging for a failed recording. A recording in the same bug report died halfway through and could not be saved. The error told us nothing useful, so we could not find the cause. Recording failures now say which audio or video track broke, at what point in the recording, and whether its timing had drifted. Nothing about recording behaviour changed, only what we can see when it goes wrong.

@netlify

This comment has been minimized.

@github-actions

github-actions Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Here's a visual recap of what changed:

Visual recap

Open the full interactive recap

@builder-io-integration builder-io-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Builder reviewed your changes and found 2 potential issues 🟡

Review Details

Code Review Summary

PR #3369 adds a WebRTC playback watchdog for the Clips camera bubble, delays declaring WebRTC active until decoded frames exist, falls back to the established canvas pump when rendering stalls, and enriches native recording append diagnostics with track/timestamp context. The overall direction is sound: it addresses the reported WebKit autoplay/rendering failure without removing the proven fallback, uses a bounded retry loop, and adds focused predicate tests. The native logging also preserves realtime behavior by continuing to skip backpressured samples.

Risk assessment: Standard (browser media/state behavior plus native recording diagnostics).

Key Findings

  • 🟡 MEDIUM: A late playing event can switch the bubble back to WebRTC after the popover has selected the canvas fallback, hiding the working fallback.
  • 🟡 MEDIUM: A queued ontrack callback from a torn-down peer can overwrite the current receiver’s video and watchdog state during renegotiation.
  • 🟢 LOW: Append bookkeeping is committed before appendSampleBuffer succeeds, so failure diagnostics count the rejected sample as successfully appended.

🧪 Browser testing: Will run after this review (PR touches UI code)

Comment thread templates/clips/desktop/src/overlays/bubble.tsx
@@ -272,17 +399,16 @@ export function Bubble() {
const incomingStream = ev.streams[0];
if (!incomingStream) return;
videoEl.srcObject = incomingStream;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Stale WebRTC ontrack can revive a torn-down peer

The ontrack callback only checks the component-level stopped flag. During renegotiation, teardownPeer() can be followed by a new peer while an already-queued callback from the old peer still runs; it then replaces video.srcObject and updates trackArrivedAtRef with the stale stream. Gate the callback with the current peer/handshake identity before mutating playback state.

Additional Info
Found by 2 of 3 review agents.

Fix in Builder

@builder-io-integration builder-io-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Builder reviewed your changes — no new findings

Review Details

Incremental Code Review Summary

This incremental review examined the latest PR head and the added playback-path guard. The previously reported late-WebRTC-claim issue is fixed: fallbackRequestedRef now prevents a delayed playing event from reclaiming the surface after the canvas pump is requested, and a fresh track explicitly re-arms the watchdog. The stale-peer ontrack race remains present and is intentionally not reposted because its existing review comment is still open.

No genuinely new actionable issues were identified in the latest changes. The append-statistics accuracy concern from the prior review remains in the diff, but it was already identified previously and is not reposted here.

Risk assessment: Standard (desktop media state/fallback behavior and native recording diagnostics). Added predicate coverage is appropriate and targeted validation reported by reviewers passed for the TypeScript/Vitest portions.

🧪 Browser testing: Will run after this review (PR touches UI code)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants