fix(media): accept portrait video at the same bound as landscape - #5560
Closed
AleksandarIlic wants to merge 1 commit into
Closed
AleksandarIlic wants to merge 1 commit into
AleksandarIlic wants to merge 1 commit into
Conversation
## Summary - compare the video resolution bound against the long and short edge instead of raw width and height - cover portrait 4K (2160x3840) and a portrait clip one pixel past the long edge ## Root cause `validate_video_file()` checked `width > 3840 || height > 2160`, which reads the bound as landscape-only. The same 4K frame is 3840x2160 landscape and 2160x3840 portrait: the first is accepted, the second rejected on height, despite being identical in pixel count, codec, and file size. This affects anything recorded in portrait above 2160 tall — 4K phone video, and native-resolution phone screen recordings such as 1180x2556. No client caps resolution before upload: the desktop transcode pads to even dimensions without downscaling and mobile uploads at native resolution, so these files are rejected only after the whole body has been received. Landscape behaviour is unchanged, and the bound still applies in portrait — 2160x3841 is still rejected. ## Validation - `cargo test -p buzz-media`: 110 passed, including the two new cases; the existing 3841x2161 rejection test is unaffected - `cargo check --workspace --all-targets`, `cargo clippy -p buzz-media -p buzz-relay --all-targets`, `cargo fmt --all --check`: clean - `cargo test -p buzz-relay --lib`: 859 passed; the 8 failures are pre-existing and infra-dependent (`Sqlx(PoolTimedOut)`, no local Postgres/Redis) Signed-off-by: Aleksandar Ilic <aleksandar@appollo41.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Root cause
validate_video_file()checkedwidth > 3840 || height > 2160, which reads the bound as landscape-only. The same 4K frame is 3840x2160 landscape and 2160x3840 portrait: the first is accepted, the second rejected on height, despite being identical in pixel count, codec, and file size.This affects anything recorded in portrait above 2160 tall — 4K phone video, and native-resolution phone screen recordings such as 1180x2556. No client caps resolution before upload: the desktop transcode pads to even dimensions without downscaling and mobile uploads at native resolution, so these files are rejected only after the whole body has been received.
Landscape behaviour is unchanged, and the bound still applies in portrait — 2160x3841 is still rejected.
Validation
cargo test -p buzz-media: 110 passed, including the two new cases; the existing 3841x2161 rejection test is unaffectedcargo check --workspace --all-targets,cargo clippy -p buzz-media -p buzz-relay --all-targets,cargo fmt --all --check: cleancargo test -p buzz-relay --lib: 859 passed; the 8 failures are pre-existing and infra-dependent (Sqlx(PoolTimedOut), no local Postgres/Redis)Summary
Related issue
Testing