fix(moq-video): pick the V4L2 mode nearest the requested resolution - #3355
Merged
Conversation
Frando
added a commit
to Frando/moq
that referenced
this pull request
Sep 3, 2026
…oq-dev#3355) Squashed from `pr/v4l2-mode-nearest` at 9bc5098, open as moq-dev#3355.
kixelated
marked this pull request as ready for review
September 5, 2026 00:29
Contributor
|
Warning Review limit reachedNext included review available in 13 minutes. View limit detailsLimit details: You’ve used all 4 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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. |
![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.
Reviewed commit: 9bc5098dbe
ℹ️ 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".
`negotiate` asked for YUYV, then MJPEG, and kept the first reply that came back as a format we convert. V4L2 has no "what would you offer me" call: `VIDIOC_S_FMT` asks and applies in one step, substituting the driver's nearest supported mode for anything it does not have. A successful reply therefore says nothing about geometry, and taking the first one pins most laptop webcams to VGA. USB bandwidth does not fit uncompressed 4:2:2 above that, so they offer YUYV only at small sizes and reach HD through MJPEG alone; asking such a camera for YUYV at 720p gets 640x480 back, which is a valid YUYV mode and nowhere near what the caller asked for. Probe every format we handle, score each reply against the requested size, and re-apply the winner, so the geometry decides and the format only breaks ties. Ties go to YUYV, which resamples, over MJPEG, which costs a JPEG decode per frame. Also report what the driver actually offered when nothing is convertible: the error printed a `FourCC` with its derived `Debug`, so an IR camera that speaks only GREY came out as `repr: [71, 82, 69, 89]`. Confirmed against an Integrated Camera (UVC) that offers MJPEG up to 2592x1944 but YUYV only at 640x480 and 640x360: requests for 1280x720 and 1920x1080 each returned 640x480 before and now negotiate exactly.
kixelated
force-pushed
the
pr/v4l2-mode-nearest
branch
from
September 5, 2026 01:50
9bc5098 to
1c5ccb6
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c5ccb68b4
ℹ️ 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".
Co-Authored-By: Codex <codex@openai.com>
kixelated
force-pushed
the
pr/v4l2-mode-nearest
branch
from
September 5, 2026 02:02
1c5ccb6 to
0b9ca41
Compare
Open
steelhead99x
added a commit
to steelhead99x/moq
that referenced
this pull request
Sep 5, 2026
* docs(quest): settle scope narrowing in place, and mark pre-media sidecar placement (moq-dev#3427) Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> * fix(moq-video): pick the V4L2 mode nearest the requested resolution (moq-dev#3355) Co-authored-by: Luke Curley <kixelated@gmail.com> Co-authored-by: Codex <codex@openai.com> * feat(moq-video): add the Android MediaCodec encoder and decoder (moq-dev#3354) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Luke Curley <kixelated@gmail.com> * docs(quest): import the post-grooming issues as quests (moq-dev#3431) Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> * docs(quest): apply the Codex findings on the issue import (moq-dev#3432) Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> * fix(claude): adopt a quest branch at the remote tip that was inspected (moq-dev#3421) Co-authored-by: Claude Opus 5 <noreply@anthropic.com> * docs(quest): record four findings from the m1 quest wave (moq-dev#3424) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: reorganize the site around what a reader can do (moq-dev#3426) Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> * chore: ignore Claude Code's scratch directories (moq-dev#3428) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(moq-audio,moq-cli): assert publish_capture stays Send off macOS (moq-dev#3433) Co-authored-by: Claude Opus 5 <noreply@anthropic.com> * docs: correct claims found during merge review (moq-dev#3435) Co-authored-by: GPT-5 <noreply@openai.com> * docs(quest): import the open issues that had no quest, and gate the dev merge (moq-dev#3434) Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> * docs(moq-audio): scope the local-task guidance to macOS (moq-dev#3436) Co-authored-by: Claude Opus 5 <noreply@anthropic.com> * docs: track deferred review findings (moq-dev#3438) Co-authored-by: GPT-5 <noreply@openai.com> * chore: remove redundant packaging work and plan relay ownership fixes (moq-dev#3440) Co-authored-by: GPT-6 <noreply@openai.com> * perf(net): avoid redundant chunk copies and plan performance investigations (moq-dev#3443) Co-authored-by: GPT-6 <noreply@openai.com> * fix(transcode): follow a source resolution change with the ladder (moq-dev#3381) Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: GPT-5 <noreply@openai.com> * feat(watch): share one AudioContext across audio decoders Spatial playback needs every remote in the same Web Audio graph. Injected contexts are never closed. Co-Authored-By: Cursor Grok 4.6 <noreply@cursor.com> Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Luke Curley <kixelated@gmail.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> Co-authored-by: Franz Heinzmann <frando@unbiskant.org> Co-authored-by: Codex <codex@openai.com> Co-authored-by: GPT-5 <noreply@openai.com> Co-authored-by: Cursor Grok 4.6 <noreply@cursor.com> Co-authored-by: Cursor <cursoragent@cursor.com>
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
2 participants
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.
negotiateasked for YUYV, then MJPEG, and kept the first reply that came back as a format this crate can convert. Taking the first one pins most laptop webcams to VGA.This PR is part of a series to update iroh-live to latest moq, see n0-computer/iroh-live#45. The initial code and description were written by Claude Code; the final review repair was written by Codex.
The problem
V4L2's non-mutating
VIDIOC_TRY_FMTis optional. The requiredVIDIOC_S_FMTasks and applies in one step, substituting the driver's nearest supported mode for anything it does not have, and it reports success either way. A successful reply therefore says nothing about the geometry or pixel format that came back.USB bandwidth does not fit uncompressed 4:2:2 much above VGA, so a typical UVC camera offers YUYV only at small sizes and reaches HD through MJPEG alone. Asking such a camera for YUYV at 720p gets 640x480: a valid YUYV mode, a successful call, and nowhere near what the caller asked for. Since YUYV was tried first and its reply accepted, MJPEG never got asked, and a camera capable of 1080p published VGA.
The fix
Test plan
just fix origin/mainand GitHub Check, Test, and Swift workflows.Public API changes
None.
negotiateis internal to the V4L2 capture backend.(Written by GPT-5.6 Sol)