Filters#73
Conversation
|
@thinkter is attempting to deploy a commit to the Outreach - ACM Team on Vercel. A member of the Team first needs to authorize it. |
Greptile SummaryThis PR adds a camera-effects pipeline (
Confidence Score: 3/5The core camera-effects pipeline has two defects that silently degrade the user experience and leak GPU resources under reachable conditions. In
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User selects filter] --> B[createManagedCameraTrack]
B --> C{effect type?}
C -->|none| D[Raw camera stream]
C -->|blur| E[createBlurredTrack]
C -->|party-hat / cat-ears| F[createFaceOverlayTrack]
C -->|3d-glasses| G[createThreeFaceOverlayTrack]
E --> H[getImageSegmenterWithTimeout\nMediaPipe segmenter]
F --> I[getFaceLandmarkerWithTimeout\nMediaPipe face landmarker]
G --> J[loadThreeModule\nThree.js + GLTFLoader]
G --> K[loadThreeModel\nglasses.glb]
G --> I
H --> L[waitForVideoReady\n2.5s timeout]
I --> L
J --> L
K --> L
L -->|ready| M[RAF render loop\ncanvas captureStream]
L -->|timeout / error| N[throw -> caller catches\nfallback: raw camera]
N --> O[glasses-3d: WebGLRenderer\nnot disposed on timeout]
M --> P[ManagedCameraTrack\nstream + track + stop]
P --> Q[useMeetMedia\nupdateVideoQuality -> WebRTC producer]
Reviews (4): Last reviewed commit: "fix: camerafilterdrawer fix" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| export type BackgroundEffect = CameraEffect; | ||
|
|
There was a problem hiding this comment.
BackgroundEffect alias includes face effects, misleading consumers
BackgroundEffect is typed as an exact alias for CameraEffect, which means it includes "party-hat", "cat-ears", and "3d-glasses" — none of which are background effects. Everywhere BackgroundEffect is used (props, state, hooks), readers expecting a background-only type will be surprised that face filters are valid. If the intent is to keep a single enum for all camera effects, renaming the alias to something like CameraEffectOption throughout, or narrowing BackgroundEffect to only "none" | "blur", would reduce confusion.
| const loadVisionModule = async (): Promise<VisionModule> => { | ||
| if (!visionModulePromise) { | ||
| visionModulePromise = import("@mediapipe/tasks-vision"); | ||
| } | ||
|
|
||
| return visionModulePromise; | ||
| }; |
There was a problem hiding this comment.
visionModulePromise is never cleared on rejection. If the dynamic import("@mediapipe/tasks-vision") fails (chunk load error, CDN hiccup), the rejected promise is cached permanently. Both segmenterPromise and faceLandmarkerPromise correctly reset themselves to null in their .catch handlers, but when they retry they call loadVisionModule() again, which returns the same permanently-rejected visionModulePromise. The result is that blur and every face filter silently falls back to raw camera for the rest of the session — a page refresh is required to recover. This is the identical pattern that was already fixed for threeModulePromise in glasses-3d.ts.
| const loadVisionModule = async (): Promise<VisionModule> => { | |
| if (!visionModulePromise) { | |
| visionModulePromise = import("@mediapipe/tasks-vision"); | |
| } | |
| return visionModulePromise; | |
| }; | |
| const loadVisionModule = async (): Promise<VisionModule> => { | |
| if (!visionModulePromise) { | |
| visionModulePromise = import("@mediapipe/tasks-vision").catch((error) => { | |
| visionModulePromise = null; | |
| throw error; | |
| }); | |
| } | |
| return visionModulePromise; | |
| }; |
| try { | ||
| await video.play(); | ||
| } catch {} | ||
| await waitForVideoReady(video); | ||
|
|
||
| const capturedStream = outputCanvas.captureStream(frameRate); |
There was a problem hiding this comment.
THREE.WebGLRenderer is created before waitForVideoReady is called, but there is no cleanup path if waitForVideoReady rejects (it now times out after 2.5 s via VIDEO_READY_TIMEOUT_MS). When the timeout fires, the error propagates uncaught out of createThreeFaceOverlayTrack, and renderer.dispose() is never called — leaking a WebGL context and the associated GPU memory. Browsers cap the number of concurrent WebGL contexts (typically 16–32), so repeated filter switches on sluggish devices or after transient camera failures will eventually exhaust available contexts.
| try { | |
| await video.play(); | |
| } catch {} | |
| await waitForVideoReady(video); | |
| const capturedStream = outputCanvas.captureStream(frameRate); | |
| try { | |
| await video.play(); | |
| } catch {} | |
| try { | |
| await waitForVideoReady(video); | |
| } catch (err) { | |
| renderer.dispose(); | |
| stopMediaStream(sourceStream); | |
| throw err; | |
| } | |
| const capturedStream = outputCanvas.captureStream(frameRate); |
1 participant
read greptile
see vid sent on grp