fix(android): report the clip an Android recording really captured - #2566
Conversation
|
Size Report
Startup median (7 runs, lower is better):
|
|
No blocking findings at e590e85. The measured duration reaches the response and recovery metadata, and the short-clip warning reaches normal CLI output. The reported Android run matches ffprobe and current checks are green. The shared MP4 reader replaces the existing scan and reasonably accounts for the size increase; ready for human review. |
thymikee
added
the
ready-for-human
Android `screenrecord` encodes a frame only when the screen changes, so a window that ends on an unchanged screen returns a video far shorter than the requested duration, and `record stop` had nothing to say about it: the reported `durationMs` is host wall clock from `record start` until the export finished, which is not the length of the file that was just pulled. `record stop` now measures the pulled MP4 timelines and reports them as `capturedDurationMs`, and warns with the clip length against the window when the video is two or more seconds short. The window is measured on the device's own elapsed clock, read before the stop signal and at launch, because host wall clock drifts against the clock the encoder timestamps frames with; an unreadable clock or a chunk that answers no duration costs the caller the claim, never the recording. Measuring the timeline needed an ISO-BMFF box walk, which now lives in `@agent-device/capture-kit/recording-mp4-duration` and replaces the private top-level atom scan that MP4 container detection was doing. Stop replay through daemon recovery carries the field too, so a completion read back from the session resource reports the same numbers it did live.
…n holds `@agent-device/capture-kit/recording-mp4-duration` and its fixture sibling are new declared package subpaths, so the boundary enumeration that holds every exported workspace subpath has to name them for the layering scan to accept the Android recorder's read of a pulled clip's timeline.
…ated The Coverage job's ADR-0019 eager-closure probe failed: `recording/video.ts` evaluated 25 modules on import where the merge-base evaluated 24, because the MP4 container gate statically imported the box walk it now shares with the clip-duration read, and `recording/overlay.ts` grew by the same module. An entry the merge-base already carries gets no growth budget, so the edge moves behind a function-scoped `await import`: the scan is something recording completion asks for, and importing this module for `waitForStableFile` or WebM detection should not evaluate a box walker. The alternative the probe offered -- hosting the walker in a module both growing entries already evaluate -- would have put an ISO-BMFF walk in `swift-cache.ts` or `video-webm.ts`, or made the duration read import the Swift validator machinery that sits behind `video.ts`.
e590e85 to
625dc00
Compare
|
Human review at 625dc00. The MP4 side is fine: the walker is a fair replacement for the old top-level scan since it has to descend into 1. The device clock is not needed; use host time. 2. Merge 3. The function-scoped 4. Not for this PR: Requested changes: 1 and 2. |
Human review of #2566: the device-clock read defended against host-vs-encoder drift that does not matter at this threshold. Quartz drifts by tens of ppm, so a 30-minute chunked recording moves the window under 100 ms against a 2s warning threshold, while the read cost a transport operation, its own probe budget, and two adb round trips per recording. The window is now the host elapsed time between `Date.now()` immediately before the recorder launches and `Date.now()` immediately before the stop signal, so the contract change and the extra device I/O are gone, and the one case where the clocks genuinely diverge -- a host that sleeps mid-recording -- reports a shorter window and misses the warning rather than inventing one. The surviving clock arithmetic is one subtraction, so it lives in the window module that already owns that concern instead of a module of its own. A stop recovered through daemon recovery now passes the manifest's own start instant, which is the first host timestamp the recording ever had, so a recovered stop gets the same comparison a live stop gets.
|
Addressed in 1. Device clock removed. 2. Live at 3. Left the function-scoped 4. Agreed, and the count is worse than the comment says for the next field: the daemon metadata encoder, both hand-written validators, the stop-recovery reader, the response builder, two contracts and the MCP schema. A |
1 participant
Summary
Android
screenrecordencodes a frame only when the screen changes, so a window ending on an unchanged screen returns a video far shorter than the requested duration whilerecord stopreported only host wall-clockdurationMs.record stopnow also reportscapturedDurationMs, measured from the pulled MP4 timelines, and warns with the clip length against the window the host bracketed around the recorder:Date.now()immediately before the recorder launches and immediately before the stop signal. A host that slept mid-recording reports a shorter window, which costs the warning rather than inventing one. iOS is unchanged.Closes #2551. 30 files, inside the recording family plus its contracts, MCP schema, help and docs. Measuring the timeline needed an ISO-BMFF box walk, which replaces the private atom scan MP4 container detection used to do.
Validation
Rebased onto
04052fdcd2; conflicts were keep-both merges over #2564'sprobeRunningWritersop and its manifest-retirement help. Human review findings 1 and 2 are ind4c0ad2adb: the device clock is gone and the window module owns the clock arithmetic.emulator-5554atd4c0ad2adb: swipe then 10s idle →capturedDurationMs: 6736againstffprobe duration=6.736144, warning above, recorder and session gone. Earlier heads measured13102/13.101478and9733/9.732689.pnpm check:affected --runatd4c0ad2adb: every gate passed exceptmutation-model, which fails identically at04052fdcd2(a kernel is owned by tests that reach it indirectly) and is not this PR's. Validation is therefore not fully green.625dc00bab; both rerun on this head.