Skip to content

feat: Media view — inline images and video via herdr's graphics socket - #147

Open
szcharlesji wants to merge 8 commits into
smarzban:mainfrom
szcharlesji:feat/kitty-graphics
Open

feat: Media view — inline images and video via herdr's graphics socket#147
szcharlesji wants to merge 8 commits into
smarzban:mainfrom
szcharlesji:feat/kitty-graphics

Conversation

@szcharlesji

Copy link
Copy Markdown

Summary

Adds a Media view: images and video rendered inline in the content pane, through herdr's
documented pane.graphics.* socket API.

Selecting a media file shows the picture itself, with a caption above it
([image: 3008×1546 · PNG · 8-bit RGBA · 655 KiB]). Video adds playback — p play/pause, {/}
seek ±5s, 0 restart — plus a click/drag-to-seek progress bar.

The transport is what makes this cheap in spec terms: image bytes travel base64 inside JSON, so
no escape sequence is ever written to stdout. AC-27's neutralizer, tests/render_escape.rs,
SECURITY.md's "a hostile file cannot drive the terminal", and ARCHITECTURE.md trust boundary #1
are all untouched. A raw kitty-APC implementation would have needed a carve-out to every one of
them. Placement is data (a cell rect), so ratatui's differential redraw and the image never fight
over the cursor.

No new Cargo dependencies — base64 is ~20 lines, per the minimal-deps house style. ffmpeg /
ffprobe are optional runtime dependencies, like glow / delta / bat; without them media
degrades to a caption plus a notice.

Changes

  • src/graphics.rs (new) — the herdr graphics socket client. One request per connection
    (verified: the server closes after responding), hand-rolled base64, and a worker thread with a
    last-wins collapse. The thread is not optional: a set round-trip measures ~150 ms, so calling it
    from the event loop would stall input. The collapse is sound because GraphicsCommand carries
    absolute state (Show/Hide), not deltas.
  • src/media.rs, src/media/player.rs (new) — pure media decisions (classification, PNG header
    parsing, aspect-preserving placement, pixel budgets) and the ffmpeg frame decoder with a bounded,
    drop-oldest queue.
  • ViewMode::Media — chosen automatically, and it outranks "changed". A diff of compressed
    binary is noise, and the picture, which is the only thing worth looking at, is unreachable; so an
    edited image still shows the image, and media offers no diff views in the v cycle. This
    deliberately extends AC-9 rather than weakening it — that rule exists so an edited file shows what
    you edited, which for binary media a diff cannot do. Media did not exist when it was written.
  • Quality ladder — an image larger than the pane is resampled to the pane's own pixel box with
    lanczos first (those pixels can never be shown, so nothing visible is lost), and only if it
    still exceeds the host's cap does a ladder trade sharpness away.
  • Config: image, video, media_max_kib. New intents media_play_pause (p),
    media_seek_back/media_seek_forward ({/}), media_restart (0). Space was unavailable —
    it is already page_down.
  • Unrelated fix, included because it surfaced during testing: opening the viewer while a viewer
    pane is focused rooted the new one at the plugin's own install directory. herdr launches a plugin
    pane from the plugin root (its manifest command is relative), so a viewer pane's cwd is
    ~/.config/herdr/plugins/github/herdr-file-viewer-… — and both focused_pane_cwd and
    workspace_cwd are derived from the focused pane, so the launch context offered nothing else. The
    viewer now recognises its own install directory and asks herdr for the workspace's other panes.
    Happy to split this out if you'd prefer it separately.

Measured host limits

