diff --git a/frontend/src/components/VideoPlayer.jsx b/frontend/src/components/VideoPlayer.jsx index 2afa624..d708921 100644 --- a/frontend/src/components/VideoPlayer.jsx +++ b/frontend/src/components/VideoPlayer.jsx @@ -3,6 +3,7 @@ import { forwardRef, useEffect, useImperativeHandle, useMemo, useRef, useState } const isDev = import.meta.env.DEV; const DIRECT_LOAD_TIMEOUT_MS = 5000; +const EMPTY_COMPANION_AUDIO_URLS = []; function logSourceInputs({ mp4Url, hlsUrl, dashUrl, hasAudio, sourceKind }) { if (!isDev) return; @@ -25,7 +26,7 @@ const VideoPlayer = forwardRef(function VideoPlayer( mp4Url, hlsUrl, dashUrl, - companionAudioUrls = [], + companionAudioUrls = EMPTY_COMPANION_AUDIO_URLS, hasAudio = null, sourceKind = null, posterUrl = '', diff --git a/issues/005-redgifs-playback-loop.md b/issues/005-redgifs-playback-loop.md index daf03cb..1585e3b 100644 --- a/issues/005-redgifs-playback-loop.md +++ b/issues/005-redgifs-playback-loop.md @@ -4,6 +4,50 @@ None. Standalone bug, parked from session work on 2026-05-08. +## 2026-05-26 evidence (root cause identified) + +Re-investigated under branch `fix/005-redgifs-playback` with temporary instrumentation on the 17-dep `useEffect` in `frontend/src/components/VideoPlayer.jsx`. The instrumentation logs an effect run counter and which deps changed between consecutive fires. + +**Observed:** + +- The loop fires at roughly 1500 effect runs per second (~7900 fires in a 5-second capture). +- `loadedmetadata` DOES fire (duration appears in the native controls bar), contra the original "never reaches `loadedmetadata`" assumption above. The loop is downstream of metadata, not upstream. +- The **only** dep that changes between fires is `sanitizedCompanionAudioUrls`. All 16 other deps (including `preferredMuted`, `onDiagnostics`, `mp4Url`, `sourceKind`, etc.) are stable. + +**Cross-reference against the original four hypotheses:** + +- Hypothesis #1 (`preferredMuted` feedback loop via `volumechange` → `onMutedChange` → parent `setPersistMuted`): **eliminated**. `preferredMuted` never appears in the changed-dep set. +- Hypothesis #2 (inline-computed prop with new ref per render): **partially right direction, wrong location**. The fresh literal is not in the parent's JSX; it's the default value in VideoPlayer's own destructure. +- Hypothesis #3 (`onDiagnostics` instability): **eliminated**. LightboxModal does not pass `onDiagnostics`, and the dep is stable in the trace. +- Hypothesis #4 (`current` reference instability): **eliminated**. Other deps tied to `current` (`mp4Url`, `sourceKind`, `posterUrl`) are stable in the trace. + +**Actual root cause:** + +`frontend/src/components/VideoPlayer.jsx:28` destructures the prop with a default value: + +```js +companionAudioUrls = [], +``` + +JavaScript evaluates default-expression `[]` fresh on every function invocation, so when a caller omits `companionAudioUrls`, the destructure produces a brand-new array reference on every render of VideoPlayer. + +`frontend/src/components/LightboxModal.jsx:596` — the RedGIFs embed `` call site — does **not** pass `companionAudioUrls`. The regular video call site at LightboxModal.jsx:576-591 does pass it (as a memoized prop on lines 157-160), which is why the v.redd.it HLS path does not loop. + +The unstable `companionAudioUrls` reference flows into `useMemo` (VideoPlayer.jsx:55-58): + +```js +const sanitizedCompanionAudioUrls = useMemo( + () => (Array.isArray(companionAudioUrls) ? companionAudioUrls.filter(Boolean) : []), + [companionAudioUrls] +); +``` + +React's `Object.is` check on `[companionAudioUrls]` fails every render, the memo factory re-runs, and `.filter(Boolean)` produces yet another fresh array. `sanitizedCompanionAudioUrls` is therefore a new reference every render, which invalidates the 17-dep `useEffect`. Cleanup runs `video.removeAttribute('src')` + `video.load()`, events fire, `setVideoMetrics` / diagnostics setState trigger a re-render, and the loop closes. + +The same path is also active for the prebuffer `` at LightboxModal.jsx:647, which also omits `companionAudioUrls`. The fix below resolves both surfaces at once. + +This is a **fifth hypothesis** — destructure-default-array-literal — that was not on the original list. + ## What's broken When a RedGIFs-embed post is opened in the lightbox, the `