Skip to content

fix(ios): weight resolution pixel-distance so targetResolution isn't overridden by FPS#3964

Open
richkuo wants to merge 1 commit into
mrousavy:mainfrom
richkuo:fix/resolution-bias-vs-fps
Open

fix(ios): weight resolution pixel-distance so targetResolution isn't overridden by FPS#3964
richkuo wants to merge 1 commit into
mrousavy:mainfrom
richkuo:fix/resolution-bias-vs-fps

Conversation

@richkuo
Copy link
Copy Markdown

@richkuo richkuo commented May 29, 2026

Summary

Fixes a case where targetResolution is effectively ignored when a higher FPS is requested: on an iPhone front TrueDepth camera, asking for 1080p @ 60fps records 3840×2160@60 instead of the device's native 1920×1080@60 format.

Addresses #3963.

Root cause

In CMVideoDimensions.penalty(_:), pixel-distance is the raw log-ratio of pixel counts (range ~0–1.4), while the FPS penalty (AVFrameRateRange.penalty(for:)) is in raw frame units (max(undershoot, overshoot), up to 30) and carries a higher weight in ConstraintResolver's weighted sum. So resolution distance is a very weak signal — when the requested FPS isn't available at the target resolution, the resolver escalates resolution to satisfy FPS.

Per-format penalties at target 1080p / 60fps on a front TrueDepth camera (lower = selected):

format @60fps penalty
1920×1080 @60 54.5
1280×720 @60 62.6
3840×2160 @60 9.7 ← selected

The resolutionBias term actually favors 1080p (its logPixelDistance is 0; 4K's is 1.386), but it's swamped by the other constraints.

Change

Scale logPixelDistance by the existing aspectMismatchWeight so pixel-distance is weighted comparably to the aspect-ratio term — keeping the requested resolution a first-class signal. This is intentionally local to the resolution penalty and does not change FPS tie-breaking among equally-close resolutions.

let logPixelDistance = aspectMismatchWeight * Swift.abs(log(actualPixels / targetPixels))

Notes

  • Marked draft — I'm happy to take a different direction if you'd prefer, e.g. normalizing the FPS penalty to a relative scale instead of strengthening the resolution term. Either resolves the incommensurable-scales issue; this PR takes the smaller, more localized route.
  • Verified on-device: with this change, a front-camera 1k@60 request records native 1920×1080@60 (no escalation, no re-encode); 4k mode is unaffected.

LLM: Claude Opus 4.8 (1M) | Effort: High | Harness: Claude Code

…overridden by FPS

The resolution-bias penalty puts pixel-distance on a raw log scale
(~0-1.4), far weaker than the FPS penalty (raw frame units, up to 30).
So when a requested FPS isn't available at the target resolution, the
session escalates resolution to satisfy FPS — e.g. a front-camera
1080p@60 request lands on 4K@60 even though a native 1080p@60 format
exists. Scale pixel-distance by aspectMismatchWeight so the requested
resolution stays a first-class signal, with FPS as a tiebreaker among
equally-close resolutions.

Addresses mrousavy#3963
@vercel
Copy link
Copy Markdown

vercel Bot commented May 29, 2026

@richkuo is attempting to deploy a commit to the Margelo Team on Vercel.

A member of the Team first needs to authorize it.

@richkuo richkuo marked this pull request as ready for review May 29, 2026 13:49
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.

1 participant