Repository navigation
[buzz-app] Fit uploaded videos to the relay's 4K envelope and name video rejections - #1129
Conversation
…deo rejections Full-screen QuickTime recordings on Retina displays are captured at native pixel size (e.g. 3406x2236). Video preparation converted them to MP4 but kept that size, so the relay rejected them for exceeding the 2160 short-edge limit, and the app showed only "format or metadata may not be supported". Preparation now scales videos down (never up) to fit 3840x2160 in either orientation before the existing odd-edge pad. Relay rejections for resolution, duration, codec and container now get their own messages instead of the generic one. Co-authored-by: murderbot <3754f8729004d95654c46dbab3129e4ab9ef05cc2534e2a3fbfc155983bd637b@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: Bradley Axen <[email protected]>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
kalvinnchau
left a comment
kalvinnchau
left a comment
There was a problem hiding this comment.
🤖 One non-blocking cosmetic finding; no merge blockers identified.
| // Fit the relay's 3840x2160 envelope in either orientation (Retina screen | ||
| // recordings exceed it), never upscale, then pad odd edges for yuv420p. | ||
| const VIDEO_FILTER = | ||
| "scale=w='trunc(iw*min(1,min(3840/max(iw,ih),2160/min(iw,ih))))':h='trunc(ih*min(1,min(3840/max(iw,ih),2160/min(iw,ih))))',pad=ceil(iw/2)*2:ceil(ih/2)*2"; |
There was a problem hiding this comment.
🤖 [P3] Round scaled dimensions to even values before padding
A 3456×2234 screen recording scales to an odd width of 3341, then the existing pad filter adds a black column to reach 3342×2160 (reproduced with FFmpeg 9.0.1). The portrait equivalent similarly adds a black row. This remains inside the relay envelope, but introduces a cosmetic border for an originally even-sized recording. Round the scaled edges to even values inside the scale expression, preserving padding for unscaled odd inputs. Apply the same adjustment to the mirrored filter in src-tauri/src/relay/media_preparation.rs:18.
There was a problem hiding this comment.
🤖 Fixed in 8338162. Scaled edges now round to even inside the scale expression, so padding only touches odd-sized unscaled input. 3456×2234 → 3342×2160 and 2234×3456 → 2160×3342, with no added column. Both real-ffmpeg tests now use 3456×2234.
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Carl, an automated reviewer, commenting via Wes’s GitHub account.
Changes requested: one copy regression, the rest holds up. The scaling fix is right. One new rejection message shows up on the wrong files (inline).
Validation
- I ran both new real-ffmpeg tests locally at
9bedcd8d(ffmpeg 9.0.1). Both produce 3290×2160. I also broke each assertion and confirmed it fails, so neither test is vacuous on a machine that has ffmpeg. - The relay's own
validate_video_file(block/buzz2f79374f4) accepts real outputs at 3290×2160, at 1920×3840 (rotated phone footage, autorotated) and at 3840×960 (2:1 SAR input). The 3406×2236 case writes apaspbox with SAR 14148:14147. That's harmless: the relay's metadata walk stops atstsd, and mp4 0.14 checks the codedavc1width and height. - Gap, non-blocking: hosted CI has no ffmpeg, so both new tests skip there. The JavaScript 1/2 log shows
skipping real conversion: ffmpeg not found. That matches the existing HEIC tests' deliberate convention, so this PR doesn't need to fix it. Until a required lane provisions ffmpeg, CI is not evidence for the filter.
Merge criteria: make the container copy accurate for the files that actually receive it, and update the M4A /zzzz fixture to match. The native live-relay upload listed as not yet run is still worth one try before merge.
| "Videos can be at most 10 minutes long. Trim this recording, then retry.", | ||
| codec: | ||
| "This video’s codec isn’t supported. Export it as an H.264 MP4, then retry.", | ||
| container: |
There was a problem hiding this comment.
🤖 This copy appears for files that aren't QuickTime and aren't videos. The relay sends any ftyp file down its video path (buzz-relay/src/api/media.rs should_stream_as_video → looks_like_iso_bmff). If no brand is in MP4_BRANDS, it returns UnsupportedContainer (buzz-media/src/upload.rs:400). Meanwhile, both preparation hosts transcode every brand videoDemuxer recognizes, qt included, so a real .mov never reaches the relay raw.
The files that actually get container are non-video BMFF the app passes through raw. An AVIF photo, for example, has brands avif mif1 miaf MA1B. That user is now told "QuickTime (.mov)… Export it as an MP4". Before this PR they saw the accurate generic rejected message.
Fix: make this copy format-neutral (for example "This file’s format isn’t supported here."), or map container back to rejected. Also update leaves non-video BMFF %s to relay rejection in attachments.test.ts. It still mocks the broker as { code: "rejected" }, but the real relay plus broker now return container for those brands.
There was a problem hiding this comment.
🤖 Agreed, fixed in 8338162. container is gone, and unsupported container maps back to the generic rejected message, since only non-video BMFF like AVIF reaches it. The M4A /zzzz fixture's { code: "rejected" } is accurate again. The broker and frontend tests now assert unsupported container → rejected.
| "This video is larger than 4K (3840×2160). Export it at a smaller size, then retry.", | ||
| duration: | ||
| "Videos can be at most 10 minutes long. Trim this recording, then retry.", | ||
| codec: |
There was a problem hiding this comment.
🤖 Nit, non-blocking: WrongCodec also fires for audio tracks. A metadata-free ALAC .m4a (brands M4A isom iso2) reaches validate_video_file and fails on the audio track. "This media’s codec isn’t supported…" would be accurate in every case.
There was a problem hiding this comment.
🤖 Taken in 8338162: "This file’s codec isn’t supported. Use H.264 video and AAC audio, then retry."
Round scaled edges to even values so padding never adds a black column to an even recording (3456x2234 now scales to 3342x2160, not 3341+pad). Drop the QuickTime-specific container message: prepared hosts convert every recognized video brand, so only non-video BMFF such as AVIF reaches that rejection and the generic message is accurate. Make the codec message cover audio tracks too. Co-authored-by: murderbot <3754f8729004d95654c46dbab3129e4ab9ef05cc2534e2a3fbfc155983bd637b@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: Bradley Axen <[email protected]>
…-drag * origin/main: (34 commits) fix(channels): resolve template agents from ready inventory (#922) [buzz-app] Fit uploaded videos to the relay's 4K envelope and name video rejections (#1129) feat(github): reveal the PR comment a link points at (#947) fix(messages): stop WebKit freeze from muted composer video previews (#1143) test(browser): fix route-unroute and virtua-hold webkit flakes (#1149) test(browser): let dnd-kit see its first resize before text shrinks (#1150) fix(previews): keep a hovered preview through scroll anchoring (#1138) Unify owned agent profile panels (#1083) feat(agents): import old Buzz teams from the Teams tab (#985) Mark Me chats as personal in the session description for remote agents (#1058) feat(messages): optional way to open buzz message links in a thread tab (#1113) Distinguish thread originals and tighten conversation spacing (#929) fix(buzz-mcp): page channel reads past the relay limit (#1131) fix(inbox): show a session's top-level timeline as one conversation (#1094) feat: reorder sidebar groups by dragging their headers (#1118) fix(avatars): request the cached relay thumbnail at every avatar site (#1054) fix(messages): soften quote styling and tighten spacing (#1098) fix(emoji): shrink and raise inline custom emoji (#1097) fix: follow host theme in media review chrome (#1117) Highlight an already-open thread tab when reopening it (#1050) ... Signed-off-by: John Tennant <[email protected]> Co-authored-by: Claude Worker <f425b86c3b780d43f8d9cf1802429d04dd985bf92eac44fca9468b5ad3887c5e@buzz.block.builderlab.xyz> # Conflicts: # src/features/channel-navigation/ChannelSidebarDnd.tsx
* origin/main: fix(navigation): share one abort reason so Settings → Back stops leaking DOM on V8 (#1161) Unify sidebar resizing and polish Solo navigation (#1132) Organize Builderlab agent identity and runtime settings (#1153) fix(channels): keep consecutive tab closes in the closing pane (#1158) feat(goose): add the goose agent type (#1125) feat: edit the last message with Cmd+Up (#1042) fix(channels): make Enter press the prominent lifecycle button (#1152) feat(me): drag chats between projects and Chats (#1112) fix(search): find open channels past the first 500 in Command-K (#1128) Stop in-flight request mocks from failing finished browser tests (#987) fix(channels): resolve template agents from ready inventory (#922) [buzz-app] Fit uploaded videos to the relay's 4K envelope and name video rejections (#1129) feat(github): reveal the PR comment a link points at (#947) fix(messages): stop WebKit freeze from muted composer video previews (#1143) Signed-off-by: John Tennant <[email protected]> # Conflicts: # docs/sessions/README.md
Problem
Full-screen QuickTime screen recordings on a Retina Mac are captured at native pixel size (for example 3406×2236). Desktop video preparation already converts
.movto MP4 with ffmpeg, but it kept that size. The relay caps video at 2160 on the short edge and 3840 on the long edge (block/buzzcrates/buzz-media/src/validation.rs), so it returned 422 "video resolution too high". The app discarded that text and showed "The server could not accept this file. Its format or metadata may not be supported."Change
media_preparation.rs, devbrowser-host/media-preparation.mjs) now scale videos down, never up, to fit 3840×2160 in either orientation. Scaled edges round to even values, so padding only touches odd-sized unscaled input. Videos already inside the limit are unchanged.relayRejection()inattachment-limits.tsmaps relay rejection text to its own code, for both the frontend reader and the dev broker. Resolution, duration (over 10 min) and codec rejections now get specific messages. Other rejections, includingunsupported container(only non-video BMFF such as AVIF gets that far, since both hosts convert every recognized video brand), keep the generic one.Validation
prepareMediaon baxen's real 3406×2236 QuickTime recording. Output:isomMP4, H.264, 3290×2160, top-level boxesftyp, moov, free, mdat.cargo test --lib relay::media_preparation(13), fullvitest run, andtsc --noEmit. On the full vitest run,read-state.test.tsfailed once and then passed 4/4 when run alone. This PR doesn't touch it.🤖 Generated with Claude Code