Skip to content

fix: encode stream-video frames once, into the container they claim - #130

Closed
onevcat wants to merge 1 commit into
mainfrom
fix/minimal-frame-transcode
Closed

onevcat wants to merge 1 commit into
mainfrom
fix/minimal-frame-transcode

Conversation

@onevcat

@onevcat onevcat commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Summary

stream-video's screenshot-backed formats emitted PNG payloads under an image/jpeg header at the default settings. Fixes it by having each format declare the container its consumers can decode, and encoding the capture into that container exactly once.

Supersedes #94, which diagnosed the bug correctly — thanks @SunsetWan. Its MJPEG frame-parsing test helper is carried over here.

Root cause

Both platforms shared a fast path that treated --scale 1.0 / --quality 80 as an unconditional passthrough, while every MJPEG frame header hardcoded image/jpeg. iOS screenshots and Android screencap -p are both PNG, so the default stream was mislabelled PNG.

The capture was PNG only because we asked for PNG: captureScreenshotData hardcoded format: .png. Producing JPEG from that meant a PNG encode, a decode, and a re-encode — three codec passes to change a label.

What changed

Each format names its container:

--format frames why
mjpeg JPEG (--quality) ffmpeg's mpjpeg demuxer and the IP-camera clients that read multipart/x-mixed-replace reject any other container
ffmpeg PNG, lossless the pipeline README documents (-f image2pipe) sniffs the container
raw PNG, lossless frames are self-delimiting via a 4-byte length prefix
  • iOS: SimulatorFrameSource reads the framebuffer as a CGImage; each sink encodes once into its container. This keeps --quality honoured — idb's jpegImageData() passes nil properties, so reusing takeScreenshot(format: .jpeg) would have silently dropped it.
  • Android: screencap's PNG now reaches raw/ffmpeg with no transcode at all. Only mjpeg pays an encode, since screencap has no JPEG option.
  • record-video's iOS screenshot fallback gets its CGImage straight from the framebuffer, dropping a PNG encode-then-decode per frame on the one capture path with no fps headroom to spare.
  • The run banner reports the frame container (Frames: jpeg q80) instead of a --quality that never applied to the PNG formats.

Verification

  • make build, make test — 1389 tests pass.
  • Live on a booted iPhone 17 Pro (iOS 27.0), all three formats:
    • mjpeg — Content-Type: image/jpeg, Content-Length matches the payload, payload starts FF D8. 455 KB/frame, where the old mislabelled PNG was 3.5 MB.
    • ffmpeg — opens with a PNG frame; fed to the exact pipeline in the README (ffmpeg -f image2pipe), 26 frames demuxed into 1206x2622 H.264.
    • raw — length prefix describes the frame, payload starts 89 50 4E 47.
    • mjpeg output also demuxes through ffmpeg -f mpjpeg (24 frames) once our HTTP status line is stripped — see the follow-up issue below.
  • Unit coverage for the container contract: passthrough only for a matching lossless container, JPEG always re-encoded so --quality is honoured, mimeType asserted against the bytes each container actually produces.
  • E2E assertions upgraded from "does stdout contain the string image/jpeg" to parsing the frame and checking MIME, Content-Length and magic bytes — the weak assertion is why this shipped.

Android E2E not run: no emulator reachable in this environment.

Follow-ups (not in this PR)

  • Our MJPEG stream opens with HTTP/1.1 200 OK, which ffmpeg -f mpjpeg cannot parse; stripping it makes the stream demux cleanly. Filed separately.
  • iOS stream-video has no native H.264 format, unlike Android's screenrecord passthrough. record-video already drives FBVideoStreamConfiguration in H.264 mode, so the plumbing exists. Filed separately.

Both platforms' screenshot-backed stream formats treated the default
`--scale 1.0` / `--quality 80` as an unconditional passthrough, while
every MJPEG frame header hardcoded `image/jpeg`. At default settings that
handed strict MJPEG clients a PNG payload under a JPEG MIME type.

Relabelling the bytes is the wrong fix: the capture is PNG only because
we asked for PNG. `captureScreenshotData` hardcoded `format: .png`, so
producing JPEG meant a PNG encode, a decode, and a re-encode — three
codec passes to change a label.

Each format now declares the container its consumers can actually
decode. `mjpeg` carries JPEG, because ffmpeg's `mpjpeg` demuxer and the
IP-camera clients that read `multipart/x-mixed-replace` reject anything
else. `raw` and `ffmpeg` carry the capture's lossless PNG untouched:
`raw` delimits frames with its own length prefix, and the `ffmpeg`
pipeline README documents sniffs the container via `-f image2pipe`
(verified against a live stream — 26 PNG frames demuxed and encoded).

On iOS, `SimulatorFrameSource` reads the framebuffer as a CGImage and
each sink encodes once into the container it needs, so `--quality` stays
honoured (idb's `jpegImageData()` passes nil properties and would have
silently dropped it). The recorder now gets its CGImage directly, which
also removes the PNG round-trip from the `record-video` screenshot
fallback — the one capture path with no fps headroom to spare. On
Android, `screencap`'s PNG reaches `raw`/`ffmpeg` with no transcode at
all, and only `mjpeg` pays an encode.

The run banner reports the frame container (`Frames: jpeg q80`) instead
of a `--quality` that never applied to the PNG formats, and `--quality`'s
help says which formats it affects.

Live-verified on a booted iPhone 17 Pro (iOS 27.0): mjpeg frames carry
JPEG under a matching header at 455 KB/frame, where the old PNG payload
was 3.5 MB.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: onevcat <onevcat@gmail.com>
@onevcat

onevcat commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

Follow-ups filed: #131 (MJPEG framing is unparseable by ffmpeg -f mpjpeg) and #132 (iOS stream-video has no native H.264 format).

@onevcat

onevcat commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

Closing in favour of a different approach — see #132.

The measurements that settled it, all on the same booted iPhone 17 Pro / iOS 27.0 over 6-second captures:

path frames effective fps bytes
screenshot loop (mjpeg) 24 4.1 11.2 MB
screenshot loop (PNG) 26 4.5 92.8 MB
native h264 Annex B 144 24.0 1.33 MB

Native H.264 streaming through createStream is ~5.9x the frame rate at 1/8th the bytes of mjpeg, and it consumes cleanly (ffmpeg -f h264 remuxes it to a 1206x2622 MP4, SIGTERM exits 0). It reuses the plumbing streamBGRA already has.

Given that, stream-video is moving to H.264 as its single capture path, and mjpeg/raw/ffmpeg are being deprecated — so this PR would have hardened a path that is on its way out. The frame-container bug it fixes is real, but it stops mattering once the formats carrying it are gone.

Two pieces of this branch are worth keeping and will be carried into the follow-up work where they still apply:

  • the record-video screenshot-fallback fix (frames no longer round-trip through PNG encode-then-decode) — that fallback survives H.264 becoming the primary path
  • Tests/MJPEGTestSupport.swift, the multipart frame parser

Branch stays at fix/minimal-frame-transcode if any of it needs to be revived.

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.

1 participant