These drove the design, and are worth knowing independently of this PR (all probed against a live
herdr 0.8.0, protocol 19):

  • Hard cap: exactly 512 KiB of decoded image bytes, any format — 511.4 KiB accepted, 513.5 KiB
    rejected with image_too_large. Past ~1 MiB of base64 the server closes the connection with no
    response at all; the client treats that as "too large" rather than panicking.

  • Throughput plateaus at ~7–8 fps for any non-trivial frame (~120 ms fixed per call, dominated
    by herdr's full client-frame re-render, not payload size):

    PNG size 0.8 KiB 6.3 KiB 21 KiB 38 KiB 102 KiB 203 KiB 409 KiB
    fps 111 62 8.9 7.9 6.5 4.8 3.2

    So ~8 fps is a host ceiling, not a viewer limitation — documented as such in
    docs/renderers.md.

  • Nearest-neighbour beats smoothing for screenshots. Re-encoding a 655 KiB screenshot with the
    default bicubic filter produced 711 KiB — larger than the original: interpolation invents
    intermediate colours across the flat regions PNG compresses so well. At the same target width:
    neighbour 350 KiB, area 572, bilinear 589, lanczos 794, pal8 1096. Neighbour is also sharper
    on text, so it is the default; -sws_flags area is a one-word config change for photo-heavy use.

  • -re is load-bearing. Without it ffmpeg races to EOF — a 6.9 s clip emitted its entire frame
    stream in 0.377 s — so the drop-oldest queue discarded nearly every frame and the decoder
    finished before playback began.

Things I'd flag to a reviewer

  • Scope. Video playback in a read-only file viewer is a stretch against the constitution's
    YAGNI principle, and I'd rather say so than have you find it. It stays inside the others:
    read-only, delegated rendering (ADR-0001 — nothing is decoded in-process), and good-plugin-citizen
    (documented socket, no escapes around the host).
  • One deliberate trust-boundary narrowing. Video decoding passes the file path to ffmpeg —
    you cannot -ss-seek a pipe. The path is the canonicalized in-root one, passed as its own argv
    element with no shell. Documented in ARCHITECTURE.md and docs/renderers.md.
  • No snapshot can prove an image appeared — ratatui's TestBackend is a text grid. The
    GraphicsSink / GraphicsHost recorders are the automated oracle; actual pixels needed manual
    verification in a real herdr pane.
  • The graphics path is unreachable in CI (no herdr, no socket), so its tests are hermetic by
    construction. I'd rather state that plainly than imply CI covers it.
  • The version is deliberately not bumped. Cargo.toml, Cargo.lock, and herdr-plugin.toml
    stay at 1.15.0 and the changelog entry sits under ## [Unreleased] — batching and release timing
    are yours to decide.

Test plan

  • cargo test is green — 1409 tests
  • cargo fmt --all --check, cargo clippy --all-targets -- -D warnings, cargo audit pass
  • Docs updated in this PR (CHANGELOG + docs/renderers.md, docs/keys.md,
    docs/configuration.md, docs/usage.md, ARCHITECTURE.md, config.example.toml)

Manually verified in a real herdr pane on macOS (Ghostty, retina cell metrics 20×41): PNG / JPEG /
video render inline; over-cap images resample and still display; playback runs to the end of the
clip and stops cleanly with no orphaned ffmpeg; pausing holds the current frame; seeking while
paused shows that frame without starting playback; the progress bar tracks, freezes on pause, and
seeks on click/drag; leaving the file and quitting both clear the image.

Not verified: Linux and Windows behaviour of the graphics path (no herdr graphics host available to
me there) — the code degrades to the caption plus a notice wherever the socket or ffmpeg is absent.

Related

Two self-contained modules, with no callers yet.

`graphics` talks to herdr's documented `pane.graphics.*` socket API. Images
travel base64 inside JSON, so no escape sequence is ever written to stdout and
the AC-27 neutralizer, its pinned tests, and trust boundary smarzban#1 all stay intact.
Calls run on a worker thread because a round-trip measures ~150 ms against a
live host, and the worker collapses its backlog last-wins -- sound because a
GraphicsCommand carries absolute state rather than a delta. Base64 is
hand-rolled to avoid a new dependency, per the minimal-deps house style.

`media` holds the pure decisions: extension classification, PNG header parsing
(dimensions and colour, no decoder), aspect-preserving placement, and the pixel
budgets. `media::player` adds the ffmpeg frame decoder and its bounded,
drop-oldest queue.

MAX_IMAGE_BYTES is measured, not guessed: bisection against herdr 0.8.0 put the
limit at exactly 512 KiB of decoded image data, and past ~1 MiB of base64 the
server closes the connection without answering at all.
Adds `ViewMode::Media`, chosen automatically for a media file and joining the
Tab cycle, plus the machinery behind it.

The render worker produces the payload off the input thread: a PNG that already
fits is sent byte-for-byte, anything larger is resampled to the pane's own pixel
box with lanczos, and only if it still exceeds the host's cap does a quality
ladder trade sharpness away. That ordering matters -- resampling to the display
size costs nothing visible, whereas shrinking by a byte ratio degrades a picture
the pane could have shown in full.

The controller compares a desired placement against what the host is showing and
issues clear/set on a difference, so correctness for selection, mode, scroll,
resize, zoom, and overlays falls out of one comparison rather than a directory of
call sites. Playback is paced by the run loop's tick and yields the surface to
the player while it is playing, so the still and the frames cannot fight.

Placement uses each source's NATURAL size -- the video's own resolution from
ffprobe, not the poster frame's -- because frames are decoded small to fit the
byte cap and `fit` never upscales. The caption occupies the content box's top
row and the picture is placed below it.

Three defaults are measured rather than assumed: nearest-neighbour scaling (a
smoothing filter made a 655 KiB screenshot re-encode to 711 KiB, larger than the
original), `-re` on the decoder (without it ffmpeg emits a 6.9s clip's frames in
0.377s and the queue discards nearly all of them), and `-frames:v 1` placed
before the output URL, where ffmpeg actually honours it.

New config keys `image`, `video`, `media_max_kib`; new intents media_play_pause
(`p`), media_seek_back/forward (`{`/`}`), media_restart (`0`). `Space` was
unavailable -- it is already `page_down`.
Filed under [Unreleased]; the release version is the maintainer's call, so
Cargo.toml, Cargo.lock, and herdr-plugin.toml are untouched at 1.15.0.
The bar occupies the content pane's last row for a video of known duration --
`▶ 0:03 / 0:07 ━━━━━━╸────────` -- and a click or drag along it seeks. Its
position is wall-clock derived rather than counted in displayed frames: `-re`
paces the decoder to real time, but the host ceiling means we show fewer frames
than it emits, so counting them would make the bar drift slow. The label is
built by one shared function that the Presenter draws and the Controller
measures to locate the track, because computing it twice would mis-map every
drag without looking obviously broken.

Two placement bugs fall out of the same row accounting, so they land here:

Playback passed the raw content box while the still passed the reserved rect, so
a playing video painted over its own caption while a paused one looked fine.
Both now go through `media_cell_rect`, which reserves the caption row and -- for
a video -- the bar row, and which the decoder also sizes to.

Pausing handed the surface back to `sync_media`, which re-sent the poster still,
so pausing jumped to frame 0 instead of holding the frame you paused on. The
player keeps the surface once it has shown a frame, not merely while playing.
Adds the progress bar and click/drag-to-seek to the keys and renderers pages,
and notes that pausing holds the current frame. Changelog entry stays under
[Unreleased].
Three faults conspired to make a paused seek show nothing.

Seeking defaulted to "playing" when no player existed yet -- the state right
after selecting a video, when the poster still is what is on screen -- so
scrubbing the bar or pressing {/} silently STARTED playback. `p` is the only
thing that should start a video.

A paused tick pulled no frames at all, so even once the seek was correct the
frame the decoder produced a moment later was never collected. A paused player
now claims the single frame a seek owes it.

And the decoder runs on to EOF in the background after a seek (it is `-re`
paced, so seconds later). Treating that as "playback ended" dropped the player
and with it the position, so the bar fell back to 0:00 and the next `p`
restarted from the beginning. Only a PLAYING video has ended; a paused one keeps
its player and respawns the decoder when you resume.
Opening the viewer while a viewer pane is focused showed the plugin's own source
instead of the user's project.

herdr launches a plugin pane from the plugin root -- the manifest's pane command
is relative -- so a viewer pane's cwd IS the install directory. Both
`focused_pane_cwd` and `workspace_cwd` are derived from the focused pane, so when
that pane is a viewer the entire launch context offers nothing but
`~/.config/herdr/plugins/github/herdr-file-viewer-...`, and the fallback chain
had nothing better to fall back to.

The viewer now recognises the directory its own executable lives in and, when
that is all the context provides, asks herdr for the workspace's other panes and
roots at one of those. Scoped to the same workspace, since another workspace's
pane is different work and would look plausible while being wrong. Best-effort
throughout: no herdr, a failed query, or no sibling leaves the context exactly as
parsed.

Verified against herdr 0.8.0: `herdr pane list` emits JSON by default and has no
`--json` flag (passing one prints "unknown option" and the parse silently yields
nothing); `--workspace <id>` does the host-side scoping.
A changed image or video opened in the diff view, where delta rendered a diff of
compressed binary -- noise -- and the one thing worth looking at, the picture,
was unreachable. Media now wins over AC-9's changed-wins rule, and media files
offer no diff views in the `v` cycle at all: it is `[Media, SyntaxContent]`.

Nothing is really lost. The tree still marks the file changed, and for a
text-based format like SVG the second cycle step is the actual source.

This deliberately extends AC-9 rather than weakening it: the rule exists so an
edited file shows what you edited, which for binary media the diff cannot do.
Media did not exist when that criterion was written.
@szcharlesji

Copy link
Copy Markdown
Author
Screen.Recording.2026-08-09.at.19.07.51-under-10mb.mp4

@szcharlesji
szcharlesji marked this pull request as ready for review August 9, 2026 23:20
@szcharlesji
szcharlesji requested a review from smarzban as a code owner August 9, 2026 23:20
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