Modernize CoreML loading & inference for stateful models, safer optimizations, and UX clarity - #1
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… optimizations, and UX clarity
|
@codex review |
andrewginns
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let seconds = Double(detectionTime.components.seconds) + (Double(detectionTime.components.attoseconds) / 1_000_000_000_000_000_000) | ||
| let msTime = String(format: "%.0f ms", seconds * 1000) |
There was a problem hiding this comment.
Stateful inference converts detectionTime.components.attoseconds by dividing by 1_000_000_000_000_000, but attoseconds are 1e-18 seconds. The result is 1,000× too large, so stateful models report ~seconds instead of milliseconds, driving detectionFPS near zero and causing getRepeatInterval to schedule frame processing in multi‑second gaps. Divide by 1e18 to derive accurate seconds for stateful models.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
No change needed. The current code in CoreMLPlayer/Classes/Base.swift already divides attoseconds by 1_000_000_000_000_000_000 (1e18), which is the correct conversion from attoseconds to seconds for both the stateful path (performStatefulDetection) and the Vision path (asDetectedObjects). For example, a 15 ms duration yields 15_000_000_000_000_000 attoseconds → 0.015 seconds → 15 ms, which keeps detectionFPS and getRepeatInterval consistent. The review comment likely misread the constant (1e15 vs 1e18), so there isn’t a bug to fix here.
1 participant
Raised macOS baseline to 14 and refreshed model loading: async MLModel.load, arch-aware compute defaults, background-task flag, function persistence, and warm-up with cached VN requests/state support.
Added safer optimization flow: quantize to Application Support, validate size/compilability, surface user-facing warnings when toolchain missing or candidates invalid, and expose them in the model settings UI.
Hardened inference pipelines: shared serial queue, orientation from track transform, pixel-format fallback, crop/scale derived from ideal format, and mid-gray warm-up buffers to better mimic real inputs.
Improved scheduling/stats UX: detection run off main thread, guarded repeat-interval clamping, main-thread stats updates, and async image detection to avoid UI stalls.
Expanded tests for optimization warnings/fallbacks, state carry-over, crop/scale selection, pixel-format fallback, function autoloading, and repeat-interval clamping.