diff --git a/packet/README.md b/packet/README.md new file mode 100644 index 000000000..98fa39619 --- /dev/null +++ b/packet/README.md @@ -0,0 +1,101 @@ +# The capture packet + +The capture packet is how a scan leaves the phone. It is specified by the server team's capture +packet spec, `formatVersion` 0.4, together with its intake API. The spec is +`docs/capture-packet-spec.md` in the private repository `huntertcarver/house-scanning-server`, +and the intake API is its Appendix D. That spec is the one contract; this folder does not define +a format of its own. + +This folder holds the app side's proposals for the next revision (0.5): what the app records +that 0.4 has no field for, and three arrival checks for the server's validator +([validator-checks.md](validator-checks.md)). Packet 1.1, which this folder used to define, is +retired; it is still readable at `t3/packet` commit `d5439cf`. + +## Proposals for 0.5 + +Each item is something the app knows at capture time and the server would otherwise have to +guess. All eleven are unapproved proposals. None is part of the current contract, and the app must +not send them until the contract's owners adopt them in a version and with a compatibility rule +of their choosing. The study numbers cited motivate device tests; none is measured phone +performance. + +1. **The meter anchor's orientation.** 0.4 records the meter tap's point, not the wall it sits + on. Every placement distance is measured along that wall from the meter (C1's `s = 0`), and + the app knows the wall at the tap: the vertical plane its raycast hit. Proposal: on the + `meter` tap, a `meterAnchor` with `transform` (anchor to world in the tap's epoch: origin at + the meter, +y up, +z the wall's outward normal, +x along the wall to the right seen from + outside) and `normalSource`: `single_raycast` (one plane hit at the tap), `fitted` (fitted + across the walk), `mesh` or `estimated`. The source matters: the signals lab (#59, + `experiments/edge-geometry`) found a yaw error of 2.2° median and 6.7° p90 for a stand-in for + the app's plane, a 0.3 m plane fitted to depth on ETH3D photos, not a device raycast. At that + p90, 10 ft along the wall moves a point about 1 ft. A per-capture yaw uncertainty is worth + adding once a device study measures it. +2. **The ground's height at the meter.** 0.4 notes that world y = 0 is the phone's height at + session start, not the ground. The app measures the ground where it can: the horizontal plane + below the meter. Proposal: `meterAnchor.groundY` (world meters) with `groundSource` + (`plane`, `mesh`, `assumed`), so headroom and "ground under the battery" start from a + measurement when there is one. +3. **What a marked end or feature is.** 0.4's tap labels name the point, not the answers the + homeowner gave about it: + - whether a wall end is a real end (something blocks it) or unexplored (it turns a corner, + or no answer), which decides whether the server may reject for want of space; + - whether a window opens, which the rules may use (whether a fixed window is exempt from the + opening clearance is the rules' choice, not the app's); + - which corner taps belong to the same door or window. + + Proposal: `taps[].attrs` with `endKind` (`limit`, `unexplored`) and `operable`, and + `taps[].group` for taps that outline one object. +4. **The guidance log.** Every request the homeowner was shown (walk, tilt to the ground, mark + an end, close-up, a gap the phone or the server asked for), when, and its outcome: `met`, + `skipped`, `cannot_reach`, `superseded` or `unresolved`. Today "I can't get there" never + reaches the server, so the server can ask for the same unreachable view again. Proposal: + `guidance[]` with `id`, `kind`, `origin` (`phone`, `server`), `message`, optional `band` and + `span`, `tShown`, `tResolved` and `outcome`. +5. **Depth between keyframes** (from S6's live map, draft PR #21). Keyframes are kept about every + 0.5 m, but the phone fuses depth at about 10 Hz to decide what was seen. With keyframe depth + alone the server cannot rebuild that decision. Proposal: optional `depthFrames[]` with `t`, + `epoch`, pose, intrinsics of the depth map's own grid, and the same depth and confidence files + as a keyframe. At 256 × 192, one frame with confidence is about 246 KB, so a few hertz rather + than ten. +6. **Estimated depth with its uncertainty** (S6). A phone without LiDAR can infer metric depth + from the image, but used without its uncertainty it creates surfaces that are not there. + Proposal: a depth `source` of `estimated`, which requires a `sigma` file (float32 meters, one + standard deviation per depth pixel, the depth's layout). +7. **The frame of a plane's boundary.** 0.4 lists `planes[].boundary` without saying its frame. + ARKit's `boundaryVertices` are relative to the anchor transform, not to `planeExtent`'s centre + and `rotationOnYAxis`, and packet 1.1 found that mixing the two is an easy mistake. Proposal: + carry the anchor-frame boundary in a new field whose frame is stated, rather than giving the + existing `boundary` a meaning an existing reader might not share, and have the validator check + it lies within `planeExtent` once the centre and rotation are applied. +8. **Photos that frame one span.** With learned depth on ETH3D photos, an along-wall edge came to + 0.9 in p90 when seen within 15° of face-on and 24 in beyond 45° (#59, + `experiments/edge-geometry`), so a distance measured inside one face-on photo is likely far + better than one chained across the walk. Proposal: a still or + keyframe purpose `span` that names the marks the photo frames. +9. **Scale references in view.** One reference near the meter may not be enough: on MARViN's + walks ARKit's scale varied 1.8% (robust SD) within a walk (#59, `experiments/drift-anatomy`), + so a reference helps where it appears. Proposal: `references[]` with `kind` (door, brick course, and hand-held ones such + as an ID card or a Letter sheet), nominal size, and its corners in each photo that shows it. + Hand-held references ask the homeowner to hold something; #59 rates them worth a device test + (`experiments/sensor-budget`), and whether the flow may ask is Sam's and Hunter's call. +10. **The meter anchor's pose over time.** ARKit re-estimates an anchor as tracking corrects, so + the anchor pose at tap time goes stale. Proposal: an optional stream of the meter anchor's + world pose at each ARKit update, so the server can see how far the meter frame moved. Each + sample needs its timestamp, on the pose stream's clock, and its epoch: the epoch tells a + world reset from a correction within one world, and the timestamp ties the sample to the + keyframes and taps around it. There is no measurement of that movement yet. +11. **Distances from a second phone (low priority).** When a second phone with UWB is present, + an optional stream of phone-to-phone distances. In #59's simulation on MARViN walks + (`experiments/drift-anatomy`, question A4), a second phone left at the meter cut the p90 error + at 20 ft from 10.4 in to 7.7 in at 10 cm ranging noise, and to 4.9 in at 5 cm. That result + depends on the second phone staying still at a known place, so the proposal requires three + things: where the second phone stands (at the meter, or a stated position) and that it did + not move; a time for each range on the capture's clock, so it matches a pose; and the range's + uncertainty. Ranges from a second phone that moves, or whose place is unknown, are not what + the study simulated. + +Already in 0.4, so not proposed: the kind and distance of each tap's hit, a keyframe for every +tap, and feature-point identifiers. Packet 1.1 also had a fixed sharpness score, distance walked +and a flag for consent to share location. They are not proposed here: distance walked can be +derived from the pose stream, and whether a sharpness score or a sharing-consent record belongs +in the contract is for its owners. diff --git a/packet/validator-checks.md b/packet/validator-checks.md new file mode 100644 index 000000000..37a8cf041 --- /dev/null +++ b/packet/validator-checks.md @@ -0,0 +1,92 @@ +# Three arrival checks for the capture packet validator + +Three checks we propose for the server's validator, each catching a mistake that each part of a +packet can pass on its own. We read the server's spec and parts of its code but did not audit its +whole validator, so some of these may already exist there. They are described here in our own +words; nothing is copied from the server's private code. Each ran in the packet 1.1 validator (`t3/packet` `d5439cf`, `packet/validate.py`) +with a test that breaks it on purpose. Whether they reach the server as a pull request or stay a +proposal is Sam's and Hunter's call. + +## 1. Keyframes lie on the pose stream + +For each keyframe, find the `arkitPoses` row of the same epoch nearest its `timestamp`. It must be +within one frame of the stream's rate (1/60 s at 60 Hz), and its position within 1 cm of the +keyframe pose's translation. + +**What it catches.** Keyframes and the pose stream written in different frames, or on different +clocks: for example, keyframe poses converted to another frame while the stream stays raw, or +timestamps taken from `Date` on one side and uptime on the other. Every other check passes such a +packet, because each part is valid on its own. + +**Limits.** It compares positions only: a keyframe with the right position and a wrong rotation +(an inverted transform, other camera axes) passes. A rotation tolerance needs a measured basis +first. Packet 1.1 had one world per packet, so matching within an epoch is part of the proposal, +not something 1.1 tested. + +**Evidence.** On the ADVIO sample, 79 keyframes matched their stream rows exactly. Moving one +keyframe 10 cm, or its timestamp past the stream's end, fails the check (1.1 tests +`test_a_photo_off_its_trajectory` and `test_a_photo_between_trajectory_samples`). + +## 2. Depth has its keyframe's shape + +A depth map covers its keyframe's field of view at lower resolution, so: +- its `w / h` equals the image's within 1%, and it is no larger than the image; +- values are finite and non-negative, with 0 for no measurement, and at least 1% are measured; +- confidence values are at most 2. + +**What it catches.** A depth map rotated 90° or cropped to another aspect (for example a portrait +map with a landscape image), which a byte-count check passes because `w × h` is unchanged. It +also catches NaN written for "no reading", and an all-zero map. + +**Limits.** It checks shape, not alignment. A map flipped 180°, mirrored, or taken from another +keyframe with the same size passes. Proving alignment would need orientation and capture-link +fields the contract does not have. + +**Evidence.** On ETH3D electro with laser-rendered depth, ETH3D's own 3-D points reprojected +through the packet's poses to a median 0.65 to 1.01 px, and the depth agreed with them to a median +0.2 to 0.6%. The aspect, NaN and confidence tests in 1.1's `tests/test_packet.py` each fail their +case. + +## 3. The mesh file is what the manifest says + +`geometry/mesh.ply` is a binary little-endian PLY. 0.4 does not fix its index and count types, so +this check proposes one layout as well: +- `float x, y, z` per vertex, and per face a `uchar` count followed by that many `int` indices, + then an optional `uchar classification`; +- the body length equals exactly what the vertex and face counts imply; +- vertex and face counts are non-negative, and every vertex coordinate is finite; +- every face is a triangle, and every index satisfies `0 <= index < vertex_count`; +- classification values are 0 to 7 (`ARMeshClassification`), and all 0 when `mesh.classified` is + false. + +**What it catches.** A truncated upload that still hashes as sent, a negative index or one past +the vertex list (which crashes or corrupts a reconstruction reader), `NaN` or infinite vertices, `double` coordinates or `uint` counts from a +different exporter, and classes written when classification was off, which cannot be told from +ARKit's own 0 ("none"). + +**Evidence.** 1.1's `tests/test_checks.py` breaks each of these in turn (index out of range, +class 8, a truncated body, a changed property type). The ARKit mesh has not yet been exercised +from a device. + +## Hazards the 1.1 review found + +Reviewers of the 1.1 validator (#22) found ways a packet passed that it should not. Each applies to +any validator that reads packets the same way: + +- **Symlinks.** A folder packet whose file is a symlink to an identical file outside it passes + a path-spelling check. Resolve each path and require it to stay inside the packet. +- **Duplicate zip entries.** Python's `ZipFile` reads the last entry of a repeated name, while + other unzip tools take the first, so a bad first copy can hide behind a good second one. Refuse + a zip with repeated names. +- **Truncated JPEGs.** Pillow's `Image.open` reads only the header, so format, size and EXIF can + look right on a truncated file. Decode the pixels (`Image.load()`) before accepting it. +- **Malformed binaries.** Parsing a short mesh or depth body can raise before the length check + runs. Report it as a problem, not an exception. +- **Out-of-range numbers.** `json.loads` accepts `NaN` and `Infinity` by default, and parses the + valid JSON number `1e400` as infinity without calling `parse_constant`. It parses a 400-digit + integer as an exact `int`, on which `math.isfinite` raises `OverflowError` rather than + returning `False`. So check floats with `math.isfinite`, check integers against the range their + field allows (a count, an index, a size) before any conversion to float, and report a value + that fails either check, or cannot be converted, as a problem, not an exception. +- **Stream times outside the capture.** A stream with samples far outside the session's time span + passed with only a warning. Decide whether that is an error.