Skip to content

potential fix: av foundation take the duration from the matched range… - #1314

Merged
stephen-derosa merged 1 commit into
ladvoc/livekit-capturefrom
sderosa/livekit-capture/potential-bug
Aug 4, 2026
Merged

potential fix: av foundation take the duration from the matched range…#1314
stephen-derosa merged 1 commit into
ladvoc/livekit-capturefrom
sderosa/livekit-capture/potential-bug

Conversation

@stephen-derosa

Copy link
Copy Markdown
Contributor

No description provided.

…'s own bounds instead of deriving it from the rounded fps.
@stephen-derosa stephen-derosa self-assigned this Aug 4, 2026
@stephen-derosa
stephen-derosa requested a review from ladvoc as a code owner August 4, 2026 17:08

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

View 3 additional findings in Devin Review.

Open in Devin Review

Comment on lines +953 to +957
} else {
// The rate lies strictly inside the range, so its reciprocal lies
// strictly inside the range's duration bounds.
// SAFETY: `requested` is finite and greater than zero here.
unsafe { CMTime::with_seconds(1.0 / requested, 600) }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Requested frame rates that don't divide evenly can still crash the app on some cameras

The frame time is rounded to a fixed 600-tick clock (CMTime::with_seconds(1.0 / requested, 600) at livekit-capture/src/sources/device/avfoundation.rs:957) without being clamped to what the camera advertises, so for rates such as 45 the value can land just outside the accepted window and abort the app.
Impact: On certain cameras, asking for a frame rate the fixed clock cannot represent exactly makes the application terminate instead of starting capture.

Rounding to a 600 timescale can exceed the range's minFrameDuration

device_format_frame_duration snaps to maxFrameDuration/minFrameDuration only at the endpoints; for a rate strictly inside the range it derives the duration from 1.0 / requested with timescale 600. Only rates that divide 600 (1,2,3,4,5,6,8,10,12,15,20,24,25,30,40,50,60,75,…) are exactly representable. For 45 fps, 600/45 = 13.33 rounds to 13 ticks → 46.15 fps, i.e. a duration shorter than the range's minFrameDuration whenever maxFrameRate lies in (45, 46.15) — exactly the near-integral advertised maxima this PR was written to handle (e.g. max = 45.00003). The resulting value is then handed to device.setActiveVideoMinFrameDuration/setActiveVideoMaxFrameDuration (livekit-capture/src/sources/device/avfoundation.rs:1033-1036) or input.setActiveLockedVideoFrameDuration (livekit-capture/src/sources/device/avfoundation.rs:443), which raises the uncatchable Objective-C exception the comment describes.

Prompt for agents
In device_format_frame_duration (livekit-capture/src/sources/device/avfoundation.rs), the interior-of-range case builds the duration with CMTime::with_seconds(1.0 / requested, 600). A timescale of 600 cannot exactly represent every integer frame rate (e.g. 45 fps needs 13.33 ticks), and CMTimeMakeWithSeconds rounds, so the resulting duration can be shorter than the range's minFrameDuration when maxFrameRate is only slightly above the requested rate. Since an out-of-range duration triggers an uncatchable Objective-C exception at the setActiveVideoMin/MaxFrameDuration and setActiveLockedVideoFrameDuration call sites, the computed duration should be exact and/or clamped. Options: build the CMTime exactly as value=1, timescale=framerate, and/or clamp the resulting CMTime into [minFrameDuration, maxFrameDuration] of the matched range before returning.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@ladvoc ladvoc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me! What version of macOS did you test on?

@stephen-derosa

Copy link
Copy Markdown
Contributor Author

Makes sense to me! What version of macOS did you test on?

26.5.2 (25F84)!

@stephen-derosa
stephen-derosa merged commit 254c7d3 into ladvoc/livekit-capture Aug 4, 2026
2 checks passed
@stephen-derosa
stephen-derosa deleted the sderosa/livekit-capture/potential-bug branch August 4, 2026 18:21
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.

2 participants