Skip to content

feat(camera): NV12/Y16 decode + skip undecodable devices in auto-detection - #90

Open
tyvsmith wants to merge 5 commits into
mainfrom
fix/ipu7-camera-formats
Open

feat(camera): NV12/Y16 decode + skip undecodable devices in auto-detection#90
tyvsmith wants to merge 5 commits into
mainfrom
fix/ipu7-camera-formats

Conversation

@tyvsmith

@tyvsmith tyvsmith commented Aug 8, 2026

Copy link
Copy Markdown
Owner

Summary

First of three PRs for #89 (Intel IPU7 camera exposed through v4l2-relayd could not enroll).

  • NV12 decode (semi-planar 4:2:0) — the native output of Intel IPU6/IPU7 processed cameras via v4l2-relayd. No more relay reconfiguration to YUY2.
  • Y16 decode (16-bit IR grayscale) with bit-depth-aware conversion: shift derived from the frame's effective bit depth, so 10/12-bit sensors map to full 8-bit range while genuinely dark frames stay dark for the dark-frame gate. Unit-tested; not hardware-verified (no Y16 device available).
  • Auto-detection excludes undecodable devices from every selection tier — raw Bayer sensor nodes (e.g. IPU7's SGRBG10) can no longer be auto-selected. When nothing decodable exists, the error lists every device and its formats.
  • Camera::open fails fast with an actionable error instead of negotiating an undecodable format that every subsequent capture would silently reject (the issue's captured=0 symptom).
  • Negotiation priority is now quirk pref > GREY > Y16 > YUYV > NV12 > MJPG; docs/contracts.md auto-detection contract updated.
  • New docs: Intel IPU6/IPU7 + v4l2-relayd recipe in docs/compatibility.md (videoconvert caveat, SPLASHSRC for exclusive_caps flapping, and the security note that the relay path is RGB — security.require_ir must not be disabled as a workaround); "Enrollment captures 0 frames" troubleshooting section.

IR classification semantics (H1: GREY/Y16 alone is not proof of IR) are unchanged; only selection/decoding changed.

Validation

  • cargo clippy --workspace -- -D warnings clean; full workspace tests pass (new unit tests: NV12 converter incl. YUYV-equivalence, Y16 normalization, format selection priority/rejection, auto-detect filtering incl. BRIO multi-node regression corpus).
  • Container PAM tier: 22/22.
  • Hardware (Logitech BRIO): auto-detect still picks the IR node, negotiates GREY, live auth matches (similarity: 0.85); container oneshot tier's multi-node IR gate incl. "negotiated capture format is GREY" passes.
  • NV12/IPU7 end-to-end needs the reporter's hardware — @felipecsl, a test of this branch (or the merged main) on the XPS 14 with the relay's default FORMAT=NV12 and no [device] path override would be very welcome.

Part of #89

🤖 Generated with Claude Code

…ction

Intel IPU6/IPU7 MIPI cameras expose raw Bayer sensor nodes that facelock
cannot decode; auto-detection picked one and every capture failed silently
(issue #89). The processed v4l2-relayd camera outputs NV12, which was also
unsupported.

- Decode NV12 (semi-planar 4:2:0) and Y16 (16-bit IR grayscale, bit-depth-
  aware conversion that keeps dark frames dark) natively; negotiation
  priority is now GREY > Y16 > YUYV > NV12 > MJPG.
- Auto-detection excludes devices advertising no decodable format from every
  selection tier; when nothing decodable exists the error lists each device
  and its formats.
- Camera::open fails fast with an actionable error instead of negotiating an
  undecodable format that every subsequent capture would reject.
- Document the Intel IPU6/IPU7 + v4l2-relayd recipe (videoconvert caveat,
  SPLASHSRC for exclusive_caps flapping) and the RGB/require_ir security note.

Part of #89

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 8, 2026 19:52

Copilot AI 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.

Pull request overview

This PR extends facelock-camera to support additional V4L2 pixel formats commonly exposed by Intel IPU6/IPU7 relay pipelines, and hardens camera selection/opening so auto-detection avoids devices that cannot be decoded (e.g., raw Bayer sensor nodes). It also updates documentation/contracts and changelog to reflect the revised negotiation and detection behavior.

Changes:

  • Add NV12 (semi-planar 4:2:0) decode and Y16 (16-bit grayscale) decode with bit-depth-aware normalization.
  • Update camera format negotiation to prefer only decodable formats (plus optional quirk preference) and fail fast with actionable errors when none are supported.
  • Update auto-detection to exclude devices that advertise no decodable format, and expand docs/troubleshooting/compat guidance accordingly.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
docs/troubleshooting.md Adds “Enrollment captures 0 frames” troubleshooting steps tied to new failure modes and logging guidance.
docs/contracts.md Updates the auto-detection and open/negotiation contract to exclude undecodable devices and to fail fast.
docs/compatibility.md Documents NV12/Y16 support and adds an Intel IPU6/IPU7 + v4l2-relayd recipe and security note.
crates/facelock-camera/src/preprocess.rs Implements NV12→RGB and Y16→8-bit gray conversion plus unit tests.
crates/facelock-camera/src/device.rs Filters auto-detection to only consider devices advertising decodable formats; improves “none found” error detail.
crates/facelock-camera/src/capture.rs Centralizes decodable format priority, selects formats accordingly, and adds capture-time decode paths for NV12 and Y16.
CHANGELOG.md Records new format support and the auto-detection/open behavior changes.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread crates/facelock-camera/src/preprocess.rs
…ty, fourcc normalization

Y16 8-bit scaling is now pinned once at camera open instead of being derived
from each frame's max — security-relevant: the per-frame shift was contrast
normalization upstream of check_ir_texture, moving the scale that
ir_texture_min_stddev is calibrated against, and a single saturated glint pixel
blacked out whole frames mid-session. Bright samples now clamp to white rather
than wrapping.

NV12 UV rows are sized 2 * ceil(width/2), fixing an out-of-bounds panic on odd
widths where the old guard accepted a short buffer.

Camera::open rejects padded bytesperline for uncompressed formats, which would
otherwise shear every decoded frame on ISP devices.

V4L2 FourCCs are normalized once where they enter facelock (query_device, format
negotiation, quirk file load), so no comparison downstream trims.

A quirk format_preference naming an undecodable format is warned about and
ignored instead of winning negotiation and failing every capture.

nv12_to_rgb returns Option<Vec<u8>> instead of an empty-vec sentinel.

DECODABLE_FORMATS is pub(crate); nv12_to_rgb, y16_to_gray and y16_shift are
re-exported from the crate root.

IR-classified nodes excluded from auto-detection for lacking a decodable format
are logged with their path and formats.

docs/security.md documents the pinned Y16 scale; docs/compatibility.md covers
the intel_cvs requirement on Panther Lake and the absence of any Linux-reachable
IR sensor on IPU6/IPU7; the CHANGELOG no longer implies those laptops can
authenticate.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@tyvsmith

tyvsmith commented Aug 8, 2026

Copy link
Copy Markdown
Owner Author

Review fixes applied in c653af2 (principal review, all findings verified before fixing):

  • Y16 scale pinned at camera open (security): shift derived once from a calibration frame, stored on Camera, never recomputed — per-frame max derivation was attacker-modulatable contrast normalization upstream of the IR texture check, and a single specular glint blacked out whole frames. Noted in docs/security.md.
  • NV12 odd-width OOB panic fixed (UV stride = 2*ceil(w/2)), with 3x3 regression test.
  • Padded strides (bytesperline > row size) rejected at open with both values named, instead of decoding sheared frames.
  • FourCC normalized once at the query boundary; DECODABLE_FORMATS order pinned by test (changing it now requires a conscious contracts.md update).
  • Quirk format_preference naming an undecodable format is warned and ignored instead of winning negotiation.
  • nv12_to_rgb returns Option; excluded IR nodes are warn-logged with their formats; CHANGELOG no longer oversells (IPU relay ≠ IR); cargo fmt fixed.

Resolved judgment call: kept the hand-rolled YUV converters per the repo's dependency-minimalism rule (cross-converter equivalence test pins the math).

🤖 Generated with Claude Code

tyvsmith and others added 3 commits August 8, 2026 16:03
The 'no IR sensor is reachable on IPU6/IPU7' section overstated the facts:
no in-tree or Intel-shipped driver exists, but the out-of-tree
svp7500-camera-fix-pack has the HM1092 IR sensor streaming (incl. face
unlock with the in-tree ir_flood_led illuminator) on SVP7500-bridged IPU7
laptops, and kernel 7.2-rc1 gained an in-tree intel_cvs bridge driver.
See intel/ipu7-drivers#26 for the history; facelock-side enablement
(capture format + hm1092 quirk) is tracked in #101.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
An external review flagged that the session-wide Y16 scale was derived
from the single first frame after the stream starts, which races the
camera's own AGC/AE warmup. A dark or covered frame at that moment pins
an 8-bit scale, so every warmed-up 10/12-bit frame clips to flat white;
a glinted frame pins the opposite. Both directions fail closed (the flat
frame fails the IR texture check, the dark one fails the dark gate), so
this is a reliability fragility rather than a bypass — but Y16 has no
hardware coverage in the test fleet, which is exactly where a
scene-dependent constant should not live.

Two changes, in trust order:

- New quirk key `y16_bit_depth` (8..=16). Quirks are the repo's existing
  channel for hardware truth, so a device that declares its sensor depth
  gets `shift = bit_depth - 8` with no frame inspected at all. A depth
  outside the 16-bit container is treated as a quirks-file typo: warn
  and calibrate instead.
- The fallback now samples a burst (at least the device's declared
  `warmup_frames`, minimum 8, bounded to one second and tolerant of
  individual capture errors) and takes the peak sample across it. The
  peak is a lower bound on the sensor's full scale, so more frames can
  only move the estimate toward the true bit depth, never past it. A
  calibration that ends at 8-bit range or saturates the container warns
  and names the quirk key as the fix.

The security semantics are unchanged: the scale is still pinned once at
open and never recomputed per frame, because a per-frame scale is
contrast normalization upstream of the IR texture check whose
`min_stddev` cutoff assumes a fixed scale (docs/security.md §1.C).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@tyvsmith

tyvsmith commented Aug 9, 2026

Copy link
Copy Markdown
Owner Author

New info on the SGRBG10 exclusion in this PR (no change requested — merge as is)

Read the fix pack's howdy/ir_reader.py and IR-FINDINGS.md. They pin down the format this PR's auto-detect filter deliberately skips, and it turns out to be more interesting than "a raw Bayer sensor node".

The SGRBG10 node on IPU7 is the IR node. The HM1092 is physically monochrome; the driver tags it SGRBG10 ('BA10') only because that is what the Windows IPU graph settings declare. Measured layout: 648x368, bytesperline = 1344 (672 px of stride for 648 active), 16-bit LE containers holding 10-bit values, decoded as crop to width then >> 2. Honouring the Bayer tag is what breaks it — libcamera's SoftwareIsp debayers mono data, and the PipeWire path additionally renegotiates to 1920x1080 and lands the payload in a mostly-black buffer.

So the two behaviours this PR adds both correctly exclude the node today (we genuinely cannot decode it yet), and both will need a carve-out later:

  1. DECODABLE_FORMATS has no BA10, so auto-detection skips it — the PR description's "raw Bayer sensor nodes (e.g. IPU7's SGRBG10) can no longer be auto-selected" is accurate but reads as if the node is uninteresting. It is the IR sensor.
  2. expected_stride() fail-fasts on the padded stride: 648 x 2 = 1296 != 1344.

The decode itself is essentially already here: 10-bit-in-16-bit-LE >> 2 is this PR's Y16 path with y16_bit_depth = 10. What is missing is a stride-aware arm plus three subsystems facelock does not have at all (no references to media-ctl, v4l-subdev, or /sys/class/leds anywhere in the workspace; ir_emitter.rs is UVC-XU-only):

  • BA10/SGRBG10-as-mono decode arm, routed through the existing Y16 conversion with the shift pinned by y16_bit_depth = 10
  • Stride-aware capture — support padded bytesperline (crop per row) instead of rejecting it, for formats where padding is expected
  • Media-graph configuration (VIDIOC_SUBDEV_S_FMT + MEDIA_IOC_SETUP_LINK, or media-ctl, or a boot-time oneshot). On a fresh boot the CSI2 -> capture link is disabled and the CSI-2 pads sit at 4096x3072, so VIDIOC_STREAMON yields zero frames. The v4l crate does not cover subdevs. New subsystem, not a format arm.
  • Entity-name device resolution/dev/videoN and /dev/v4l-subdevN shuffle between boots on IPU7, and the CSI-2 port differs per machine (port 2 on XPS 16 DA16260, port 1 on Dell Pro 14)
  • sysfs LED-class emitter backend for /sys/class/leds/HIMX1092_00::ir_flood_led/brightness, with guaranteed off-on-Drop (their Python version leaked the illuminator on after every auth when os.open was torn down during interpreter shutdown). INT3472 owns the GPIO, so the sensor driver reports ir_led=none and cannot drive it.
  • force_ir quirk on a hm1092/HIMX1092 name pattern
  • No rotation quirk — the sensor applies H+V flip in hardware (reg 0x0101=0x03); only the RGB relay needs rotate-180

Full detail, including the link_frequency = 180,480,000 prerequisite and what still needs hardware confirmation, is in #101.

Nothing here blocks this PR — the exclusion is the right behaviour until the decode arm exists. Filing it so the next person who reads DECODABLE_FORMATS does not conclude the IPU7 IR node is a dead end.

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