From a9314f9aea5d8d947726ac34391c7dfd0b2176d7 Mon Sep 17 00:00:00 2001 From: Charles Ji Date: Sun, 9 Aug 2026 18:02:37 -0400 Subject: [PATCH 1/8] feat(media): add the herdr graphics seam and pure media helpers 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 #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. --- src/graphics.rs | 757 ++++++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 2 + src/media.rs | 428 +++++++++++++++++++++++++ src/media/player.rs | 325 +++++++++++++++++++ 4 files changed, 1512 insertions(+) create mode 100644 src/graphics.rs create mode 100644 src/media.rs create mode 100644 src/media/player.rs diff --git a/src/graphics.rs b/src/graphics.rs new file mode 100644 index 00000000..f7d5a175 --- /dev/null +++ b/src/graphics.rs @@ -0,0 +1,757 @@ +//! Graphics Host — inline images in the content pane, via herdr's documented socket API. +//! +//! The viewer never writes graphics escape sequences to its own stdout. Instead it asks the +//! host to place an image, over the same unix socket herdr's own CLI uses +//! (`pane.graphics.set` / `.clear` / `.info`). Two consequences worth stating plainly: +//! +//! - **The AC-27 escape-neutralizer is untouched.** Image bytes travel base64-encoded inside a +//! JSON request; no `ESC` byte is ever emitted. A hostile file still cannot drive the terminal +//! (`SECURITY.md`), and [`crate::render::to_text`] keeps sanitizing every byte that becomes text. +//! - **Placement is data, not cursor choreography.** We hand herdr a cell rect, so ratatui's +//! differential redraw and the image never fight over the cursor. +//! +//! Measured against herdr 0.8.0 (protocol 19) — see [`MAX_IMAGE_BYTES`] and [`GraphicsWorker`] +//! for the two numbers that shape everything here. + +use std::io; +use std::path::PathBuf; +use std::sync::mpsc; +use std::time::Duration; + +/// The largest image herdr accepts, in **decoded** bytes, for any format. +/// +/// Measured by bisection against a live herdr 0.8.0: 511.4 KiB was accepted and 513.5 KiB was +/// rejected with `{"code": "image_too_large"}`, identically for `png` and `rgb`. Beyond roughly +/// 1 MiB of base64 the server stops answering and closes the connection outright, which surfaces +/// here as [`GraphicsError::Transport`] rather than a clean error code — so callers must treat a +/// transport failure as "too big / gone" and degrade, never panic. +pub const MAX_IMAGE_BYTES: usize = 512 * 1024; + +/// How long a single socket round-trip may take before we give up on it. +/// +/// Generous on purpose: the measured worst case for a legal payload was ~310 ms, and this runs on +/// the graphics worker thread where a stall costs nothing but a late image. Bounding it at all is +/// what matters — an unbounded read would wedge the worker for good if herdr stopped answering. +const CALL_TIMEOUT: Duration = Duration::from_secs(2); + +/// The pixel size of one terminal cell, from `pane.graphics.info`. +/// +/// Needed to turn a ratatui [`Rect`](ratatui::layout::Rect) (in cells) into a pixel budget for the +/// image. Retina terminals report large values — Ghostty reported 20×41 on the probed machine. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct CellMetrics { + pub cell_width_px: u32, + pub cell_height_px: u32, +} + +/// Where the image sits, in terminal cells, relative to the pane viewport. +/// +/// `viewport_col`/`viewport_row` are signed so an image may be scrolled partly off the top or +/// left edge without the caller having to clamp it. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct Placement { + pub grid_cols: u32, + pub grid_rows: u32, + pub viewport_col: i32, + pub viewport_row: i32, +} + +/// The wire formats herdr accepts. +/// +/// Only [`Format::Png`] is used. `rgb`/`rgba` are also accepted by the host but measured *slower* +/// at every size (the payload is simply larger, and herdr's decode was never the bottleneck), and +/// they hit the same [`MAX_IMAGE_BYTES`] cap far sooner — a 420×420 raw `rgb` frame is already +/// over it. The variant list mirrors the host's enum so a future need is a one-line change. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Format { + Png, +} + +impl Format { + fn as_str(self) -> &'static str { + match self { + Format::Png => "png", + } + } +} + +/// One image ready to hand to the host: the encoded bytes plus where to put them. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Frame { + pub format: Format, + pub width: u32, + pub height: u32, + pub data: Vec, + pub placement: Placement, +} + +/// Why a graphics call could not be completed. Every variant is a "degrade gracefully" signal — +/// the caller shows the text placeholder and a notice, and the viewer carries on. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum GraphicsError { + /// No host to talk to: not running under herdr, or the pane id / socket path is unknown. + /// Distinguished from the others because it is the *expected* state outside herdr and so + /// earns a gentler notice than a genuine failure. + Unavailable, + /// The host rejected the image for exceeding [`MAX_IMAGE_BYTES`]. + TooLarge, + /// The host answered with an error object (e.g. `pane_not_found`). + Host(String), + /// The socket could not be reached, timed out, or closed mid-request. + Transport(String), +} + +impl std::fmt::Display for GraphicsError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + GraphicsError::Unavailable => write!(f, "no herdr graphics host"), + GraphicsError::TooLarge => write!(f, "image too large for the host"), + GraphicsError::Host(m) => write!(f, "host error: {m}"), + GraphicsError::Transport(m) => write!(f, "transport error: {m}"), + } + } +} + +// --------------------------------------------------------------------------- +// The seam the rest of the app depends on +// --------------------------------------------------------------------------- + +/// Synchronous access to the host's graphics surface. +/// +/// Behind a trait so tests substitute a recorder and never open a socket. Implementations block +/// for the duration of a round-trip (~150 ms for a real image — see [`GraphicsWorker`]), so this +/// is never called from the UI thread directly. +pub trait GraphicsHost: Send { + fn info(&self) -> Result; + fn set(&self, frame: &Frame) -> Result<(), GraphicsError>; + fn clear(&self) -> Result<(), GraphicsError>; +} + +/// What the controller wants on screen right now. +/// +/// Each command is **absolute state**, never a delta — `Show` means "this image, here", `Hide` +/// means "nothing". That is precisely what makes the worker's last-wins collapse +/// ([`GraphicsWorker`]) correct rather than merely convenient. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum GraphicsCommand { + Show(Box), + Hide, +} + +/// A non-blocking outbox for [`GraphicsCommand`]s. +/// +/// The controller holds one of these and never waits: sending is fire-and-forget so a 150 ms +/// socket round-trip can't land on the input thread. Tests use a recorder that captures commands +/// synchronously. [`close`](Self::close) exists so teardown can *block* until the desired end +/// state ("nothing") has actually reached the host before the process exits — the worker joins +/// its thread, the null/no-host sink is already done. +pub trait GraphicsSink: Send { + fn send(&self, command: GraphicsCommand); + /// Flush everything that has been sent and release the sink. The default is appropriate for a + /// sink that has nothing to drain; [`GraphicsWorker`] overrides it to join its thread. + fn close(&mut self) {} +} + +// --------------------------------------------------------------------------- +// The worker: keeps slow socket calls off the UI thread +// --------------------------------------------------------------------------- + +/// Owns the one long-lived thread that talks to the host. +/// +/// **Why a thread at all.** A `pane.graphics.set` round-trip was measured at ~120–155 ms for any +/// non-trivial image and plateaus there (the cost is herdr's full client-frame re-render, not our +/// payload size). Calling that from the event loop would stall input for a sixth of a second per +/// image and make video playback impossible, so every call is dispatched here instead. +/// +/// **Why last-wins collapsing.** The worker drains its whole backlog and executes only the final +/// command, exactly as the render worker collapses stale render jobs. This is sound *because* +/// [`GraphicsCommand`] carries absolute state: if three frames queue up while one is in flight, +/// showing only the newest is not an approximation, it is the correct result. Without it a slow +/// host would build an unbounded queue of frames nobody will ever see. +pub struct GraphicsWorker { + tx: Option>, + handle: Option>, +} + +impl GraphicsWorker { + /// Spawn the worker around `host`. The thread lives until the sender is dropped (see + /// [`GraphicsSink::close`] — teardown calls it so the final `Hide` is flushed, not lost in a + /// fire-and-forget race with process exit). + pub fn spawn(host: Box) -> Self { + let (tx, rx) = mpsc::channel::(); + let handle = std::thread::spawn(move || { + while let Ok(mut command) = rx.recv() { + // Collapse the backlog: only the newest command describes the desired state. + while let Ok(newer) = rx.try_recv() { + command = newer; + } + let _ = match command { + GraphicsCommand::Show(frame) => host.set(&frame), + GraphicsCommand::Hide => host.clear(), + }; + } + }); + Self { + tx: Some(tx), + handle: Some(handle), + } + } +} + +impl GraphicsSink for GraphicsWorker { + fn send(&self, command: GraphicsCommand) { + // A dead worker is not worth surfacing: the image simply doesn't appear, and the text + // placeholder underneath is still correct. + let _ = self.tx.as_ref().map(|tx| tx.send(command)); + } + + fn close(&mut self) { + // Teardown must leave the pane clean: the end state is "nothing". Send a final Hide so + // the desired state is flushed through the same last-wins collapse (a queued Show after + // us would be wrong anyway — this is the exit path), then drop the sender so the worker's + // `recv` returns `Err(Disconnected)` and it exits, and join so the Hide has actually been + // delivered to the host before we return to `run`'s teardown. + let _ = self.tx.take().map(|tx| tx.send(GraphicsCommand::Hide)); + let _ = self.handle.take().map(|h| h.join()); + } +} + +/// A sink that drops everything, for when there is no host (outside herdr, or on Windows). +pub struct NullSink; + +impl GraphicsSink for NullSink { + fn send(&self, _command: GraphicsCommand) {} +} + +// --------------------------------------------------------------------------- +// Pure protocol helpers — testable on every platform, no socket involved +// --------------------------------------------------------------------------- + +/// Resolve this process's own pane id. +/// +/// herdr sets `HERDR_PANE_ID` for every pane process it launches, which is authoritative for +/// *our* pane — unlike `herdr pane current`, which reports the pane the user is looking at and +/// may belong to someone else entirely. Taken as a parameter rather than read here so the +/// resolution logic is testable without touching the real environment. +pub fn pane_id_from_env(var: Option) -> Option { + var.filter(|id| !id.is_empty()) +} + +/// Build the JSON request line for `pane.graphics.set`. +/// +/// Separate from the transport so the exact wire shape is asserted in a unit test on every +/// platform. Verified against herdr 0.8.0 (protocol 19): +/// `{"id":…,"method":"pane.graphics.set","params":{pane_id, format, image_width, image_height, +/// data_base64, placement:{grid_cols, grid_rows, viewport_col, viewport_row}}}`. +pub fn set_request(pane_id: &str, frame: &Frame) -> String { + let params = serde_json::json!({ + "pane_id": pane_id, + "format": frame.format.as_str(), + "image_width": frame.width, + "image_height": frame.height, + "data_base64": encode_base64(&frame.data), + "placement": { + "grid_cols": frame.placement.grid_cols, + "grid_rows": frame.placement.grid_rows, + "viewport_col": frame.placement.viewport_col, + "viewport_row": frame.placement.viewport_row, + }, + }); + request_line("fv:set", "pane.graphics.set", params) +} + +/// Build the JSON request line for `pane.graphics.clear`. +pub fn clear_request(pane_id: &str) -> String { + request_line( + "fv:clear", + "pane.graphics.clear", + serde_json::json!({ "pane_id": pane_id }), + ) +} + +/// Build the JSON request line for `pane.graphics.info`. +pub fn info_request(pane_id: &str) -> String { + request_line( + "fv:info", + "pane.graphics.info", + serde_json::json!({ "pane_id": pane_id }), + ) +} + +/// The envelope every request shares: `{"id":…,"method":…,"params":…}` plus the trailing newline +/// that terminates a message on this socket. +fn request_line(id: &str, method: &str, params: serde_json::Value) -> String { + let request = serde_json::json!({ "id": id, "method": method, "params": params }); + format!("{request}\n") +} + +/// Interpret a response line: `{"result":…}` on success, `{"error":{"code","message"}}` otherwise. +/// +/// The `image_too_large` code is promoted to [`GraphicsError::TooLarge`] because callers act on +/// it (re-encode smaller) rather than merely reporting it. +pub fn parse_response(line: &str) -> Result { + let value: serde_json::Value = serde_json::from_str(line) + .map_err(|e| GraphicsError::Transport(format!("unparsable response: {e}")))?; + if let Some(result) = value.get("result") { + return Ok(result.clone()); + } + let code = value + .get("error") + .and_then(|e| e.get("code")) + .and_then(|c| c.as_str()) + .unwrap_or("unknown"); + if code == "image_too_large" { + return Err(GraphicsError::TooLarge); + } + let message = value + .get("error") + .and_then(|e| e.get("message")) + .and_then(|m| m.as_str()) + .unwrap_or(code); + Err(GraphicsError::Host(message.to_string())) +} + +/// Read [`CellMetrics`] out of a `pane_graphics_info` result. +pub fn parse_cell_metrics(result: &serde_json::Value) -> Result { + let field = |name: &str| { + result + .get(name) + .and_then(|v| v.as_u64()) + .filter(|v| *v > 0) + .ok_or_else(|| GraphicsError::Host(format!("missing {name}"))) + }; + Ok(CellMetrics { + cell_width_px: field("cell_width_px")? as u32, + cell_height_px: field("cell_height_px")? as u32, + }) +} + +/// Standard base64 with padding. +/// +/// Hand-rolled rather than pulled in as a crate: it is twenty lines, and the house style treats a +/// new dependency as a deliberate decision (see `AGENTS.md`, "Minimal-deps house style") — the +/// test suite rolls its own temp dirs for the same reason. +pub fn encode_base64(bytes: &[u8]) -> String { + const ALPHABET: &[u8; 64] = b"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; + let mut out = String::with_capacity(bytes.len().div_ceil(3) * 4); + for chunk in bytes.chunks(3) { + let b0 = chunk[0] as u32; + let b1 = *chunk.get(1).unwrap_or(&0) as u32; + let b2 = *chunk.get(2).unwrap_or(&0) as u32; + let triple = (b0 << 16) | (b1 << 8) | b2; + out.push(ALPHABET[(triple >> 18) as usize & 0x3f] as char); + out.push(ALPHABET[(triple >> 12) as usize & 0x3f] as char); + out.push(if chunk.len() > 1 { + ALPHABET[(triple >> 6) as usize & 0x3f] as char + } else { + '=' + }); + out.push(if chunk.len() > 2 { + ALPHABET[triple as usize & 0x3f] as char + } else { + '=' + }); + } + out +} + +// --------------------------------------------------------------------------- +// LiveGraphics — the real socket client (unix only) +// --------------------------------------------------------------------------- + +/// The real [`GraphicsHost`]: one unix-socket round-trip per call. +/// +/// **One request per connection.** Verified against herdr 0.8.0: the server writes its response +/// and closes the stream, so a second write on the same connection fails with `EPIPE`. We +/// therefore reconnect for every call rather than holding a session open. +pub struct LiveGraphics { + socket: PathBuf, + pane_id: String, +} + +impl LiveGraphics { + /// Build a client from the environment, or `None` when this process has no host to talk to + /// (not launched by herdr, or on a platform without unix sockets). + /// + /// Taking both values as parameters keeps the availability rule testable; [`from_env`] is the + /// thin wrapper that reads the real environment. + /// + /// [`from_env`]: LiveGraphics::from_env + pub fn new(socket_path: Option, pane_id: Option) -> Option { + if !cfg!(unix) { + return None; + } + let socket = socket_path.filter(|p| !p.is_empty())?; + let pane_id = pane_id_from_env(pane_id)?; + Some(Self { + socket: PathBuf::from(socket), + pane_id, + }) + } + + /// [`LiveGraphics::new`] against the real process environment. + pub fn from_env() -> Option { + Self::new( + std::env::var("HERDR_SOCKET_PATH").ok(), + std::env::var("HERDR_PANE_ID").ok(), + ) + } + + #[cfg(unix)] + fn round_trip(&self, request: &str) -> Result { + use std::io::{BufRead, BufReader, Write}; + use std::os::unix::net::UnixStream; + + let transport = |e: io::Error| GraphicsError::Transport(e.to_string()); + let mut stream = UnixStream::connect(&self.socket).map_err(transport)?; + stream + .set_read_timeout(Some(CALL_TIMEOUT)) + .and_then(|()| stream.set_write_timeout(Some(CALL_TIMEOUT))) + .map_err(transport)?; + stream + .write_all(request.as_bytes()) + .and_then(|()| stream.flush()) + .map_err(transport)?; + + let mut line = String::new(); + // An empty read means the server closed without answering — what an over-sized payload + // does past roughly 1 MiB of base64. Report it as the size problem it almost always is. + match BufReader::new(&stream).read_line(&mut line) { + Ok(0) => Err(GraphicsError::Transport( + "host closed the connection".into(), + )), + Ok(_) => parse_response(line.trim_end()), + Err(e) => Err(transport(e)), + } + } + + #[cfg(not(unix))] + fn round_trip(&self, _request: &str) -> Result { + Err(GraphicsError::Unavailable) + } +} + +impl GraphicsHost for LiveGraphics { + fn info(&self) -> Result { + parse_cell_metrics(&self.round_trip(&info_request(&self.pane_id))?) + } + + fn set(&self, frame: &Frame) -> Result<(), GraphicsError> { + // Reject locally what the host would reject anyway: this saves shipping up to a megabyte + // of base64 across the socket only to be told `image_too_large`. + if frame.data.len() > MAX_IMAGE_BYTES { + return Err(GraphicsError::TooLarge); + } + self.round_trip(&set_request(&self.pane_id, frame)) + .map(|_| ()) + } + + fn clear(&self) -> Result<(), GraphicsError> { + self.round_trip(&clear_request(&self.pane_id)).map(|_| ()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::sync::{Arc, Mutex}; + + fn frame(data: Vec) -> Frame { + Frame { + format: Format::Png, + width: 4, + height: 2, + data, + placement: Placement { + grid_cols: 10, + grid_rows: 5, + viewport_col: 2, + viewport_row: -1, + }, + } + } + + // -- base64 ------------------------------------------------------------- + + #[test] + fn base64_matches_the_rfc4648_test_vectors() { + // The canonical vectors, so a hand-rolled encoder can't drift: padding at every residue. + for (input, expected) in [ + ("", ""), + ("f", "Zg=="), + ("fo", "Zm8="), + ("foo", "Zm9v"), + ("foob", "Zm9vYg=="), + ("fooba", "Zm9vYmE="), + ("foobar", "Zm9vYmFy"), + ] { + assert_eq!(encode_base64(input.as_bytes()), expected, "input {input:?}"); + } + } + + #[test] + fn base64_covers_the_whole_byte_range_including_the_high_indices() { + // 0xFB..0xFF exercise the '+' and '/' end of the alphabet, which a truncated table + // would silently get wrong for real (binary) PNG data. + let all: Vec = (0u8..=255).collect(); + let encoded = encode_base64(&all); + assert_eq!(encoded.len(), 344, "4 chars per 3 bytes, padded"); + assert!(encoded.starts_with("AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8g")); + // 240..=255 contains 0xF0..=0xFF: the 'w' onwards half of the alphabet plus '+' and '/'. + assert!(encoded.ends_with("8PHy8/T19vf4+fr7/P3+/w==")); + } + + // -- request shape ------------------------------------------------------ + + #[test] + fn set_request_matches_the_verified_wire_shape() { + // Pinned against herdr 0.8.0 (protocol 19), probed live during design. If herdr renames + // a field this fails here rather than silently painting nothing. + let line = set_request("wJ:p1", &frame(b"foo".to_vec())); + assert!(line.ends_with('\n'), "messages are newline-terminated"); + let v: serde_json::Value = serde_json::from_str(line.trim_end()).expect("valid JSON"); + assert_eq!(v["method"], "pane.graphics.set"); + assert_eq!(v["params"]["pane_id"], "wJ:p1"); + assert_eq!(v["params"]["format"], "png"); + assert_eq!(v["params"]["image_width"], 4); + assert_eq!(v["params"]["image_height"], 2); + assert_eq!(v["params"]["data_base64"], "Zm9v"); + assert_eq!(v["params"]["placement"]["grid_cols"], 10); + assert_eq!(v["params"]["placement"]["grid_rows"], 5); + assert_eq!(v["params"]["placement"]["viewport_col"], 2); + assert_eq!( + v["params"]["placement"]["viewport_row"], -1, + "a partly scrolled-off image sends a negative row rather than being clamped" + ); + assert!(v["id"].is_string(), "the envelope requires a string id"); + } + + #[test] + fn clear_and_info_requests_target_the_pane() { + for (line, method) in [ + (clear_request("wJ:p1"), "pane.graphics.clear"), + (info_request("wJ:p1"), "pane.graphics.info"), + ] { + let v: serde_json::Value = serde_json::from_str(line.trim_end()).expect("valid JSON"); + assert_eq!(v["method"], method); + assert_eq!(v["params"]["pane_id"], "wJ:p1"); + } + } + + // -- response parsing --------------------------------------------------- + + #[test] + fn parse_response_reads_a_result() { + let ok = parse_response(r#"{"id":"m","result":{"type":"ok"}}"#).expect("a result"); + assert_eq!(ok["type"], "ok"); + } + + #[test] + fn image_too_large_is_promoted_to_its_own_variant() { + // Callers re-encode smaller on this specific code, so it must not be lumped in with + // generic host errors. The code string is herdr's, observed live. + assert_eq!( + parse_response(r#"{"id":"m","error":{"code":"image_too_large","message":"…"}}"#), + Err(GraphicsError::TooLarge) + ); + } + + #[test] + fn other_host_errors_keep_their_message() { + assert_eq!( + parse_response(r#"{"id":"m","error":{"code":"pane_not_found","message":"pane x"}}"#), + Err(GraphicsError::Host("pane x".into())) + ); + } + + #[test] + fn malformed_json_is_a_transport_error_not_a_panic() { + assert!(matches!( + parse_response("not json at all"), + Err(GraphicsError::Transport(_)) + )); + } + + #[test] + fn cell_metrics_reject_missing_or_zero_fields() { + let good = serde_json::json!({"cell_width_px": 20, "cell_height_px": 41}); + assert_eq!( + parse_cell_metrics(&good), + Ok(CellMetrics { + cell_width_px: 20, + cell_height_px: 41 + }) + ); + // Zero would make the fit maths divide by zero, so it is rejected at the boundary. + for bad in [ + serde_json::json!({"cell_width_px": 0, "cell_height_px": 41}), + serde_json::json!({"cell_height_px": 41}), + ] { + assert!(matches!( + parse_cell_metrics(&bad), + Err(GraphicsError::Host(_)) + )); + } + } + + // -- availability ------------------------------------------------------- + + #[test] + fn a_client_needs_both_a_socket_and_a_pane_id() { + assert!(LiveGraphics::new(Some("/s".into()), Some("wJ:p1".into())).is_some() == cfg!(unix)); + for (socket, pane) in [ + (None, Some("wJ:p1".to_string())), + (Some("/s".to_string()), None), + (Some(String::new()), Some("wJ:p1".to_string())), + (Some("/s".to_string()), Some(String::new())), + ] { + assert!( + LiveGraphics::new(socket.clone(), pane.clone()).is_none(), + "empty or missing values must not produce a half-configured client: \ + socket={socket:?} pane={pane:?}" + ); + } + } + + #[test] + fn an_oversized_frame_is_rejected_without_touching_the_socket() { + // The socket path is deliberately nonexistent: if the size guard did not short-circuit, + // this would fail with Transport instead of TooLarge. + let Some(client) = + LiveGraphics::new(Some("/nonexistent/herdr.sock".into()), Some("p".into())) + else { + return; // non-unix: no client to test + }; + let big = frame(vec![0u8; MAX_IMAGE_BYTES + 1]); + assert_eq!(client.set(&big), Err(GraphicsError::TooLarge)); + } + + // -- worker collapsing -------------------------------------------------- + + #[derive(Default)] + struct Recorder { + calls: Arc>>, + gate: Option>>, + /// Signalled just before the worker parks on `gate`, so a test can wait for the first + /// `set` to have *started* without sleeping (a synchronous, observable tell). + arrived_tx: Option>, + } + + impl GraphicsHost for Recorder { + fn info(&self) -> Result { + Err(GraphicsError::Unavailable) + } + fn set(&self, frame: &Frame) -> Result<(), GraphicsError> { + self.notify_arrived(); + // Holding the gate models a slow host, so a backlog provably builds up behind the + // first call rather than us hoping the scheduler produces one. + let _held = self.gate.as_ref().map(|g| g.lock().unwrap()); + self.calls + .lock() + .unwrap() + .push(format!("set:{}", frame.width)); + Ok(()) + } + fn clear(&self) -> Result<(), GraphicsError> { + let _held = self.gate.as_ref().map(|g| g.lock().unwrap()); + self.calls.lock().unwrap().push("clear".into()); + Ok(()) + } + } + + impl Recorder { + fn notify_arrived(&self) { + if let Some(tx) = &self.arrived_tx { + let _ = tx.send(()); + } + } + } + + #[test] + fn the_worker_collapses_a_backlog_to_the_newest_command() { + // Force the race rather than hope for it (AGENTS.md): the gate is held here, so the + // worker blocks inside the FIRST set while the next three commands queue behind it. + // Releasing it must yield exactly the first and the last — the first (`set:1`) ran + // because the worker had already started it when the backlog formed, then the queued + // 2/3/4 collapse to the newest (`set:4`). A timing-based version of this test would + // pass (or fail) vacuously depending on the scheduler. + let calls = Arc::new(Mutex::new(Vec::new())); + let gate = Arc::new(Mutex::new(())); + let held = gate.lock().unwrap(); + let (arrived_tx, arrived_rx) = mpsc::channel(); + + let worker = GraphicsWorker::spawn(Box::new(Recorder { + calls: Arc::clone(&calls), + gate: Some(Arc::clone(&gate)), + arrived_tx: Some(arrived_tx), + })); + let mut first = frame(vec![1]); + first.width = 1; + worker.send(GraphicsCommand::Show(Box::new(first))); + // Synchronous tell: this returns only once the worker has STARTED the first set and is + // parked on the gate, so the commands below are guaranteed to queue rather than race + // ahead of it. No sleep, no yield loop. + arrived_rx.recv().expect("worker must start the first set"); + + for width in [2u32, 3, 4] { + let mut f = frame(vec![1]); + f.width = width; + worker.send(GraphicsCommand::Show(Box::new(f))); + } + drop(held); + + // Bounded, not timed: wait for the terminal state to appear rather than measuring how + // long it took. A stuck worker fails by timing out the loop, not by a flaky margin. + let deadline = std::time::Instant::now() + Duration::from_secs(2); + loop { + if calls.lock().unwrap().last().is_some_and(|c| c == "set:4") { + break; + } + assert!( + std::time::Instant::now() < deadline, + "worker never reached the newest command: {:?}", + calls.lock().unwrap() + ); + std::thread::yield_now(); + } + assert_eq!( + *calls.lock().unwrap(), + vec!["set:1".to_string(), "set:4".to_string()], + "the first ran (already started when the backlog formed), then the queued 2/3/4 \ + collapse to the newest — widths 2 and 3 never appear" + ); + } + + #[test] + fn hide_is_absolute_state_so_it_survives_collapsing_after_a_show() { + // Show-then-Hide must end hidden. This is the property that makes last-wins correct. + let calls = Arc::new(Mutex::new(Vec::new())); + let worker = GraphicsWorker::spawn(Box::new(Recorder { + calls: Arc::clone(&calls), + gate: None, + arrived_tx: None, + })); + worker.send(GraphicsCommand::Show(Box::new(frame(vec![1])))); + worker.send(GraphicsCommand::Hide); + + let deadline = std::time::Instant::now() + Duration::from_secs(2); + loop { + if calls.lock().unwrap().last().is_some_and(|c| c == "clear") { + break; + } + assert!( + std::time::Instant::now() < deadline, + "worker never cleared: {:?}", + calls.lock().unwrap() + ); + std::thread::yield_now(); + } + } + + #[test] + fn the_null_sink_accepts_and_drops_everything() { + // Outside herdr this is the whole graphics path; it must never panic. + NullSink.send(GraphicsCommand::Show(Box::new(frame(vec![1])))); + NullSink.send(GraphicsCommand::Hide); + } +} diff --git a/src/lib.rs b/src/lib.rs index c6d6e9ba..b17340c5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -12,6 +12,7 @@ pub mod editor; pub mod finder; pub mod fuzzy; pub mod git; +pub mod graphics; pub mod help; pub mod herdr; pub mod highlight; @@ -21,6 +22,7 @@ pub mod infile; pub mod input; pub mod intent; pub mod launch; +pub mod media; pub mod open_target; pub mod opener; pub mod picker; diff --git a/src/media.rs b/src/media.rs new file mode 100644 index 00000000..3e4186de --- /dev/null +++ b/src/media.rs @@ -0,0 +1,428 @@ +//! Media — pure decisions about images and video, with no I/O. +//! +//! Everything here is a pure function over already-in-hand data (a file name, some bytes, the +//! pane geometry), so the whole module is unit-testable on every platform without a socket, a +//! graphics host, or an ffmpeg binary. The enclosing pipeline (see `graphics.rs`, the render +//! worker, and the controller's `media_shown` discipline) does the actual work. + +use crate::graphics::{CellMetrics, Placement}; +use ratatui::layout::Rect; + +/// Video playback: the ffmpeg decoder + bounded drop-oldest queue. +pub mod player; + +/// What kind of media a file is, from its extension. One view mode covers all three: only the +/// *payload production* differs (PNG bytes are sent as-is; other images convert; video decodes +/// managed frames). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum MediaKind { + /// A `.png` — sent natively, no conversion required. + Png, + /// Any other image that must be converted to PNG to reach the host. + Image, + /// A video: a still from frame 0 for the preview, then selectable playback. + Video, +} + +impl MediaKind { + /// Classify a path by its (case-insensitive) extension. Unknown extensions return `None` — + /// the file is not media at all. Modelled on `controller::is_markdown`. + pub fn from_path(path: &std::path::Path) -> Option { + let ext = path.extension()?.to_str()?.to_ascii_lowercase(); + match ext.as_str() { + "png" => Some(MediaKind::Png), + // The images herdr can host after a PNG conversion. jpg/jpeg, gif, webp, bmp, tiff, + // svg, avif, heic, ico. (A missing converter degrades to the placeholder + notice.) + "jpg" | "jpeg" | "gif" | "webp" | "bmp" | "tif" | "tiff" | "svg" | "avif" | "heic" + | "heif" | "ico" | "jxl" => Some(MediaKind::Image), + // The container formats ffmpeg can decode for playback. mp4, mkv, webm, mov, avi, + // m4v, mpg, mpeg, flv, wmv, ogv, ts, 3gp. + "mp4" | "mkv" | "webm" | "mov" | "avi" | "m4v" | "mpg" | "mpeg" | "flv" | "wmv" + | "ogv" | "ts" | "3gp" | "mts" | "m2ts" => Some(MediaKind::Video), + _ => None, + } + } + + /// A short human label for the text fallback line (`[image: …]` / `[video: …]`). + pub fn label(self) -> &'static str { + match self { + MediaKind::Png | MediaKind::Image => "image", + MediaKind::Video => "video", + } + } +} + +/// Parse the pixel dimensions of a PNG from its IHDR chunk. +/// +/// Returns `None` for anything that is not a well-formed PNG header: a file too short to hold +/// the 24-byte header, a wrong magic signature, a first chunk that is not `IHDR`, or a +/// non-positive dimension. Malformed input is `None`, never a panic — the caller then falls back +/// to converting via ffmpeg or showing the placeholder. +pub fn png_dimensions(bytes: &[u8]) -> Option<(u32, u32)> { + if bytes.len() < 24 { + return None; + } + const MAGIC: [u8; 8] = [0x89, b'P', b'N', b'G', 0x0d, 0x0a, 0x1a, 0x0a]; + if bytes[..8] != MAGIC { + return None; + } + if bytes[12..16] != *b"IHDR" { + return None; + } + let width = u32::from_be_bytes(bytes[16..20].try_into().ok()?); + let height = u32::from_be_bytes(bytes[20..24].try_into().ok()?); + (width > 0 && height > 0).then_some((width, height)) +} + +/// A PNG's colour description, read from the same IHDR chunk as its dimensions. +/// +/// Byte 24 is the bit depth and byte 25 the colour type, so this costs nothing beyond the header +/// read we already do — no decoder, no subprocess. Returns `None` for a header we cannot parse or +/// a colour type outside the PNG spec's five. +pub fn png_colour(bytes: &[u8]) -> Option { + if bytes.len() < 26 || png_dimensions(bytes).is_none() { + return None; + } + let depth = bytes[24]; + let kind = match bytes[25] { + 0 => "grey", + 2 => "RGB", + 3 => "indexed", + 4 => "grey+alpha", + 6 => "RGBA", + _ => return None, + }; + Some(format!("{depth}-bit {kind}")) +} + +/// Human-readable byte size, e.g. `655 KiB` / `1.4 MiB`. Used in the media info line. +pub fn human_size(bytes: u64) -> String { + const KIB: u64 = 1024; + const MIB: u64 = KIB * 1024; + match bytes { + b if b >= MIB => format!("{:.1} MiB", b as f64 / MIB as f64), + b if b >= KIB => format!("{} KiB", b / KIB), + b => format!("{b} B"), + } +} + +/// A duration in seconds as `m:ss` (or `h:mm:ss` past an hour), for the video info line. +pub fn human_duration(seconds: f64) -> String { + if !seconds.is_finite() || seconds < 0.0 { + return "?".to_string(); + } + let total = seconds.round() as u64; + let (h, m, s) = (total / 3600, (total % 3600) / 60, total % 60); + if h > 0 { + format!("{h}:{m:02}:{s:02}") + } else { + format!("{m}:{s:02}") + } +} + +/// The pixel size of a cell rectangle, the budget an image (or a video frame) must fit within. +/// +/// This is the number handed to ffmpeg's `scale` filter (via `force_original_aspect_ratio`-style +/// fit), and the denominator against which `fit` measures "does this image already fit?". +/// Clamped so the decoded size (4 bytes/px) stays under the host's [`MAX_IMAGE_BYTES`] cap — a +/// pane-sized budget at retina cell metrics would otherwise happily exceed what herdr accepts, and +/// every video frame would be rejected as `image_too_large`. Saturated at the pixel cap too, so an +/// absurd configured cell size still yields a finite budget. +pub fn frame_budget(cell_rect: Rect, cell_px: CellMetrics) -> (u32, u32) { + let w = cell_rect.width as u32 * cell_px.cell_width_px; + let h = cell_rect.height as u32 * cell_px.cell_height_px; + clamp_pixels_to_cap((w, h)) +} + +/// Shrink a pixel box until it plausibly encodes under the host's byte cap, preserving aspect. +/// +/// Factored out of [`frame_budget`] so the still preview and the playback decoder can be handed +/// the *same* number. They used to disagree — the still was hardcoded to 640x360 while playback +/// used the pane-derived budget — so a video visibly changed size the moment you pressed play. +pub fn clamp_pixels_to_cap(box_px: (u32, u32)) -> (u32, u32) { + let mut w = box_px.0 as u64; + let mut h = box_px.1 as u64; + // Pixel cap derived from the host's byte cap, then the box is fitted inside it preserving the + // pane's aspect (so neither dimension alone exceeds the budget). + // + // The divisor is bytes-per-pixel for the ENCODED frame, not a raw RGBA buffer. Measured: a + // 640x360 PNG video frame is ~84 KiB, i.e. ~0.36 B/px. The original 4 B/px assumed raw RGBA + // and capped frames at ~362x362 — needlessly soft, since we transmit PNG. 2 B/px keeps a + // ~5x margin over the measurement while roughly doubling each edge; an unusually busy frame + // that still overshoots is rejected by the host and skipped, which costs one frame of + // playback rather than correctness. + let max_px = crate::graphics::MAX_IMAGE_BYTES as u64 / 2; + if w.saturating_mul(h) > max_px { + let scale = (max_px as f64 / (w as f64 * h as f64)).sqrt(); + w = (w as f64 * scale).floor().max(1.0) as u64; + h = (h as f64 * scale).floor().max(1.0) as u64; + debug_assert!( + w.saturating_mul(h) <= max_px, + "budget clamp must satisfy the decoded cap" + ); + } + let max = u32::MAX as u64; + (w.min(max) as u32, h.min(max) as u32) +} + +/// The pixel size to re-encode an over-cap image at, so it lands under the host's byte cap. +/// +/// PNG size tracks pixel count closely for the screenshots and diagrams that actually appear in a +/// repo, so scaling both edges by `sqrt(cap / actual)` targets the cap directly. The extra 0.85 +/// leaves headroom, because resampling can *hurt* compression (it introduces intermediate colours +/// a flat-region screenshot didn't have). Callers re-measure the result and shrink again if the +/// estimate missed — this is a starting point, not a guarantee. +/// +/// Never upscales: an image already under the cap returns its own dimensions. +pub fn downscale_target(dims: (u32, u32), actual_bytes: usize, cap: usize) -> (u32, u32) { + if actual_bytes <= cap { + return dims; // already fits — the margin is for shrinking, not for shaving a fitting image + } + let ratio = (cap as f64 / actual_bytes.max(1) as f64).sqrt() * 0.85; + let scale = |v: u32| ((v as f64 * ratio).floor() as u32).max(1); + (scale(dims.0), scale(dims.1)) +} + +/// Aspect-preserving fit of an image into the pane's cell grid: as large as the content box allows. +/// +/// Returns the cell rectangle the image occupies, anchored at `cell_rect`'s origin (the content +/// pane's inner top-left) and grown until one axis touches the box. +/// +/// **Display size is deliberately independent of the byte budget.** [`frame_budget`] bounds how +/// many pixels we may *transmit*; the host then scales whatever it receives into the rect named +/// here, so a byte-limited image still fills the pane. Conflating the two is what made every +/// picture render postage-stamp sized — the ~512x256 transmission clamp was being used as the +/// display box. +/// +/// Still never upscales past the image's natural pixel size: a 16x16 favicon stays a favicon +/// rather than being blown up into a blurry wall. +pub fn fit(image_px: (u32, u32), cell_px: CellMetrics, cell_rect: Rect) -> Placement { + let box_w = cell_rect.width as f64 * cell_px.cell_width_px.max(1) as f64; + let box_h = cell_rect.height as f64 * cell_px.cell_height_px.max(1) as f64; + let scale = (box_w / image_px.0.max(1) as f64) + .min(box_h / image_px.1.max(1) as f64) + .min(1.0); // never upscale: a small icon stays small + let fit_w = (image_px.0 as f64 * scale).ceil().max(1.0) as u64; + let fit_h = (image_px.1 as f64 * scale).ceil().max(1.0) as u64; + let cols = fit_w.div_ceil(cell_px.cell_width_px.max(1) as u64) as u32; + let rows = fit_h.div_ceil(cell_px.cell_height_px.max(1) as u64) as u32; + Placement { + grid_cols: cols.min(cell_rect.width as u32).max(1), + grid_rows: rows.min(cell_rect.height as u32).max(1), + viewport_col: cell_rect.x as i32, + viewport_row: cell_rect.y as i32, + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn metrics(w: u32, h: u32) -> CellMetrics { + CellMetrics { + cell_width_px: w, + cell_height_px: h, + } + } + + // -- downscale target -------------------------------------------------- + + #[test] + fn downscale_target_leaves_an_already_small_image_alone() { + // Never upscale: a 400 KiB image under a 512 KiB cap keeps its own dimensions. + assert_eq!( + downscale_target((800, 600), 400 * 1024, 512 * 1024), + (800, 600) + ); + } + + #[test] + fn downscale_target_shrinks_both_edges_toward_the_cap() { + // 2 MiB against a 512 MiB… no: against a 512 KiB cap is a 4x overage, so each edge scales + // by sqrt(1/4)=0.5, times the 0.85 safety margin → 0.425. + let (w, h) = downscale_target((4000, 2000), 2048 * 1024, 512 * 1024); + assert_eq!((w, h), (1700, 850)); + assert!( + (w as f64 / h as f64 - 2.0).abs() < 0.01, + "aspect ratio is preserved: {w}x{h}" + ); + } + + #[test] + fn downscale_target_never_returns_a_zero_dimension() { + // A pathological overage must still yield something ffmpeg can scale to, not 0x0. + let (w, h) = downscale_target((10, 4), usize::MAX, 1); + assert!(w >= 1 && h >= 1, "got {w}x{h}"); + } + + #[test] + fn a_large_image_fills_the_content_box_rather_than_the_transmission_budget() { + // THE REGRESSION: `fit` used to size the placement from `frame_budget`, which is clamped + // to the host's ~512 KiB transmission cap (~362x362 px). Display and transmission are + // independent — the host rescales whatever it receives into the rect we name — so that + // clamp rendered every picture postage-stamp sized inside a big pane. + let cells = metrics(20, 41); // the probed retina metrics + let rect = Rect::new(30, 2, 60, 30); // a 1200x1230 px content box + let p = fit((3008, 1546), cells, rect); + + // 3008x1546 is wider than it is tall relative to the box, so width is the binding axis. + assert_eq!( + p.grid_cols, 60, + "the image spans the full width of the content box" + ); + assert!( + p.grid_rows >= 15, + "and takes a proportional share of the height, not a clamped sliver: {p:?}" + ); + assert!( + p.grid_rows <= rect.height as u32, + "never taller than the box" + ); + assert_eq!( + (p.viewport_col, p.viewport_row), + (30, 2), + "anchored at the box origin" + ); + } + + // -- classification ---------------------------------------------------- + + #[test] + fn extensions_classify_case_insensitively() { + for (name, kind) in [ + ("a.png", MediaKind::Png), + ("a.PNG", MediaKind::Png), + ("a.jpg", MediaKind::Image), + ("a.JPEG", MediaKind::Image), + ("a.gif", MediaKind::Image), + ("a.webp", MediaKind::Image), + ("a.svg", MediaKind::Image), + ("a.mp4", MediaKind::Video), + ("a.MKV", MediaKind::Video), + ("a.mov", MediaKind::Video), + ("a.webm", MediaKind::Video), + ] { + assert_eq!( + MediaKind::from_path(std::path::Path::new(name)), + Some(kind), + "extension {name}" + ); + } + } + + #[test] + fn unknown_extensions_are_not_media() { + for name in ["a.txt", "a", "a.md", "a.png.bak", "dir/"] { + assert_eq!( + MediaKind::from_path(std::path::Path::new(name)), + None, + "path {name:?}" + ); + } + } + + // -- PNG IHDR parsing --------------------------------------------------- + + /// A minimal, well-formed PNG header (24 bytes): magic + IHDR length + `IHDR` + 8×8. + fn png_header(w: u32, h: u32) -> Vec { + let mut b = vec![0x89, b'P', b'N', b'G', 0x0d, 0x0a, 0x1a, 0x0a]; + b.extend_from_slice(&13u32.to_be_bytes()); + b.extend_from_slice(b"IHDR"); + b.extend_from_slice(&w.to_be_bytes()); + b.extend_from_slice(&h.to_be_bytes()); + b + } + + #[test] + fn png_dimensions_read_the_ihdr_fields() { + assert_eq!(png_dimensions(&png_header(1920, 1080)), Some((1920, 1080))); + assert_eq!(png_dimensions(&png_header(8, 8)), Some((8, 8))); + // Dimensions are big-endian regardless of host byte order. + assert_eq!( + png_dimensions(&png_header(0x0001_0203, 7)), + Some((0x0001_0203, 7)) + ); + } + + #[test] + fn png_dimensions_reject_truncated_or_malformed_input() { + // One byte short of the IHDR fields (23 of 24 bytes). + let truncated = &png_header(4, 4)[..23]; + assert_eq!(png_dimensions(truncated), None); + // Same length, wrong magic. + assert_eq!(png_dimensions(b"not a png at all at all"), None); + assert_eq!(png_dimensions(&[]), None); + assert_eq!(png_dimensions(&png_header(0, 4)), None); // zero width + assert_eq!(png_dimensions(&png_header(4, 0)), None); // zero height + // A valid header whose first *content* chunk is not IHDR isn't a displayable PNG. + let mut wrong_type = png_header(4, 4); + wrong_type[12..16].copy_from_slice(b"PLTE"); + assert_eq!(png_dimensions(&wrong_type), None); + } + + // -- fit / frame budget ------------------------------------------------ + + #[test] + fn frame_budget_is_the_cell_rect_in_pixels_capped_to_the_decoded_image_budget() { + // A small pane is the raw cell-rect product (200×410 px = 82 KB decoded, under the cap). + let budget = frame_budget(Rect::new(0, 0, 10, 10), metrics(20, 41)); + assert_eq!(budget, (200, 410)); + // A retina full-pane budget would exceed 512 KiB encoded (the host cap), so it is + // clamped: at most MAX_IMAGE_BYTES/2 pixels, aspect preserved and under the cap. + let huge = frame_budget(Rect::new(0, 0, 400, 200), metrics(20, 41)); + assert!(huge.0 as u64 * huge.1 as u64 <= crate::graphics::MAX_IMAGE_BYTES as u64 / 2); + assert!( + (huge.0 as f64 / huge.1 as f64 - 400.0 * 20.0 / (200.0 * 41.0)).abs() < 0.05, + "aspect preserved under the clamp: {huge:?}" + ); + // Even a modest pane exceeds the decoded cap at retina cell sizes — the clamp is not + // only for absurd panes (this is the load-bearing measurement behind 512 KiB). + let modest = frame_budget(Rect::new(0, 0, 80, 24), metrics(20, 41)); + assert!(modest.0 as u64 * modest.1 as u64 <= crate::graphics::MAX_IMAGE_BYTES as u64 / 2); + // Pinned to the 2 B/px encoded estimate (was 461x283 under the old, needlessly + // pessimistic 4 B/px raw-RGBA assumption — see `frame_budget`). + assert_eq!(modest, (652, 401)); + // Zero cells yield zero pixels, never a divide-by-zero downstream. + assert_eq!(frame_budget(Rect::new(0, 0, 0, 0), metrics(20, 41)), (0, 0)); + } + + #[test] + fn fit_preserves_aspect_and_stays_in_the_cell_rect() { + let cell_px = metrics(20, 41); + let pane = Rect::new(0, 0, 80, 24); + // A 16:9 landscape image in a wide pane: width is the binding constraint. + let landscape = fit((1920, 1080), cell_px, pane); + assert!(landscape.grid_cols <= pane.width as u32); + assert!(landscape.grid_rows <= pane.height as u32); + // grid_cols × cell_w ≈ 1920 and grid_rows × cell_h ≈ 1080 at the same scale. + let aspect = 1920.0 / 1080.0; + let placed = (landscape.grid_cols * 20) as f64 / (landscape.grid_rows * 41) as f64; + assert!( + (placed - aspect).abs() < 0.15, + "placed aspect {placed} drifts from {aspect}" + ); + } + + #[test] + fn fit_never_upscales_a_small_image() { + let pane = Rect::new(0, 0, 80, 24); + let icon = fit((32, 32), metrics(20, 41), pane); + // 32px in 20px cells → 2 cols; in 41px rows → 1 row. Never blown up to fill the pane. + assert_eq!(icon.grid_cols, 2); + assert_eq!(icon.grid_rows, 1); + } + + #[test] + fn fit_anchors_at_the_cell_rect_origin() { + let p = fit((100, 100), metrics(20, 41), Rect::new(3, 5, 80, 24)); + assert_eq!(p.viewport_col, 3); + assert_eq!(p.viewport_row, 5); + } + + #[test] + fn fit_clamps_to_at_least_one_cell() { + let p = fit((1, 1), metrics(20, 41), Rect::new(0, 0, 10, 10)); + assert_eq!(p.grid_cols, 1); + assert_eq!(p.grid_rows, 1); + } +} diff --git a/src/media/player.rs b/src/media/player.rs new file mode 100644 index 00000000..c2960150 --- /dev/null +++ b/src/media/player.rs @@ -0,0 +1,325 @@ +//! Video playback — a long-lived, cancellable ffmpeg decoder feeding a bounded drop-oldest queue. +//! +//! The controller owns one [`Decoder`] while a video is selected. Spawning ffmpeg, reading its +//! PNG frames, and bounding the queue are all this module's job; pacing (`tick_media`) and the +//! graphics placement belong to the controller. No ffmpeg binary is required by tests — the +//! decoder command is injected (the `video` renderer), so a fake can stand in. + +use std::process::{Command, Stdio}; +use std::sync::mpsc; +use std::sync::{Arc, Mutex}; +use std::time::Duration; + +/// Frames queued ahead of the consumer (the ~125 ms tick). The consumer can only eat ~1/tick, so +/// a tiny window is all that is needed; keeping it bounded (drop-oldest) means a slow consumer +/// can never balloon memory with a backlog nobody will ever see. +const QUEUE_CAPACITY: usize = 4; + +/// Substitutes `{start}`, `{fps}`, `{width}`, `{height}` in the configured `video` command +/// template. The file path is substituted separately (`render::with_video_name`), so a hostile +/// name can never reach ffmpeg other than as its own sanitized argv element. +pub fn substitute( + template: &[String], + start: &str, + fps: &str, + width: &str, + height: &str, +) -> Vec { + template + .iter() + .map(|arg| { + arg.replace("{start}", start) + .replace("{fps}", fps) + .replace("{width}", width) + .replace("{height}", height) + }) + .collect() +} + +/// A decoded frame: raw PNG bytes, ready to hand to the graphics host. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct DecodedFrame { + pub png: Vec, +} + +/// The bounded drop-oldest queue a decoder thread feeds. +/// +/// Unbounded `mpsc` would let ffmpeg outrun the consumer forever (the measured host ceiling is +/// ~8 fps, so the decoder WILL outrun it); a fixed-capacity deque that evicts the OLDEST on +/// overflow is the plan's "bounded, drop-oldest" requirement in concrete form. +#[derive(Debug, Clone)] +pub(crate) struct FrameQueue { + inner: Arc>>, +} + +impl FrameQueue { + fn new() -> Self { + Self { + inner: Arc::new(Mutex::new(std::collections::VecDeque::new())), + } + } + + fn push(&self, frame: DecodedFrame) { + let mut q = self.inner.lock().expect("frame queue"); + if q.len() >= QUEUE_CAPACITY { + q.pop_front(); // drop the OLDEST: a backlog nobody will ever see + } + q.push_back(frame); + } + + fn is_empty(&self) -> bool { + self.inner.lock().expect("frame queue").is_empty() + } + + /// Test-only: pop the OLDEST queued frame (the framing the fixture asserts). The live reader + /// (`next_ready`) inverts this to "newest wins" — the fixture exercises the splitter, so it + /// reads in arrival order. + #[cfg(test)] + fn pop_front_(&self) -> Option { + self.inner.lock().expect("frame queue").pop_front() + } +} + +/// The live frame source: an ffmpeg child process streaming PNG frames on stdout, owned by the +/// decoder thread that reads and splits it, pushing into a [`FrameQueue`]. +pub struct Decoder { + queue: FrameQueue, + stop_tx: mpsc::Sender<()>, + handle: Option>, +} + +impl Decoder { + /// Spawn ffmpeg with `command` (the `video` renderer argv, `{name}` already replaced with + /// the file path) and start reading its stdout. + pub fn spawn(command: &[String]) -> Option { + let prog = command.first()?; + let mut cmd = Command::new(prog); + cmd.args(&command[1..]) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::null()); + + let queue = FrameQueue::new(); + let (stop_tx, stop_rx) = mpsc::channel::<()>(); + let queue_reader = queue.clone(); + let handle = std::thread::spawn(move || { + decode_loop(cmd, stop_rx, queue_reader); + }); + Some(Self { + queue, + stop_tx, + handle: Some(handle), + }) + } + + /// The newest ready frame, if any. Drop-oldest at READ time too: if a backlog formed between + /// ticks, only the newest matters (a `GraphicsCommand` carries absolute state). + pub fn next_ready(&self) -> Option { + let mut q = self.queue.inner.lock().expect("frame queue"); + let newest = q.pop_back(); + q.clear(); // discard any older queued frames + newest + } + + /// Whether the decoder thread has finished and the queue has drained — the player has nothing + /// left to show. + pub fn finished(&self) -> bool { + self.handle.as_ref().is_some_and(|h| h.is_finished()) && self.queue.is_empty() + } + + /// Stop the decoder and join its thread (a seek/close/leave). Best-effort: a wedged child is + /// killed by the loop when the stop signal lands. + pub fn stop(&mut self) { + let _ = self.stop_tx.send(()); + if let Some(h) = self.handle.take() { + let _ = h.join(); + } + } +} + +/// Read ffmpeg's stdout, split the piped bytes into individual PNG frames, and push them into the +/// queue. Re-synchronizes on the PNG signature (0x89 P N G \r \n 0x1a \n): bytes before a +/// mid-stream signature are garbage (a pipe-read boundary, ffmpeg noise) and dropped. On EOF, a +/// trailing buffer that plausibly ends a PNG is flushed as the final frame. +fn decode_loop(mut cmd: Command, stop_rx: mpsc::Receiver<()>, queue: FrameQueue) { + use std::io::Read; + + let Ok(mut child) = cmd.spawn() else { + return; // decoder never started — the player reports "no frames" + }; + let Some(mut stdout) = child.stdout.take() else { + let _ = child.kill(); + let _ = child.wait(); + return; + }; + + let mut buf: Vec = Vec::new(); + let mut read = [0u8; 64 * 1024]; + loop { + if stop_rx.try_recv().is_ok() { + let _ = child.kill(); + break; + } + match stdout.read(&mut read) { + Ok(0) => break, + Ok(n) => { + buf.extend_from_slice(&read[..n]); + // A frame is complete when its IEND trailer lands (see emit_complete_frames), so + // the last frame is emitted live, not at EOF — nothing to flush here. + emit_complete_frames(&mut buf, &queue); + } + Err(_) => break, + } + } + let _ = crate::proc::terminate_and_reap(&mut child); +} + +/// Emit every "complete" PNG in `buf`, leaving any trailing partial in place for the next read. +/// A frame is complete when another signature follows it; the trailing run is flushed at exit. +fn emit_complete_frames(buf: &mut Vec, queue: &FrameQueue) { + loop { + // Garbage before the first signature (a partial pipe read) — drop it. + if let Some(first) = find_sig(buf) + && first > 0 + { + buf.drain(..first); + continue; + } + match find_sig_next(buf) { + // The first frame ends where the NEXT signature begins. + Some(next) => { + queue.push(DecodedFrame { + png: buf[..next].to_vec(), + }); + buf.drain(..next); + } + None => { + // No successor signature YET — but a frame is also complete when it both starts + // with the signature and ends with its IEND trailer, so the LAST frame does not + // have to wait for EOF (and a trailing frame is never delayed by one latency). + if buf.starts_with(&PNG_SIG) && ends_png(buf) { + queue.push(DecodedFrame { png: buf.clone() }); + buf.clear(); + } + return; + } + } + } +} + +const PNG_SIG: [u8; 8] = [0x89, b'P', b'N', b'G', 0x0d, 0x0a, 0x1a, 0x0a]; + +fn find_sig(hay: &[u8]) -> Option { + hay.windows(8).position(|w| w == PNG_SIG) +} + +/// The offset of the NEXT PNG signature after the first. +fn find_sig_next(hay: &[u8]) -> Option { + (1..hay.len()).find(|&i| hay[i..].starts_with(&PNG_SIG)) +} + +/// Cheap "this buffer ends a PNG" check: the IEND chunk is the file's 12-byte trailer (`00 00 00 +/// 00 49 45 4E 44` + CRC), so `IEND` sits 8 bytes from the end. Only used to recognize a complete +/// frame mid-stream; a truncated trailer is still "incoming" and the frame waits for more bytes. +fn ends_png(buf: &[u8]) -> bool { + buf.len() >= 24 && &buf[buf.len() - 8..buf.len() - 4] == b"IEND" +} + +/// The pacing interval between frames the controller tick enforces — mirrors the measured herdr +/// display ceiling of ~8 fps for any non-trivial frame (the graphics module's Table 1). Pulling +/// faster would only refill the drop-oldest queue with frames the host cannot paint. +pub const FRAME_INTERVAL: Duration = Duration::from_millis(125); + +#[cfg(test)] +mod tests { + use super::*; + + fn sig() -> Vec { + PNG_SIG.to_vec() + } + + /// A minimal well-formed PNG (signature + IHDR + a fake-but-plausible trailer), enough for the + /// splitter's plumbing even though only the split boundaries matter here. + fn png(tag: u8) -> Vec { + let mut b = sig(); + b.extend_from_slice(&[tag, tag, tag, tag]); + b.extend_from_slice(&[0, 0, 0, 0, b'I', b'E', b'N', b'D']); + b.extend_from_slice(&[0, 0, 0, 0]); + b + } + + #[test] + fn substitute_replaces_every_video_field() { + let template: Vec = [ + "ffmpeg", + "-ss", + "{start}", + "-i", + "{name}", + "-vf", + "scale={width}:{height}", + "-r", + "{fps}", + "-", + ] + .iter() + .map(|s| s.to_string()) + .collect(); + let got = substitute(&template, "1.5", "8", "320", "180"); + assert_eq!(got[2], "1.5"); + assert_eq!(got[6], "scale=320:180"); + assert_eq!(got[8], "8"); + assert_eq!(got[4], "{name}", "'name' is substituted later"); + } + + #[test] + fn bounded_queue_drops_the_oldest_and_read_returns_newest_first() { + let q = FrameQueue::new(); + for i in 0..(QUEUE_CAPACITY + 3) { + q.push(DecodedFrame { png: vec![i as u8] }); + } + // Capacity is kept: frames 0..2 were evicted, 3..6 remain. + let mut inner = q.inner.lock().unwrap(); + assert_eq!(inner.len(), 4, "capacity holds after overflow"); + // The read side ("oldest first" here is the VecDeque pop_front; the controller's + // `next_ready` re-shapes this into newest-first drop-oldest). + assert_eq!(inner.pop_front().map(|f| f.png[0]), Some(3)); + } + + #[test] + fn frame_splitter_reassembles_a_concatenated_png_stream_from_fragments() { + let png_a = png(1); + let png_b = png(2); + let stream = [&png_a[..], &png_b[..]].concat(); + + let queue = FrameQueue::new(); + let mut buf = Vec::new(); + for byte in &stream { + buf.push(*byte); + emit_complete_frames(&mut buf, &queue); + } + // Sliced delivery still yields both frames once each successor's signature appears. + let mut frames = Vec::new(); + while let Some(f) = queue.pop_front_() { + frames.push(f.png); + } + assert_eq!(frames, vec![png_a, png_b]); + } + + #[test] + fn frame_splitter_drops_garbage_before_a_mid_stream_signature() { + let stream = [vec![0xff, 0xfe, 0xfd], sig(), vec![7, 7]].concat(); + let queue = FrameQueue::new(); + let mut buf = Vec::new(); + for byte in &stream { + buf.push(*byte); + emit_complete_frames(&mut buf, &queue); + } + // No second signature ever arrives → nothing is "complete" → the garbage+partial never + // becomes a frame. + assert!( + queue.pop_front_().is_none(), + "garbage never becomes a frame" + ); + } +} From af3eea914f5eead650a81ce83b287c462342e034 Mon Sep 17 00:00:00 2001 From: Charles Ji Date: Sun, 9 Aug 2026 18:04:13 -0400 Subject: [PATCH 2/8] feat(media): render, place, and play media in the content pane 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`. --- ARCHITECTURE.md | 24 +- config.example.toml | 9 +- docs/configuration.md | 16 + docs/keys.md | 3 + docs/renderers.md | 53 +++ docs/usage.md | 22 + src/app.rs | 168 ++++++- src/config.rs | 83 ++++ src/controller/mod.rs | 468 +++++++++++++++++++- src/help.rs | 8 +- src/input.rs | 32 ++ src/intent.rs | 27 +- src/render.rs | 849 +++++++++++++++++++++++++++++++++++- src/view_policy.rs | 48 +- tests/annotations.rs | 1 + tests/common/mod.rs | 1 + tests/controller.rs | 21 + tests/controller_async.rs | 30 +- tests/docs_consistency.rs | 3 + tests/lineselect.rs | 11 + tests/media_shown.rs | 442 +++++++++++++++++++ tests/render_delegate.rs | 21 + tests/reroot.rs | 2 + tests/reveal_open.rs | 1 + tests/search_integration.rs | 2 + tests/update_banner.rs | 1 + 26 files changed, 2327 insertions(+), 19 deletions(-) create mode 100644 tests/media_shown.rs diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 3dd9a4c6..a8bf80e1 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -30,8 +30,10 @@ is unit-testable with stubs. | `herdr` | The herdr CLI seam (`$HERDR_BIN_PATH`): read-only queries (list git worktrees / which workspaces have an active agent) plus a best-effort host **layout** command (`pane zoom --current --on`/`--off`, the `Z` full-screen toggle). Neither touches file or git state; an absent or failing herdr degrades gracefully (git-only picker; in-pane zoom only). | | `worktree` | Enumerate the repo's git worktrees (`git worktree list --porcelain`) and overlay herdr's agent-active workspace + per-row agent status, feeding the switch-worktree picker. | | `tree` | The rooted, `.gitignore`-aware file tree: filters (gitignored, changed-only, hidden/dotfiles), cursor, expansion, status markers, and the `]` / `[` changed-file jump. Optionally folds a chain of single-child directories into one row (`compact_dirs`). A folded row has to look inside a **collapsed** directory, which the tree never opens otherwise, so foldability is answered by a two-entry probe rather than a listing and the answer is memoized — re-probed wherever the controller re-reads git. Listings stay uncached, so a compacted frame reads exactly the directories an uncompacted one does. | -| `view_policy` | A pure decision: which view mode a file gets (changed → diff, markdown → rendered, else → syntax content) and the cycle order. | -| `render` | Produce the content-pane text: classify the file, delegate styling to an external CLI, and **neutralize escape sequences** before display. | +| `view_policy` | A pure decision: which view mode a file gets (changed → diff, markdown → rendered, media → Media, else → syntax content) and the cycle order. | +| `render` | Produce the content-pane text: classify the file, delegate styling to an external CLI, and **neutralize escape sequences** before display; also produce the Media view's PNG payload (`render_media`: native PNG bytes, or conversion / video frame 0 through the injected `image`/`video` commands). | +| `graphics` | The herdr graphics socket seam: the verified `pane.graphics.info`/`set`/`clear` JSON protocol over `$HERDR_SOCKET_PATH` (one request per connection), the 512 KiB decoded-image cap and ~1 MiB base64 drop, a hand-rolled base64 encoder, and the last-wins-collapsing `GraphicsWorker` that keeps 120+ ms host round-trips off the UI thread (each `set` is absolute state, so collapsing is correctness, not approximation). Media bytes reach the host base64-encoded **inside a JSON request — no ESC byte is ever written**, so AC-27's neutralizer and `tests/render_escape.rs` stay untouched. | +| `media` | Pure media decisions: `MediaKind` by extension, PNG IHDR parsing, aspect-preserving `fit` into the pane's cell grid, the pixel clock of `frame_budget` (also clamped to the host's decoded cap), and the `player` submodule (the ffmpeg decoder thread + bounded drop-oldest frame queue). | | `presenter` | Draw the two-column (or zoomed / narrow) layout with ratatui, including persistent annotation markers and background-only styling; source-line backgrounds are applied beneath active line-select, ambient-selection, and search overlays, with a bounded one-cell cue for blank annotated lines. Scroll the tree/content and report viewport + pane geometry back for hit-testing. | | `picker` | The modal worktree-switcher overlay state (rows, cursor, horizontal scroll) drawn over the layout; captures its own nav / confirm / cancel keys while open. | | `proc` | Shared subprocess reaping: one `wait_bounded` (child wait + poll + timeout-kill) used by both the content renderer and the update check, so the timeout-kill semantics are defined once. | @@ -92,6 +94,17 @@ retain file/title markers where applicable but never receive guessed source-line - **Delegate rendering.** Markdown, diffs, and syntax highlighting are produced by best-in-class external CLIs (`glow`, `delta`, `bat`): the viewer builds only the shell and ingests their ANSI output. Each renderer is optional; a missing one degrades to plain text + a notice. +- **Media goes over the socket, not the terminal.** Images and video frames are handed to herdr + through its documented `pane.graphics.*` socket API as base64 inside JSON — never as escape + sequences on stdout — so a hostile image still cannot drive the terminal (AC-27, `tests/ + render_escape.rs`, and SECURITY.md's "a malicious file cannot drive the terminal" guarantee all + stay intact). Placement is data (a cell rect), refreshed by the `media_shown` clear/set + discipline after every draw, so scrolling/resizing/zooming can never strand a stale frame. + Video is a deliberate scope stretch (the plan said so): it stays inside the same boundaries — + ffmpeg only reads, playback is keyboard-driven and paused by default. +- **The herdr host sets the pace.** herdr re-renders its whole client frame per `set` (~120 ms + fixed regardless of payload), which is why video targets **~8 fps** and why the graphics worker + exists at all. This ceiling is herdr's, documented as such so it doesn't read as a bug. - **Git is first-class**, woven through the tree (status markers, colors, changed-only filter, baseline toggle) and the content pane (diff view), not a separate mode. - **In-memory, ephemeral state only**, including annotations, which start empty and are scoped to @@ -104,7 +117,12 @@ retain file/title markers where applicable but never receive guessed source-line Four untrusted inputs are handled defensively (see [SECURITY.md](SECURITY.md)): 1. **File content** is untrusted: fed to renderers on **stdin** (never as an argument), and the - renderer output is re-sanitized so no escape sequence can drive the terminal. + renderer output is re-sanitized so no escape sequence can drive the terminal. Media preserves + the stdin rule for images (raw bytes piped into the `image` converter). **One deliberate + exception:** you cannot `-ss`-seek a pipe, so video decoding passes the file **path** to + ffmpeg (as its own argv element, no shell). The path is the already-canonicalized in-root one + from the render classifier, so a hostile *filename* cannot inject; this is a documented + narrowing of this boundary, and the reason `video` is a template with explicit placeholders. 2. **The git repository** may be untrusted (an agent's worktree, a clone): every `git` invocation is hardened against repo-controlled code execution (no external diff/textconv, neutralized `core.fsmonitor`/`core.hooksPath`, scrubbed repo-redirecting env). This hardening diff --git a/config.example.toml b/config.example.toml index 36f7f748..691cb622 100644 --- a/config.example.toml +++ b/config.example.toml @@ -55,6 +55,13 @@ #markdown = "glow -s dark -w 0 -" #diff = "delta" #syntax = "bat --color=always --style=numbers --paging=never --file-name={name} -" +#image = "ffmpeg -loglevel error -i pipe:0 -sws_flags neighbor -vf scale={width}:{height}:force_original_aspect_ratio=decrease -f image2 -vcodec png pipe:1" +#video = "ffmpeg -loglevel error -re -ss {start} -i {name} -an -vf scale={width}:{height} -r {fps} -f image2pipe -vcodec png -" + +# Media size cap, in KiB: how large an image/video file may be before the Media view +# shows its placeholder instead. Separate from the text preview cap (`preview_max_kib`, +# below) whose 1 MiB default is far too small for media. Default: 8192 (8 MiB). +#media_max_kib = 8192 # OS hand-off commands: `O` opens the selected entry with an application, `R` # reveals it in a file manager. Defaults are the per-OS system openers @@ -156,7 +163,7 @@ # Keybindings tab (e.g. refresh, nav_up, switch_worktree). A value is a KEY SPEC: # a single string, or an array of strings. An entry REPLACES that action's # default key(s), so list every key you want it to answer to. The full list of -# intent names (all 39 actions) is in docs/configuration.md (Keybindings) and the +# intent names (all 45 actions) is in docs/configuration.md (Keybindings) and the # `?` overlay's Keybindings tab. # # Bindable keys: any single printable or shifted character (`g`, `<`, `?`, and diff --git a/docs/configuration.md b/docs/configuration.md index aa22d1de..eb4e5961 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -61,6 +61,8 @@ editor = "code --wait" # command to open a file with `e` (overrides $EDITOR markdown = "glow -s dark -w 0 -" # override the markdown / diff / syntax renderers diff = "delta" # (defaults: glow / delta / bat) syntax = "bat --color=always --style=numbers --paging=never --file-name={name} -" +image = "ffmpeg -loglevel error -i pipe:0 -sws_flags neighbor -vf scale={width}:{height}:force_original_aspect_ratio=decrease -f image2 -vcodec png pipe:1" # non-PNG → PNG +video = "ffmpeg -loglevel error -re -ss {start} -i {name} -an -vf scale={width}:{height} -r {fps} -f image2pipe -vcodec png -" # frames open = "xdg-open" # override the `O` open-with / `R` reveal-in-file-manager commands reveal = "nautilus" @@ -77,6 +79,7 @@ tree_position = "left" # which side the directory tree sits on: "left" (def preview_max_lines = 10000 # show at most this many lines before a truncated preview (100–100000) preview_max_kib = 1024 # ...or this size before truncating, in KiB (1024 = 1 MB; 64–65536) +media_max_kib = 8192 # media size cap, in KiB; separate from preview_max_kib (512–131072) ``` `update_check` governs release details and project spotlights. `false` disables all remote requests @@ -107,6 +110,15 @@ One caveat for **diffs**: a diff is additionally bounded at ~4 MB by the git-cap of `preview_max_kib`. So raising `preview_max_kib` above ~4 MB widens how much *file content* is shown but not how much of a very large *diff* is (a diff past that bound is shown up to ~4 MB). +`image` and `video` are the **media** converters (see [external renderers](renderers.md)). `image` +turns any non-PNG image into PNG — it receives the raw file bytes on **stdin** and must write PNG on +stdout (like glow/bat's trailing `-`, the contract is "read stdin"; the default is an ffmpeg pipe). +`video` is a frame-extraction template with `{start}` / `{fps}` / `{width}` / `{height}` placeholders +(the pane's pixel budget is substituted for `{width}`/`{height}`) and `{name}` for the file path; the +default is an ffmpeg invocation. `media_max_kib` caps how large a media file may be before the Media +view shows a placeholder instead — it is **separate** from `preview_max_kib`, whose 1 MiB default is a +*text* budget far too small for images. Default `8192` (8 MiB), clamp `512–131072`. + `compact_dirs` changes the tree's **shape**, not what it shows. With it on, a chain of directories that each hold nothing but one subdirectory is drawn as a single row — `src/main/java/br/com` instead of six rows, each indented two columns further than the last. The row leads into the deepest @@ -189,6 +201,10 @@ customized). | | `toggle_zoom` | `z` | Hide the tree so content fills the frame, or restore the split | | | `tree_scroll_left` | `H` | Scroll the tree pane left | | | `tree_scroll_right` | `L` | Scroll the tree pane right | +| | `media_play_pause` | `p` | Toggle play/pause of the selected video | +| | `media_seek_back` | `{` | Seek the selected video back | +| | `media_seek_forward` | `}` | Seek the selected video forward | +| | `media_restart` | `0` | Restart the selected video from the beginning | | **Git & filters** | `toggle_ignore` | `i` | Reveal or hide gitignored files | | | `toggle_hidden` | `.` | Hide or reveal dot-prefixed (hidden) files and folders | | | `toggle_changed_only` | `c` | Restrict the tree to changed files (baseline-aware), or restore the full tree | diff --git a/docs/keys.md b/docs/keys.md index a80548f8..cd806bdc 100644 --- a/docs/keys.md +++ b/docs/keys.md @@ -48,6 +48,9 @@ is additive and on by default. | `?` (Shift+`/`) | Open help with **What's New** details selected first, including updates and spotlights; `Esc` / `q` closes it | | `u` | Dismiss the whole advisory status row for this session only; **What's New** stays available | | `q` / `Esc` | Back out of zoom if zoomed; otherwise close the viewer and return to the prior pane. With annotations held, a confirm appears first (`y` copies them and quits, `q` quits and discards, `Esc` returns to the viewer): they are session-only, so quitting destroys them. Skip it with `confirm_discard = false` | +| `p` | **Play/pause** the selected video. Inert unless a video is selected in Media mode. Playback starts paused on the first frame, so selecting a video never starts motion unasked | +| `{` / `}` | **Seek** the selected video back / forward 5 seconds. Inert unless a video is selected | +| `0` | **Restart** the selected video from the beginning. Inert unless a video is selected | These are the **default global** keys. Remap them with a `[keys]` table in the [config file](configuration.md#keybindings). Keys handled inside line-select mode, the annotation diff --git a/docs/renderers.md b/docs/renderers.md index 50d6aa93..a7f6f147 100644 --- a/docs/renderers.md +++ b/docs/renderers.md @@ -8,6 +8,9 @@ dependencies (not Cargo dependencies) and each is **optional**: | Rendered markdown | [`glow`](https://github.com/charmbracelet/glow) | `brew install glow` / package manager | | Diffs | [`delta`](https://github.com/dandavison/delta) | `brew install git-delta` / `cargo install git-delta` | | Syntax-highlighted content | [`bat`](https://github.com/sharkdp/bat) | `brew install bat` / package manager | +| **Images** (non-PNG) → PNG | [`ffmpeg`](https://ffmpeg.org/) | `brew install ffmpeg` / package manager | +| **Video** frames → PNG | [`ffmpeg`](https://ffmpeg.org/) | `brew install ffmpeg` / package manager | +| Media caption details | [`ffprobe`](https://ffmpeg.org/) (ships with ffmpeg) | included with ffmpeg | Or install all three at once with the bundled helper (best-effort; detects brew/apt/dnf/pacman and falls back to `cargo install` for `delta` and `bat`; `glow` is written in Go, so the helper @@ -28,6 +31,56 @@ Untrusted file content is always fed to a renderer on **stdin** (never as a comm and the renderer's output is re-sanitized before display, so a hostile file name or file content cannot inject a command or drive the terminal. +### Media (images and video) + +A media file's still image is shown **inline** in the content pane (via herdr's documented +graphics socket — no escape sequences are ever written, so a hostile file still cannot drive the +terminal). What ffmpeg is needed for: + +- **A `.png` is shown natively** — no conversion, ffmpeg not involved — when it is no larger than + the pane displays and fits the host's **512 KiB** limit. A bigger one is resampled to the pane's + own pixel size with a high-quality filter (`lanczos`): those pixels could never be shown anyway, + so nothing visible is lost, and the result usually fits the cap outright. If it still does not, + a quality ladder re-encodes it — `lanczos`, then `neighbor` at the same size, then smaller — + stopping at the first rung that fits, so sharpness is given up only as far as the cap forces. + The caption always reports the file's true pixel size. Without ffmpeg there is nothing to + resample with, so an over-cap PNG shows its caption plus a notice instead of a picture. +- **Other images** (jpg, gif, webp, svg, …) convert to PNG through ffmpeg (the `image` command, + defaulting to `ffmpeg -loglevel error -i pipe:0 -sws_flags neighbor -vf scale={width}:{height}:force_original_aspect_ratio=decrease -f image2 -vcodec png pipe:1`). The file bytes + are fed on **stdin**, keeping the stdin trust boundary intact. `{width}`/`{height}` are + substituted on every call with the target box, so a replacement command must carry them. + The default uses nearest-neighbour scaling deliberately: on screenshots and diagrams — what + actually lives in a code repo — a smoothing filter invents intermediate colours across flat + regions and can make the re-encoded PNG *larger than the original*, while neighbour keeps text + crisp and roughly halves the bytes. Set `-sws_flags area` instead if you mostly view photographs. +- **Video** (mp4, mkv, webm, mov, …) decodes frames through the `video` command template + (default `ffmpeg -loglevel error -re -ss {start} -i {name} -an -vf scale={width}:{height} -r {fps} + -f image2pipe -vcodec png -`, with the pane's pixel budget substituted for `{width}`/`{height}`). + `p` plays/pauses, `{`/`}` seek ±5s, `0` restarts. A replacement command should keep `-re`: it + makes ffmpeg read at the input's native rate, so frames arrive at roughly the speed they can be + displayed. Without it ffmpeg races to the end of the file (a 7-second clip decodes in under half + a second), the queue discards almost every frame, and playback appears to stop immediately. The + poster frame and playback are decoded at the same target size, so pressing `p` never resizes the + picture. +- **A caption above the picture** reports what you are looking at, e.g. + `[image: 3008×1546 · PNG · 8-bit RGBA · 655 KiB]` or + `[video: 854×480 · 0:07 · HEVC · 982 KiB · p to play]`. Colour depth and type come from the PNG + header directly; a video's resolution, codec, and duration come from `ffprobe`, and are simply + omitted when it is unavailable. The caption always reports the file's OWN size — if the picture + had to be re-encoded smaller to fit the host's cap, a `shown at W×H` clause says so. The picture + is placed *below* the caption, never over it. + +When ffmpeg is absent, media files show a placeholder plus a notice naming ffmpeg (use +`config.example.toml` / the `image` / `video` keys to point at an alternative converter). The +`media_max_kib` config key bounds how large a media file may be before the Media view shows a +placeholder instead. + +**Playback rate is host-limited.** herdr re-renders its full client frame for every `set`, which +measures ~120 ms of fixed cost regardless of payload size — so practical video tops out around +**8 fps**. This is herdr's ceiling, not the viewer's; frames are capped at ~150 KiB so video +remains comfortably inside herdr's 512 KiB decoded-image limit. See `ARCHITECTURE.md` for the +measured table. + ### Bundled markdown palette The viewer ships a small bundled markdown style palette (`assets/markdown-style.json`) that diff --git a/docs/usage.md b/docs/usage.md index 1015d7f1..14e97d21 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -187,6 +187,28 @@ Rendering is **delegated** to `glow` (markdown), `delta` (diffs), and `bat` (syn renderer isn't installed the viewer falls back to plain text with a short notice. See [external renderers](renderers.md). +### Media (images and video) + +A `.png`, another still image, or a video shows **in the content pane** through herdr's graphics +socket — you don't need a key to reach it (it's the file's default view), and the image stays +placed even as you scroll, zoom, or resize, because its cell rectangle is recomputed every draw. + +- **Images** render automatically. A `.png` is shown natively; other formats (jpg, gif, webp, + svg, …) convert through `ffmpeg` (see [external renderers](renderers.md)). If ffmpeg is + missing, or the file is larger than the `media_max_kib` cap, you get a placeholder line instead. +- **Video** shows its first frame, paused, waiting for you to ask for motion: + - `p` — play/pause. + - `{` / `}` — seek back / forward 5 seconds (re-decodes from the new offset). + - `0` — restart from the beginning. + These keys are inert unless a video is selected. Playback **rate is host-limited to ~8 fps**: + herdr re-renders its whole client frame per image, which is a fixed ~120 ms cost, so the video + pace is herdr's ceiling, not the viewer's (see [external renderers](renderers.md)). +- The `v` mode cycle still reaches the plain text placeholder beneath an image. + +Media needs herdr's own `experimental.kitty_graphics = true` (and a kitty-compatible outer +terminal). If that's off — or you're running the viewer outside herdr — you get the text +placeholder plus a notice; nothing crashes. + ## Git awareness Git status is woven straight into the tree, not a separate mode: diff --git a/src/app.rs b/src/app.rs index 074a3cc4..ef6c43da 100644 --- a/src/app.rs +++ b/src/app.rs @@ -87,6 +87,7 @@ pub fn run(open_flag: Option) -> io::Result<()> { root: resolved.root.clone(), renderers: factory_renderers.clone(), caps, + media_max_bytes: eff.media_max_kib as u64 * 1024, }); RootProviders { git, content } }); @@ -194,6 +195,21 @@ pub fn run(open_flag: Option) -> io::Result<()> { crate::opener::CommandOpener::new(current_os_kind(), Box::new(OpenerSpawner)) .with_overrides(to_argv(eff.open.clone()), to_argv(eff.reveal.clone())), )); + // Inject the graphics host (kitty images in the content pane): the live socket client plus + // one startup probe for the cell metrics that map pane cells → image pixels. `info()` + // succeeding proves the pane exists but NOT that herdr's `experimental.kitty_graphics` is on — + // so the unavailable notice below must name that setting explicitly. Outside herdr (or when + // the probe fails) media degrades to the text line via the null sink. + match crate::graphics::LiveGraphics::from_env() { + Some(host) => { + let metrics = crate::graphics::GraphicsHost::info(&host).ok(); + controller.set_graphics( + Box::new(crate::graphics::GraphicsWorker::spawn(Box::new(host))), + metrics, + ); + } + None => controller.set_graphics(Box::new(crate::graphics::NullSink), None), + } let mut terminal = ratatui::try_init()?; // Mouse is additive to the keyboard-first design (AC-18): herdr forwards mouse events to a @@ -215,6 +231,9 @@ pub fn run(open_flag: Option) -> io::Result<()> { let outcome = event_loop(&mut terminal, &mut controller); let _ = execute!(io::stdout(), DisableMouseCapture); let _ = execute!(io::stdout(), DisableFocusChange); + // Teardown: leave the pane clean — block until the host reports nothing displayed, so a quit + // never leaves a residual graphic behind (the graphics worker joins its thread here). + controller.clear_media(); ratatui::try_restore()?; outcome } @@ -255,6 +274,10 @@ fn event_loop(terminal: &mut DefaultTerminal, controller: &mut Controller) -> io // tree-only pane) and we must paint again so the file is actually visible. need_redraw = controller.set_content_viewport(cw, ch); controller.set_pane_geometry(presenter::geometry(frame.area(), &view)); + // The clear/set discipline runs on the fresh geometry: compare what the host + // should show against what it does, and clear/set on a difference. Non-blocking + // (the graphics host worker does the round-trips off this thread). + controller.sync_media(); })?; dirty = need_redraw; } @@ -398,6 +421,11 @@ fn event_loop(terminal: &mut DefaultTerminal, controller: &mut Controller) -> io if controller.tick_flash(now) { dirty = true; } + // Advance video playback: pull one frame per tick and send it to the graphics host. A + // redraw is owed when a new frame changed the surface. + if controller.tick_media(now) { + dirty = true; + } // Launch open-range passive highlight (1s); redraw once when it expires. if controller.tick_open_range_flash(now) { dirty = true; @@ -488,14 +516,26 @@ struct LiveContent { /// The size caps (line + byte) for classifying/previewing content, resolved from config /// (`preview_max_lines` / `preview_max_kib`) at startup. `Copy`. caps: Caps, + /// The media size cap (the `media_max_kib` config key): bounds the media file read and the + /// captured converter output. Deliberately separate from the (much smaller) text-preview cap. + media_max_bytes: u64, } impl ContentProvider for LiveContent { fn render(&self, path: &Path, mode: ViewMode, raw_diff: Option<&str>) -> RenderResult { // The width-less entry point: no pane width known, so glow keeps its `-w 0` (no wrap). - self.render_at_width(path, mode, raw_diff, None, None, DiffRenderMode::default()) + self.render_at_width( + path, + mode, + raw_diff, + None, + None, + DiffRenderMode::default(), + None, + ) } + #[allow(clippy::too_many_arguments)] fn render_at_width( &self, path: &Path, @@ -504,7 +544,33 @@ impl ContentProvider for LiveContent { width: Option, pane_width: Option, diff_render_mode: DiffRenderMode, + media_box: Option<(u32, u32)>, ) -> RenderResult { + // Media view: the pane shows the image itself (via the graphics socket) over a text + // fallback line, so there is no text pipeline to classify — classify would only ever + // produce the binary placeholder for a PNG's NUL bytes. + if mode == ViewMode::Media { + let (content, notice, media) = match crate::media::MediaKind::from_path(path) { + Some(kind) => render::render_media( + &self.renderers, + path, + kind, + self.media_max_bytes, + media_box, + ), + None => ( + ratatui::text::Text::raw("[media: unsupported file]"), + None, + None, + ), + }; + return RenderResult { + content, + notices: notice.into_iter().collect(), + source: None, + media, + }; + } // Both diff modes render from git's diff text, not the file bytes — so a deleted or // binary file still shows its diff (AC-9), and there is no point classifying (a wasted // bounded file read). Other modes classify first (binary / size guards, AC-12/13). @@ -592,6 +658,7 @@ impl ContentProvider for LiveContent { content, notices: notice.into_iter().collect(), source, + media: None, } } } @@ -904,6 +971,91 @@ fn default_renderers() -> Renderers { "--file-name={name}".into(), "-".into(), ], + // Non-PNG images convert to PNG through ffmpeg. Both pipes: raw file bytes on stdin, + // PNG on stdout — the untrusted file is never an argv element (trust boundary #1). + // The `{name}` placeholder stays out of this command: the file is fed by pipe, not name. + image: vec![ + "ffmpeg".into(), + "-loglevel".into(), + "error".into(), + "-i".into(), + "pipe:0".into(), + // Nearest-neighbour, counter-intuitively, is what makes the size cap reachable. + // Measured on this repo's own 3008x1546 screenshots: rescaling with the default + // bicubic filter made the PNG *larger* than the original (655 KiB -> 711 KiB), + // because interpolation invents intermediate colours across the flat regions that + // PNG was compressing so well. At the same target width, neighbour gives 350 KiB vs + // bicubic's 711 KiB, area's 572 KiB, and lanczos's 794 KiB — and on screenshots, + // diagrams, and UI captures (what actually lives in a code repo) it is *sharper*, not + // worse, since it never blurs text. Photographs are the case that would prefer + // `area`; that is a one-word config change away. + "-sws_flags".into(), + "neighbor".into(), + // `{width}`/`{height}` are substituted on every invocation (see + // `render::with_image_size`), so this command both converts AND bounds the result. + // That matters because herdr rejects anything over 512 KiB decoded: an over-cap PNG is + // re-encoded through this same command until it fits, rather than being dropped. + // `force_original_aspect_ratio=decrease` never upscales and never distorts. + "-vf".into(), + "scale={width}:{height}:force_original_aspect_ratio=decrease".into(), + "-f".into(), + "image2".into(), + "-vcodec".into(), + "png".into(), + "pipe:1".into(), + ], + // Video frame extraction — a template, so `{start}`/`{fps}`/`{width}`/`{height}` are + // substituted per seek/playback. The FILE PATH is substituted via `{name}` (argv element, + // no shell): you cannot `-ss`-seek a pipe, the one deliberate narrowing of the stdin + // trust boundary (see ARCHITECTURE.md). `-an` guarantees silence, `-f image2pipe` emits + // standalone PNG frames, and `scale=…:force_original_aspect_ratio=decrease` keeps every + // frame's aspect inside the pane's pixel budget (the box it would otherwise letterbox). + // Codec, native resolution, and duration for the Media info line, emitted as `key=value` + // lines. The keys are kept deliberately: ffprobe orders stream fields internally, so + // parsing by position would silently mis-assign values if that order ever changed. + // Ships with ffmpeg, so it is present whenever the video command is; when it is not, the + // info line simply omits these fields. + probe: vec![ + "ffprobe".into(), + "-v".into(), + "error".into(), + "-select_streams".into(), + "v:0".into(), + "-show_entries".into(), + "stream=codec_name,width,height".into(), + "-show_entries".into(), + "format=duration".into(), + "-of".into(), + "default=noprint_wrappers=1".into(), + "{name}".into(), + ], + video: vec![ + "ffmpeg".into(), + "-loglevel".into(), + "error".into(), + // Read the input at its NATIVE frame rate. Without this ffmpeg decodes as fast as the + // CPU allows — measured: a 6.9-second clip emitted its entire frame stream in 0.377s — + // so the bounded drop-oldest queue discarded almost every frame and the decoder hit EOF + // before playback had begun. The result looked like "video doesn't work": four frames + // flashed past and playback ended. `-re` paces the producer to match the ~8 fps the + // consumer can actually display, which is what makes a video play for its real + // duration. It is an INPUT option, so it must precede `-i`. + "-re".into(), + "-ss".into(), + "{start}".into(), + "-i".into(), + "{name}".into(), + "-an".into(), + "-vf".into(), + "scale={width}:{height}:force_original_aspect_ratio=decrease".into(), + "-r".into(), + "{fps}".into(), + "-f".into(), + "image2pipe".into(), + "-vcodec".into(), + "png".into(), + "-".into(), + ], timeout: RENDER_TIMEOUT, } } @@ -987,6 +1139,7 @@ mod tests { content: ratatui::text::Text::raw("body"), notices: Vec::new(), source: None, + media: None, } } } @@ -1369,9 +1522,15 @@ mod tests { diff: vec!["cat".into()], full_diff: vec!["cat".into()], syntax: vec!["cat".into()], + image: vec!["cat".into()], + video: vec!["cat".into()], + probe: Vec::new(), timeout: Duration::from_secs(5), }, + // The two test LiveContent renderers above (using Renderers { .. }) share the media + // defaults' size cap; it only gates the disk read, so the default is always correct here. caps: Caps::default(), + media_max_bytes: render::DEFAULT_MEDIA_MAX_BYTES, } } @@ -1391,6 +1550,9 @@ mod tests { diff: vec!["cat".into()], full_diff: vec!["cat".into()], syntax: vec!["cat".into()], + image: vec!["cat".into()], + video: vec!["cat".into()], + probe: Vec::new(), timeout: Duration::from_secs(5), }, // A 50-line cap the default would never apply — proves the injected cap is what bites. @@ -1398,6 +1560,7 @@ mod tests { max_lines: 50, max_bytes: 1024 * 1024, }, + media_max_bytes: render::DEFAULT_MEDIA_MAX_BYTES, }; let out = content.render_at_width( &file, @@ -1406,6 +1569,7 @@ mod tests { None, None, DiffRenderMode::default(), + None, ); assert!( out.notices.iter().any(|n| n.contains("50-line")), @@ -1441,6 +1605,7 @@ mod tests { Some(80), None, DiffRenderMode::default(), + None, ); assert!( flatten_content(&out).contains("W=80"), @@ -1468,6 +1633,7 @@ mod tests { w, None, DiffRenderMode::default(), + None, ); assert!( flatten_content(&out).contains("W=0"), diff --git a/src/config.rs b/src/config.rs index 66187150..dbb0da12 100644 --- a/src/config.rs +++ b/src/config.rs @@ -68,6 +68,17 @@ pub const MIN_PREVIEW_MAX_KIB: u32 = 64; /// bounded-read guarantee (AC-N1) holds at every setting; a larger value clamps down to it. pub const MAX_PREVIEW_MAX_KIB: u32 = 65_536; +/// The built-in **media size cap**, in KiB (8 MiB): the byte-bound on reading a media file's +/// bytes for the Media view, deliberately separate from the ~1 MiB text-preview cap (images are +/// naturally big; a bounded read must not turn a photo into "too large"). Mirrors +/// [`crate::render::DEFAULT_MEDIA_MAX_BYTES`]. +pub const DEFAULT_MEDIA_MAX_KIB: u32 = 8192; +/// The smallest media size cap (KiB); a smaller configured value clamps up to it. +pub const MIN_MEDIA_MAX_KIB: u32 = 512; +/// The largest media size cap (KiB) — 128 MiB. Even the maximum is a finite read, so the +/// bounded-read guarantee (AC-N1) holds at every setting. +pub const MAX_MEDIA_MAX_KIB: u32 = 131_072; + /// Which side of the content pane the directory tree is drawn on (`tree_position` config key). A /// pure display preference; the config value is a lenient `Option` resolved into this by /// [`resolve`] (case-insensitive, trimmed), so this enum is never deserialized directly. `Left` is @@ -114,6 +125,19 @@ pub struct Config { pub markdown: Option, pub diff: Option, pub syntax: Option, + /// The command that converts a non-PNG image to PNG (fed the file bytes on **stdin**), in + /// the same shell-tokenized form as the other renderer commands. `None` falls back to the + /// built-in ffmpeg pipeline. Media-only; absent ⇒ the Media view shows its placeholder. + pub image: Option, + /// The video frame-extraction template, a shell-tokenized string with `{start}` / `{fps}` / + /// `{width}` / `{height}` placeholders (plus `{name}` for the file path). `None` falls back + /// to the built-in ffmpeg template. Media-only; absent ⇒ video shows its placeholder. + pub video: Option, + /// The media size cap, in KiB: past this size a media file's bytes are not read (PNG, image + /// conversion input) — a separate budget from the text-preview `preview_max_kib`, whose 1 MiB + /// default is far too small for images. `None` falls back to [`DEFAULT_MEDIA_MAX_KIB`]. + /// Clamped to `MIN_MEDIA_MAX_KIB..=MAX_MEDIA_MAX_KIB`. Media-only. + pub media_max_kib: Option, pub open: Option, pub reveal: Option, pub hide_dotfiles: Option, @@ -281,6 +305,13 @@ pub struct EffectiveSettings { pub markdown: Option>, pub diff: Option>, pub syntax: Option>, + /// The effective non-PNG → PNG converter command argv, or `None` for the built-in default. + pub image: Option>, + /// The effective video frame-extraction template argv (with placeholder fields), or `None` + /// for the built-in default. + pub video: Option>, + /// The effective media size cap, in KiB (the `media_max_kib` config key, clamped). + pub media_max_kib: u32, pub open: Option>, pub reveal: Option>, pub hide_dotfiles: bool, @@ -357,6 +388,8 @@ pub fn resolve(config: &Config, get_env: impl Fn(&str) -> Option) -> Eff .syntax .as_deref() .map(crate::editor::tokenize_command); + let image = config.image.as_deref().map(crate::editor::tokenize_command); + let video = config.video.as_deref().map(crate::editor::tokenize_command); let open = config.open.as_deref().map(crate::editor::tokenize_command); let reveal = config .reveal @@ -439,11 +472,23 @@ pub fn resolve(config: &Config, get_env: impl Fn(&str) -> Option) -> Eff .map(|n| n.clamp(MIN_PREVIEW_MAX_KIB, MAX_PREVIEW_MAX_KIB)) .unwrap_or(DEFAULT_PREVIEW_MAX_KIB); + // Config > default; no env var. Clamp to `MIN_MEDIA_MAX_KIB..=MAX_MEDIA_MAX_KIB`: the upper + // bound keeps the bounded-read guarantee (AC-N1) even at the max, so no configured value can + // ask the viewer to slurp an arbitrary file whole; the lower bound keeps a photo from being + // unusably tiny. + let media_max_kib = config + .media_max_kib + .map(|n| n.clamp(MIN_MEDIA_MAX_KIB, MAX_MEDIA_MAX_KIB)) + .unwrap_or(DEFAULT_MEDIA_MAX_KIB); + EffectiveSettings { editor, markdown, diff, syntax, + image, + video, + media_max_kib, open, reveal, hide_dotfiles, @@ -508,6 +553,10 @@ pub fn effective_renderers( diff, full_diff, syntax: eff.syntax.clone().unwrap_or_else(|| base.syntax.clone()), + image: eff.image.clone().unwrap_or_else(|| base.image.clone()), + video: eff.video.clone().unwrap_or_else(|| base.video.clone()), + // Not user-configurable: the info line is a convenience, not a rendering decision. + probe: base.probe.clone(), timeout: base.timeout, } } @@ -1406,6 +1455,37 @@ mod tests { assert_eq!(crate::render::Caps::default(), from_config); } + #[test] + fn media_max_kib_and_bytes_defaults_are_in_lockstep() { + // Same lockstep guarantee for the media cap: the config-side effective KiB (used by the + // live renderer wiring) and the render-side byte default (used by the width-less/help + // paths and tests) must describe the same budget. + let from_config = resolve(&Config::default(), |_| None).media_max_kib as u64 * 1024; + assert_eq!(from_config, crate::render::DEFAULT_MEDIA_MAX_BYTES); + } + + #[test] + fn media_config_keys_resolve_and_clamp() { + // `image`/`video` tokenize like the other renderer commands; `media_max_kib` is clamped. + let (config, outcome) = parse_config( + "image = \"ffmpeg -i pipe:0 -f image2 -vcodec png pipe:1\"\n\ + video = \"ffmpeg -ss {start} -i {name} -vf scale={width}:{height}\"\n\ + media_max_kib = 200000\n", + ); + assert_eq!(outcome, LoadOutcome::Loaded); + let eff = resolve(&config, |_| None); + assert!(eff.image.is_some(), "image renders as an argv vec"); + assert!(eff.video.is_some(), "video renders as a template argv vec"); + // 200 000 KiB exceeds MAX_MEDIA_MAX_KIB, so it clamps down (not up). + assert!(eff.media_max_kib <= MAX_MEDIA_MAX_KIB); + assert_eq!(eff.media_max_kib, MAX_MEDIA_MAX_KIB); + + // A tiny cap clamps up to the floor. + let (config, _) = parse_config("media_max_kib = 10\n"); + let eff = resolve(&config, |_| None); + assert_eq!(eff.media_max_kib, MIN_MEDIA_MAX_KIB); + } + #[test] fn tree_max_cols_non_representable_degrades_to_default() { // A non-representable value fails the parse, degrading the whole config to defaults; the @@ -1448,6 +1528,9 @@ mod tests { diff: vec!["delta".to_string()], full_diff: vec!["delta".to_string(), "--line-numbers".to_string()], syntax: vec!["bat".to_string(), "-".to_string()], + image: vec!["ffmpeg".to_string(), "-i".to_string(), "pipe:0".to_string()], + video: vec!["ffmpeg".to_string(), "{name}".to_string()], + probe: Vec::new(), timeout: std::time::Duration::from_secs(2), } } diff --git a/src/controller/mod.rs b/src/controller/mod.rs index 6ed8c613..5b084e3d 100644 --- a/src/controller/mod.rs +++ b/src/controller/mod.rs @@ -33,6 +33,7 @@ mod picker; use crate::annotation::AnnotationStore; use crate::finder::FinderState; use crate::git::{Baseline, Status}; +use crate::graphics::{CellMetrics, GraphicsSink, NullSink, Placement}; use crate::help::{HelpSection, HelpSectionState, HelpState}; use crate::herdr::HerdrCli; use crate::infile::{PromptMode, PromptState, SearchState}; @@ -44,7 +45,7 @@ use crate::presenter::{ FinderView, Focus, HelpView, LineSelectView, PaneGeometry, PickerRowView, PickerView, ViewState, }; -use crate::render::Renderers; +use crate::render::{MediaPayload, Renderers}; use crate::root::Resolved; use crate::tree::{Node, NodeKind, TreeModel}; use crate::update::{self, NoticeSnapshot, UpdateState}; @@ -52,7 +53,7 @@ use crate::view_policy::{FileDescriptor, ViewMode, applicable_modes, default_mod use annotation::{AnnotationEditorState, AnnotationListState}; use crossterm::event::{KeyCode, KeyEvent, KeyModifiers, MouseButton, MouseEvent, MouseEventKind}; use lineselect::LineSelectState; -use ratatui::layout::Position; +use ratatui::layout::{Position, Rect}; use ratatui::text::Text; use std::collections::{BTreeMap, HashMap}; use std::io; @@ -181,6 +182,95 @@ pub struct RenderResult { /// where no per-display-line source exists) and for providers that don't supply it; the /// copy paths then fall back to display-text extraction. pub source: Option>, + /// The still PNG payload for a media file in `Media` mode, produced off-thread so the pane + /// can show the image via the graphics host. `None` for every non-media mode and whenever + /// media rendering degraded to the text placeholder. + pub media: Option, +} + +/// What the graphics host currently displays (the Media view's clear/set discipline). Comparing +/// this against the freshly-computed desired value after each draw is the single point that makes +/// "an image is on screen exactly when and where it should be" true for selection change, mode +/// change, scroll, resize, zoom, and overlay-open alike — no call site has to remember to clear. +#[derive(Debug, Clone, PartialEq, Eq)] +struct MediaShown { + path: PathBuf, + mode: ViewMode, + placement: Placement, +} + +/// The live video player for the selected video: the ffmpeg decoder thread and the playback +/// clock. While it exists it OWNS the graphics surface — [`Controller::sync_media`] yields to it, +/// and [`Controller::tick_media`] sends frames. Paused playback keeps the last decoded frame up +/// but stops stepping. Seeking re-spawns the decoder at a new offset. +struct MediaPlayer { + /// The video being played (the canonicalized in-root path handed to ffmpeg). + path: PathBuf, + playing: bool, + /// Current playback offset, seconds — the `-ss` seek target for the next spawn. + position: f64, + decoder: crate::media::player::Decoder, + /// When the next frame may be pulled (pacing; see [`FRAME_INTERVAL`](crate::media::player::FRAME_INTERVAL)). + next_frame_at: Instant, +} + +impl MediaPlayer { + /// Pull the newest ready frame and send it to the graphics host, updating `media_shown` so + /// the `sync_media` discipline treats the playing frame as the current state (no clear-then-set + /// fight over the same placement). Returns whether a frame was actually sent (a redraw tell). + fn pull_and_show( + &mut self, + graphics: &mut Box, + media_shown: &mut Option, + rect: Option, + metrics: Option, + natural: Option<(u32, u32)>, + ) -> bool { + let Some(frame) = self.decoder.next_ready() else { + return false; + }; + let Some((w, h)) = crate::media::png_dimensions(&frame.png) else { + return false; // not a frame we can place; wait for the next one + }; + // Recompute the placement from THIS frame's pixels against the live content box, so + // playback fills the pane exactly as the still did. Reusing the still's placement made + // video render at the preview's aspect (and, before a still existed, at a 1x1 cell); and + // falling back to the still's rect on a resize would leave playback stuck at the old size. + let placement = match (rect, metrics) { + (Some(rect), Some(metrics)) if rect.width > 0 && rect.height > 0 => { + // Size from the VIDEO's own resolution, not this frame's: frames are decoded + // small to fit the host's byte cap, and `fit` never upscales, so using the frame + // size would render playback as a fraction of the pane. + crate::media::fit(natural.unwrap_or((w, h)), metrics, rect) + } + // No geometry yet (no graphics host, or before the first draw): keep whatever the + // still established rather than inventing a placement. + _ => match media_shown.as_ref() { + Some(shown) => shown.placement, + None => return false, + }, + }; + graphics.send(crate::graphics::GraphicsCommand::Show(Box::new( + crate::graphics::Frame { + format: crate::graphics::Format::Png, + width: w, + height: h, + data: frame.png.clone(), + placement, + }, + ))); + *media_shown = Some(MediaShown { + path: self.path.clone(), + mode: ViewMode::Media, + placement, + }); + true + } + + /// Stop the decoder and join its thread. + fn stop(mut self) { + self.decoder.stop(); + } } /// Produce the content-pane text for `(file, mode)`. `Send` so a later task can run it on a @@ -207,6 +297,10 @@ pub trait ContentProvider: Send { /// every mode except Diff/FullDiff. /// /// `width` remains the markdown wrap width (gated by the markdown wrap preference). + // Seven render inputs, each independently optional and each meaningful to exactly one view + // mode. Bundling them into a struct would only move the same list behind a name that no + // caller shares, so the parameter list stays explicit. + #[allow(clippy::too_many_arguments)] fn render_at_width( &self, path: &Path, @@ -215,10 +309,12 @@ pub trait ContentProvider: Send { width: Option, pane_width: Option, diff_render_mode: DiffRenderMode, + media_box: Option<(u32, u32)>, ) -> RenderResult { let _ = width; let _ = pane_width; let _ = diff_render_mode; + let _ = media_box; self.render(path, mode, raw_diff) } } @@ -367,6 +463,10 @@ struct RenderJob { pane_width: Option, /// Which command a Diff/FullDiff render delegates to (`D`). Ignored by other modes. diff_render_mode: DiffRenderMode, + /// The picture area in PIXELS at dispatch (the content box minus the caption row, times the + /// host's cell metrics). Lets the Media render resample to the size actually shown instead of + /// guessing. `None` before the first draw, or with no graphics host. + media_box: Option<(u32, u32)>, } /// A re-root's off-thread git result: the working-tree status (tree markers, AC-7) and the @@ -634,6 +734,26 @@ pub struct Controller { /// cleared in lockstep with `content` (`poll` / `clear_content`), so the copy paths can trust /// that when it is `Some`, index `n-1` IS displayed line `n`'s source. content_source: Option>, + /// The still PNG payload of the currently displayed content, for the Media view. Applied + /// from [`RenderResult::media`] in [`poll`](Self::poll) in lockstep with `content` (empty for + /// non-media modes and for media that degraded to the text placeholder). Cleared wherever + /// `content_source` is cleared so stale pixels can never paint over newer content. + content_media: Option, + /// The graphics host sink: images are handed to the host through this non-blocking outbox. + /// `NullSink` until [`set_graphics`](Self::set_graphics) injects the live host. Session-level. + graphics: Box, + /// The pixel size of one terminal cell, from the host's `pane.graphics.info`. Needed to turn + /// the pane's cell geometry into a pixel budget for placement; `None` until the host answered + /// (or no host — media then degrades to the text line). + cell_metrics: Option, + /// What the graphics host currently shows, or `None` when nothing is shown. Recomputed after + /// every draw: if the desired value differs, the controller clears then sets (and clears alone + /// when it is now `None`), so a stale image can never survive a selection/mode/scroll/resize/ + /// zoom/overlay change without those paths having to remember to clear. + media_shown: Option, + /// The live video player, `None` when no video is selected/played. While present it owns the + /// graphics surface (see [`MediaPlayer`]). + media_player: Option, /// The path of the file whose content is currently displayed in the pane — the title's /// source of truth, so the border label switches in lockstep with the body. `None` /// while no file's content has landed yet (launch, a re-root, or a directory/empty tree @@ -816,8 +936,13 @@ impl Controller { let renderers = renderers.unwrap_or_else(|| Renderers { markdown: vec!["herdr-no-such-markdown-renderer".into()], diff: vec!["herdr-no-such-diff-renderer".into()], + // Nothing to probe with, and nothing lost: the info line just omits codec and + // duration. + probe: Vec::new(), full_diff: vec!["herdr-no-such-full-diff-renderer".into()], syntax: vec!["herdr-no-such-syntax-renderer".into()], + image: vec!["herdr-no-such-image-renderer".into()], + video: vec!["herdr-no-such-video-renderer".into()], timeout: std::time::Duration::from_millis(100), }); let RootProviders { git, content } = providers(&resolved); @@ -872,6 +997,11 @@ impl Controller { content: Text::raw(""), content_notices: Vec::new(), content_source: None, + content_media: None, + graphics: Box::new(NullSink), + cell_metrics: None, + media_shown: None, + media_player: None, content_path: None, content_rendering: false, action_notice: None, @@ -968,12 +1098,14 @@ impl Controller { job.wrap_width, job.pane_width, job.diff_render_mode, + job.media_box, ) })) .unwrap_or_else(|_| RenderResult { content: Text::raw("[content unavailable: renderer error]"), notices: vec!["the renderer failed unexpectedly; showing a placeholder".into()], source: None, + media: None, }); if result_tx.send((job.seq, result)).is_err() { break; // controller gone @@ -1341,6 +1473,315 @@ impl Controller { self.opener = Some(opener); } + /// Inject the graphics host outbox + the measured cell metrics (from the host's + /// `pane.graphics.info`). `cell_metrics: None` means "no host / not measurable" — the Media + /// view then shows its text line and nothing is placed. Session-level (the host doesn't change + /// when the tree does). Injected post-construction so tests inject a recorder instead. + pub fn set_graphics( + &mut self, + graphics: Box, + cell_metrics: Option, + ) { + self.graphics = graphics; + self.cell_metrics = cell_metrics; + } + + /// The image the Media view should currently place, or `None` when nothing belongs on screen. + /// Derived from already-applied state (`content_path`/`content_media`/`effective_mode`) plus + /// the last measured layout (`geom.content_inner`) — so "what should be shown" needs no + /// bookkeeping of its own: a selection that moved off media, a `Tab` off Media, a zoom/resize + /// that shrank or hid the content column, an overlay that covered it — all produce a different + /// desired value (or `None`) automatically. + /// The cell rect the picture occupies: the content box with its TOP ROW RESERVED for the + /// info caption. Without this the image is placed over its own caption and hides it. + fn media_rect(inner: Rect) -> Option { + (inner.width > 0 && inner.height > 1).then(|| Rect { + x: inner.x, + y: inner.y + 1, + width: inner.width, + height: inner.height - 1, + }) + } + + /// [`media_rect`](Self::media_rect) in pixels, for the render worker's resample target. + fn media_box_px(&self) -> Option<(u32, u32)> { + let metrics = self.cell_metrics?; + let rect = Self::media_rect(self.geom.content_inner?)?; + Some(( + rect.width as u32 * metrics.cell_width_px, + rect.height as u32 * metrics.cell_height_px, + )) + } + + fn desired_media(&self) -> Option { + // The content must be media IN Media mode. + let path = self.content_path.clone()?; + if self.effective_mode(&path) != ViewMode::Media { + return None; + } + let media = self.content_media.as_ref()?; + // A payload the host would reject is not "shown" — bail here so `media_shown` never + // records a picture that was never sent. The render worker downscales over-cap images + // (`render::fit_under_cap`), so reaching this is the give-up case, which arrives with its + // own notice already on the text line. + if media.png.len() > crate::graphics::MAX_IMAGE_BYTES { + return None; + } + // Placement is decided from the SOURCE's size, not the transmitted copy's: a picture that + // was shrunk only to satisfy the host's byte cap must still be shown at pane size, or + // `fit`'s never-upscale rule renders it as a small island in a large pane. + let (width, height) = media.natural; + // And the layout must give us a non-empty pixel budget. + let metrics = self.cell_metrics?; + let rect = Self::media_rect(self.geom.content_inner?)?; + Some(MediaShown { + path, + mode: ViewMode::Media, + placement: crate::media::fit((width, height), metrics, rect), + }) + } + + /// The clear/set discipline: after each draw, compare what the host SHOULD show against what + /// it DOES show, and issue `clear()` + `set()` on a difference (or `clear()` alone when the + /// answer became `None`). One comparison replaces a directory of "remember to clear" call + /// sites; correctness for selection change, mode change, scroll, resize, zoom, and overlay-open + /// all falls out of the desired-value being a function of the current state. The host sink is + /// non-blocking (its worker does the ~150 ms socket round-trips), so calling this per-draw is + /// cheap. Sending through the last-wins worker collapse means a superseded Show is dropped, not + /// queued. + pub fn sync_media(&mut self) { + // While a video is actively playing, the player owns the graphics surface. It already + // sends a frame every `FRAME_INTERVAL` with a placement derived from that frame, so + // running the still-image discipline as well makes the two alternate — frame, + // Hide+Show(poster), frame, Hide+Show(poster) — which doubles the socket traffic and + // flickers the pane between the poster and the video. Measured before this guard: ~19 + // sets/second against an 8 fps pacer. + // + // Scoped to a player playing THIS file: a stale player must never wedge the surface for + // a file the user has already moved on from. + if let Some(player) = self.media_player.as_ref() + && player.playing + && Some(&player.path) == self.content_path.as_ref() + { + return; + } + let desired = self.desired_media(); + if desired == self.media_shown { + return; // unchanged — no redundant set on every idle draw (AC-27-test parity) + } + self.graphics.send(crate::graphics::GraphicsCommand::Hide); + if let Some(shown) = &desired + && let Some(media) = self.content_media.as_ref() + { + // `desired_media` has already established the payload is under the host's cap and + // parses as a PNG, so there is nothing left to re-check here: a Some(desired) always + // sends. + let (w, h) = + crate::media::png_dimensions(&media.png).unwrap_or((shown.placement.grid_cols, 1)); + self.graphics + .send(crate::graphics::GraphicsCommand::Show(Box::new( + crate::graphics::Frame { + format: crate::graphics::Format::Png, + width: w, + height: h, + data: media.png.clone(), + placement: shown.placement, + }, + ))); + } + self.media_shown = desired; + } + + /// Teardown: tell the host to show nothing and block until it has. Called from `run`'s exit + /// path (beside `suspend_tui`) so a quit never leaves a residual graphic in the pane; the + /// editor hand-off already forces an `Effects::clear` on its own path. + pub fn clear_media(&mut self) { + if let Some(p) = self.media_player.take() { + p.stop(); + } + self.media_shown = None; + self.graphics.close(); + } + + // -- video playback ---------------------------------------------------------------- + + /// How far one `{`/`}` seek steps, in seconds. Roughly a tenth of a typical short clip and a + /// comfortable nudge on a longer one — small enough to be precise, big enough to be visible. + const MEDIA_SEEK_STEP: f64 = 5.0; + /// Playback default rate handed to ffmpeg's `-r` when no frame is pacing (the tick paces each + /// frame against [`FRAME_INTERVAL`](crate::media::player::FRAME_INTERVAL)); the stream rate is + /// informational for the decoder, which outputs whenever it can. + const MEDIA_FPS: u32 = 8; + + /// Whether a video is selected and Media mode is active — media intents are inert otherwise. + fn media_active(&self) -> Option { + let path = self.content_path.clone()?; + if self.effective_mode(&path) != ViewMode::Media + || self.content_media.as_ref().map(|m| m.kind) != Some(crate::media::MediaKind::Video) + { + return None; + } + Some(path) + } + + /// Build the decoder command for `path` starting at `position` seconds: the configured + /// `video` renderer template with `{start}/{fps}/{width}/{height}` substituted, `{name}` + /// replaced with the (canonical, in-root) path, and the pixel budget from the pane geometry. + /// The geometry must be known — the first draw has to have run. + fn media_command(&self, path: &Path, position: f64) -> Vec { + let metrics = self.cell_metrics.unwrap_or(CellMetrics { + cell_width_px: 10, + cell_height_px: 20, + }); + // The caption row is reserved here too, so the decoder's target matches the still's and + // the placement's exactly — otherwise pressing play visibly resized the video. + let rect = self + .geom + .content_inner + .and_then(Self::media_rect) + .unwrap_or(Rect { + x: 0, + y: 1, + width: 80, + height: 23, + }); + let (width, height) = crate::media::frame_budget(rect, metrics); + let substituted = crate::media::player::substitute( + &self.renderers.video, + &position.to_string(), + &Self::MEDIA_FPS.to_string(), + &width.to_string(), + &height.to_string(), + ); + crate::render::with_video_name(&substituted, &path.to_string_lossy()) + } + + /// Start (or restart) the player for the current video at `position`. Spawns the decoder and, + /// unless `playing`, shows the first frame without stepping (paused-on-frame-0 semantics: a + /// selection never starts motion unasked). + fn media_start(&mut self, position: f64, playing: bool) { + let Some(path) = self.media_active() else { + return; + }; + if let Some(p) = self.media_player.take() { + p.stop(); + } + let command = self.media_command(&path, position); + let Some(decoder) = crate::media::player::Decoder::spawn(&command) else { + return; + }; + // A restart frames the desired absolute state as a Hide + Show so the host cannot leave a + // stale frame of the previous segment under the new one. + self.graphics.send(crate::graphics::GraphicsCommand::Hide); + let mut player = MediaPlayer { + path, + playing, + position, + decoder, + next_frame_at: Instant::now(), + }; + if !playing { + let (rect, metrics) = (self.geom.content_inner, self.cell_metrics); + let natural = self.content_media.as_ref().map(|m| m.natural); + player.pull_and_show( + &mut self.graphics, + &mut self.media_shown, + rect, + metrics, + natural, + ); + } + self.media_player = Some(player); + } + + /// `Space` on a selected video: play if paused/idle, pause if playing. + fn media_play_pause(&mut self) -> Effects { + match &mut self.media_player { + Some(p) => { + p.playing = !p.playing; + // Restart the pacing clock so a resume doesn't burst past a stale deadline. + p.next_frame_at = Instant::now(); + Some(Effects::redraw()) + } + None => { + self.media_start(0.0, true); + None + } + } + .unwrap_or_else(Effects::noop) + } + + /// `{` / `}`: seek back/forward by [`MEDIA_SEEK_STEP`], re-spawning the decoder at the new + /// offset (you cannot `-ss`-seek the pipe of a running ffmpeg). Keeps play state. + fn media_seek(&mut self, forward: bool) -> Effects { + let position = match &self.media_player { + Some(p) => p.position, + None => 0.0, + }; + let delta = if forward { + Self::MEDIA_SEEK_STEP + } else { + -Self::MEDIA_SEEK_STEP + }; + self.media_seek_to((position + delta).max(0.0)) + } + + /// Seek to an absolute offset; `0` restarts. Preserves the playing state across the spawn. + fn media_seek_to(&mut self, position: f64) -> Effects { + let playing = self + .media_player + .as_ref() + .map(|p| p.playing) + .unwrap_or(true); + self.media_start(position, playing); + Effects::redraw() + } + + /// Advance playback: pull at most one frame per [`FRAME_INTERVAL`](crate::media::player::FRAME_INTERVAL), + /// then hand it to the graphics host. Mirrors the `tick_flash` idiom — called each run-loop + /// tick, returns whether a redraw is owed (a frame changed). When the decoder finishes, stop + /// the player (the last frame stays; the text placeholder remains beneath). + pub fn tick_media(&mut self, now: Instant) -> bool { + // Read the live geometry first: `player` borrows `self.media_player` mutably below. + let (rect, metrics) = (self.geom.content_inner, self.cell_metrics); + let natural = self.content_media.as_ref().map(|m| m.natural); + let Some(player) = self.media_player.as_mut() else { + return false; + }; + if player.decoder.finished() { + // Decoder drained: no more frames will come. Fall back to the sync_media discipline, + // which will clear (the video's still preview text remains) — and drop the player + // so `p`/seeks no longer nop into a dead decoder. + let ended = player.playing; + if let Some(p) = self.media_player.take() { + p.stop(); + } + return ended; + } + if !player.playing { + return false; + } + if now < player.next_frame_at { + return false; // paced: one frame per FRAME_INTERVAL, no bursts + } + player.next_frame_at = now + crate::media::player::FRAME_INTERVAL; + player.pull_and_show( + &mut self.graphics, + &mut self.media_shown, + rect, + metrics, + natural, + ) + } + + /// Stop the player (a selection change / mode change / re-root / quit) — kills the ffmpeg + /// process and clears the surface so no stale frame survives the transition. + fn media_stop(&mut self) { + if let Some(p) = self.media_player.take() { + p.stop(); + } + } + /// Install the effective key bindings resolved from the registry + the config's `[keys]` table, /// plus the resolver's [`KeyLoadOutcome`](crate::input::KeyLoadOutcome) (Slice B, T-6). Called /// once by `app::run` after construction (mirrors [`set_settings_display`](Self::set_settings_display)); @@ -1964,6 +2405,10 @@ impl Controller { } }, Intent::ShowHelp => self.open_help(), + Intent::MediaPlayPause => self.media_play_pause(), + Intent::MediaSeekBack => self.media_seek(false), + Intent::MediaSeekForward => self.media_seek(true), + Intent::MediaRestart => self.media_seek_to(0.0), Intent::Close => self.close_or_unzoom(), } } @@ -2959,6 +3404,9 @@ impl Controller { let reflow = match mode { ViewMode::RenderedMarkdown => self.effective_wrap(), ViewMode::Diff | ViewMode::FullDiff => self.diff_render_mode != DiffRenderMode::Raw, + // Media resamples to the pane's pixel box, so a resize changes the right answer; + // without this the picture keeps whatever resolution the old layout asked for. + ViewMode::Media => true, ViewMode::SyntaxContent => false, }; if reflow { @@ -2978,6 +3426,9 @@ impl Controller { self.latest_seq += 1; let seq = self.latest_seq; self.reflow_seq = Some(seq); + // A resize changes the frame budget; restart playback at the same offset under the new + // geometry rather than leaving a stale-sized decoder running. + self.media_stop(); let rel = self.rel(&path); // Status mode always diffs the working tree, so a reflow must use the SAME forced // `Baseline::Head` `dispatch_render` does — otherwise a resize/wrap re-render on a @@ -3003,6 +3454,7 @@ impl Controller { wrap_width: self.md_wrap_width(), pane_width: self.pane_width(), diff_render_mode: self.diff_render_mode, + media_box: self.media_box_px(), }); } @@ -3014,6 +3466,8 @@ impl Controller { fn dispatch_render(&mut self) { self.latest_seq += 1; let seq = self.latest_seq; + // A new selection/mode invalidates any live video playback (it belongs to the old file). + self.media_stop(); // A fresh render means new content — start it at the top-left, never inheriting the // previous file's scroll offsets. self.content_scroll = 0; @@ -3089,12 +3543,14 @@ impl Controller { wrap_width: self.md_wrap_width(), pane_width: self.pane_width(), diff_render_mode: self.diff_render_mode, + media_box: self.media_box_px(), }) .is_ok() { self.content = Text::raw("Rendering\u{2026}"); self.content_notices.clear(); self.content_source = None; // the placeholder has no source; the landing render brings its own + self.content_media = None; // and no stale pixels; the landing render brings the fresh payload self.content_rendering = true; } } @@ -3107,9 +3563,11 @@ impl Controller { self.content = Text::raw(reason.label()); self.content_notices.clear(); self.content_source = None; // guidance text has no source behind it + self.content_media = None; // ... or pixels (Media view) behind it + self.media_stop(); // no file → no live video player // No file content is displayed for a directory/empty tree, and no render is in flight // (this path sends no `RenderJob`), so the title falls back to the selected node's name - //. + // self.content_path = None; self.content_rendering = false; } @@ -3135,6 +3593,7 @@ impl Controller { self.content_selection = None; self.content_notices = result.notices; self.content_source = result.source; // in lockstep with `content` (copy fidelity) + self.content_media = result.media; // in lockstep with `content` (pixels follow text) self.applied_seq = seq; // the displayed content is now this render (go-to-line guard) // A reflow keeps the user's scroll position, but the reflowed body may have a // different rendered-row count (a table re-lays-out at the new width), so re-clamp @@ -3253,6 +3712,7 @@ impl Controller { path: path.to_path_buf(), is_markdown: is_markdown(path), is_changed: self.is_changed(path), + media: crate::media::MediaKind::from_path(path), } } @@ -3396,6 +3856,7 @@ mod tests { content: Text::raw(""), notices: Vec::new(), source: None, + media: None, } } } @@ -3409,6 +3870,7 @@ mod tests { content: Text::raw(body), notices: Vec::new(), source: Some((1..=40).map(|i| format!("body line {i}")).collect()), + media: None, } } } diff --git a/src/help.rs b/src/help.rs index d93d276e..9a0649e1 100644 --- a/src/help.rs +++ b/src/help.rs @@ -287,7 +287,8 @@ pub fn settings_text( tree_position = {tree_position}\n\ tree_max_cols = {tree_max_cols}\n\ preview_max_lines = {preview_max_lines}\n\ - preview_max_kib = {preview_max_kib}", + preview_max_kib = {preview_max_kib}\n\ + media_max_kib = {media_max_kib}", open = open, reveal = reveal, hide_dotfiles = eff.hide_dotfiles, @@ -301,6 +302,7 @@ pub fn settings_text( tree_max_cols = eff.tree_max_cols, preview_max_lines = eff.preview_max_lines, preview_max_kib = eff.preview_max_kib, + media_max_kib = eff.media_max_kib, ) } @@ -795,6 +797,9 @@ mod tests { markdown: Some(vec!["glow".to_string(), "-w".to_string(), "80".to_string()]), diff: None, syntax: None, + image: None, + video: None, + media_max_kib: 8192, open: None, reveal: None, hide_dotfiles: true, @@ -846,6 +851,7 @@ mod tests { "tree_max_cols", "preview_max_lines", "preview_max_kib", + "media_max_kib", ] { assert!( text.contains(key), diff --git a/src/input.rs b/src/input.rs index 2c459689..0bf938ec 100644 --- a/src/input.rs +++ b/src/input.rs @@ -489,6 +489,34 @@ pub(crate) const REGISTRY: &[Binding] = &[ description: "Scroll the tree pane right.", category: "View & layout", }, + Binding { + intent: Intent::MediaPlayPause, + name: "media_play_pause", + default_keys: &[KeyCode::Char('p')], + description: "Toggle play/pause of the selected video.", + category: "View & layout", + }, + Binding { + intent: Intent::MediaSeekBack, + name: "media_seek_back", + default_keys: &[KeyCode::Char('{')], + description: "Seek the selected video back.", + category: "View & layout", + }, + Binding { + intent: Intent::MediaSeekForward, + name: "media_seek_forward", + default_keys: &[KeyCode::Char('}')], + description: "Seek the selected video forward.", + category: "View & layout", + }, + Binding { + intent: Intent::MediaRestart, + name: "media_restart", + default_keys: &[KeyCode::Char('0')], + description: "Restart the selected video from the beginning.", + category: "View & layout", + }, Binding { intent: Intent::ShowHelp, name: "show_help", @@ -822,6 +850,10 @@ mod tests { (KeyCode::Char('['), Intent::PrevChanged), (KeyCode::Char('H'), Intent::TreeScrollLeft), (KeyCode::Char('L'), Intent::TreeScrollRight), + (KeyCode::Char('p'), Intent::MediaPlayPause), + (KeyCode::Char('{'), Intent::MediaSeekBack), + (KeyCode::Char('}'), Intent::MediaSeekForward), + (KeyCode::Char('0'), Intent::MediaRestart), (KeyCode::Char('O'), Intent::OpenWithApp), (KeyCode::Char('R'), Intent::RevealInFileManager), (KeyCode::Char('y'), Intent::CopyRepoPath), diff --git a/src/intent.rs b/src/intent.rs index e22cb3cc..ab85e10c 100644 --- a/src/intent.rs +++ b/src/intent.rs @@ -155,6 +155,17 @@ pub enum Intent { /// [`Intent::TreeScrollLeft`] it only moves the in-pane scroll; no mutation. Bound to `L` /// (Shift+`l`) only — no event hook (AC-N6). Inert unless the tree is focused. TreeScrollRight, + /// Toggle play/pause of the currently selected video. Inert no-op unless a video is selected + /// in Media mode. Read-only: playback decodes frames, never mutates the file (AC-N1/N3). + MediaPlayPause, + /// Seek the selected video back by a fixed step. Inert no-op unless a video is selected. + /// Read-only navigation of an in-memory playback position. + MediaSeekBack, + /// Seek the selected video forward by a fixed step. Mirror of [`Intent::MediaSeekBack`], + /// same read-only guarantees. + MediaSeekForward, + /// Restart the selected video from its beginning. Read-only, like the other media intents. + MediaRestart, /// Close the viewer and return control to the prior pane (AC-20). Close, } @@ -162,7 +173,7 @@ pub enum Intent { impl Intent { /// Every intent variant — lets the dispatcher and tests enumerate the closed set so /// keyboard-completeness (AC-18) and the no-file/git-mutation invariant (AC-N3) stay checkable. - pub const ALL: [Intent; 41] = [ + pub const ALL: [Intent; 45] = [ Intent::NavUp, Intent::NavDown, Intent::PageUp, @@ -202,6 +213,10 @@ impl Intent { Intent::PrevChanged, Intent::TreeScrollLeft, Intent::TreeScrollRight, + Intent::MediaPlayPause, + Intent::MediaSeekBack, + Intent::MediaSeekForward, + Intent::MediaRestart, Intent::ShowHelp, Intent::Close, ]; @@ -258,6 +273,10 @@ mod tests { | Intent::PrevChanged | Intent::TreeScrollLeft | Intent::TreeScrollRight + | Intent::MediaPlayPause + | Intent::MediaSeekBack + | Intent::MediaSeekForward + | Intent::MediaRestart | Intent::ShowHelp | Intent::Close => (false, false), }; @@ -332,11 +351,11 @@ mod tests { } #[test] - fn all_length_is_41() { + fn all_length_is_45() { assert_eq!( Intent::ALL.len(), - 41, - "Intent::ALL must have exactly 41 variants" + 45, + "Intent::ALL must have exactly 45 variants" ); } diff --git a/src/render.rs b/src/render.rs index 135843db..e28e957b 100644 --- a/src/render.rs +++ b/src/render.rs @@ -180,6 +180,18 @@ pub struct Renderers { /// show a line-number gutter, so the file's lines are numbered with the diff shown inline. pub full_diff: Vec, pub syntax: Vec, + /// Converts a non-PNG image file (fed on **stdin** as raw bytes) to PNG on stdout, for the + /// Media view (defaults to ffmpeg). Absent ⇒ the Media view shows its placeholder + notice. + pub image: Vec, + /// Extracts video frames as PNG. A template: `{start}` / `{width}` / `{height}` / `{fps}` + /// are substituted before use (the file path is passed as an argv element — you cannot + /// seek a pipe, the one narrowing of the stdin trust boundary; see ARCHITECTURE.md). + /// Absent ⇒ video shows its placeholder + notice. + pub video: Vec, + /// Reports a video's codec and duration for the Media info line (defaults to ffprobe). + /// `{name}` is substituted with the file path. Purely informational: an empty vec, a missing + /// binary, or an unparsable answer just omits those fields — it never blocks playback. + pub probe: Vec, /// Per-invocation wall-clock bound; a renderer exceeding it is killed and the plain- /// text fallback is used, so a wedged delegate can never hang rendering. pub timeout: Duration, @@ -244,6 +256,7 @@ pub fn render( base_notice, ), ViewMode::Diff | ViewMode::FullDiff => unreachable!("handled above"), + ViewMode::Media => unreachable!("media is rendered by render_media, not render"), } } @@ -310,13 +323,22 @@ fn markdown_section_timeout(fallback: Text<'static>) -> (Text<'static>, Option Vec { +pub(crate) fn with_name(command: &[String], name: &str) -> Vec { command .iter() .map(|arg| arg.replace("{name}", name)) .collect() } +/// Substitute the `{name}` (the file PATH, `'{}`-canonicalized in-root) into the video frame +/// decoder's argv template — in addition to the player's `{start}/{fps}/{width}/{height}` +/// substitution. This is the one deliberate narrowing of the stdin trust boundary: you cannot +/// `-ss`-seek a pipe, so the canonicalized in-root path is passed as its own argv element, no +/// shell. `name` must already be the sanitized basename-or-path the caller controls. +pub(crate) fn with_video_name(command: &[String], name: &str) -> Vec { + with_name(command, name) +} + /// Bound a text block to the size cap, returning a preview plus a truncation notice when /// it exceeds it. Used for diff text (AC-13's bound applied to large diffs, keeping the /// UI path responsive regardless of how big a changed file's diff is). @@ -451,6 +473,7 @@ fn capability(mode: ViewMode) -> &'static str { ViewMode::FullDiff => "Full-file diff", ViewMode::RenderedMarkdown => "Markdown", ViewMode::SyntaxContent => "Syntax", + ViewMode::Media => "Media", } } @@ -480,6 +503,19 @@ fn run_renderer( run_renderer_until(command, input, Instant::now() + timeout) } +/// A binary-in, binary-out renderer call (the media converters — `image`/`video` commands). The +/// input is fed on stdin as raw bytes and stdout is returned raw, so a PNG pipeline is never +/// round-tripped through lossy UTF-8. Same deadline / output-cap / kill-and-reap guarantees as +/// [`run_renderer_until`]; the caller owns the byte cap (the media size cap, not the text one). +pub(crate) fn run_renderer_bytes( + command: &[String], + input: &[u8], + timeout: Duration, +) -> Result, String> { + run_renderer_bytes_until(command, input, Instant::now() + timeout) + .map_err(|e| e.notice(capability(ViewMode::Media))) +} + /// Spawn a renderer, feed `input` on stdin (writer thread, avoiding a pipe deadlock), then capture /// stdout on a reader thread through the caller's absolute deadline. /// @@ -492,6 +528,18 @@ fn run_renderer_until( input: &str, deadline: Instant, ) -> Result { + run_renderer_bytes_until(command, input.as_bytes(), deadline) + .map(|buf| String::from_utf8_lossy(&buf).into_owned()) +} + +/// The byte-oriented core shared by [`run_renderer_until`] (text, lossy-decoded) and +/// [`run_renderer_bytes`] (binary pipelines): spawn, stdin write on a writer thread, stdout +/// capture on a reader thread, one deadline, unconditional kill-and-reap on overrun. +fn run_renderer_bytes_until( + command: &[String], + input: &[u8], + deadline: Instant, +) -> Result, RendererError> { let prog = command .first() .cloned() @@ -523,7 +571,7 @@ fn run_renderer_until( if let Some(mut stdin) = child.stdin.take() { let owned = input.to_owned(); std::thread::spawn(move || { - let _ = stdin.write_all(owned.as_bytes()); // ignore a closed pipe + let _ = stdin.write_all(&owned); // ignore a closed pipe }); } @@ -536,7 +584,7 @@ fn run_renderer_until( match rx.recv_timeout(deadline.saturating_duration_since(Instant::now())) { Ok(buf) => match crate::proc::wait_until(&mut child, deadline) { - Some(status) if status.success() => Ok(String::from_utf8_lossy(&buf).into_owned()), + Some(status) if status.success() => Ok(buf), Some(status) => Err(RendererError::Failed { detail: format!("exited with {status}"), }), @@ -558,6 +606,483 @@ fn capture_renderer_output(stdout: impl Read) -> Vec { buf } +// --------------------------------------------------------------------------- +// Media: the still-preview payload for the Media view mode +// --------------------------------------------------------------------------- + +/// The default media size cap, in bytes (8 MiB) — mirror of `crate::config::DEFAULT_MEDIA_MAX_KIB` +/// (8192). Far larger than the 1 MiB text-preview budget: images are naturally big, and the +/// byte-bound here only gates the disk read so a giant/hostile file is never slurped whole. +pub const DEFAULT_MEDIA_MAX_BYTES: u64 = 8192 * 1024; + +/// The still preview for a media file, ready to hand to the graphics host. +/// +/// Carries **raw PNG bytes**; base64 happens in `graphics.rs` at send time so the payload stays +/// bytes. Dimensions are parsed at placement time via [`crate::media::png_dimensions`], so a +/// re-encode (the PNG fast-path guard) can decide from the actual bytes. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct MediaPayload { + pub kind: crate::media::MediaKind, + pub png: Vec, + /// The source's OWN pixel size, before any resample for display or for the host's byte cap. + /// + /// The placement maths must not use `png`'s dimensions: those may have been shrunk purely to + /// get under the cap, and `fit`'s never-upscale rule would then render a large photo smaller + /// than the pane just because it had to travel small. The "is this image smaller than the + /// box?" question is about the SOURCE, so it is answered with this. + pub natural: (u32, u32), +} + +/// Produce the Media view's content for a media file: a text line (so the pane is never blank — +/// the no-graphics degradation is automatic) plus, when a PNG was obtained, the payload. +/// +/// `media_max_bytes` is the dedicated media size cap (the byte-bound on the disk read and on the +/// captured output); it is deliberately separate from the text-preview cap. A missing converter, +/// an over-cap file, or a malformed result degrades to the text line plus a notice — never a +/// crash. `Png` needs no converter: the file's own bytes are the payload (the hosting layer still +/// applies the PNG fast-path guard at placement time). `Video` decodes frame 0 only here; playback +/// is the controller's, elsewhere. +pub fn render_media( + renderers: &Renderers, + path: &Path, + kind: crate::media::MediaKind, + media_max_bytes: u64, + media_box: Option<(u32, u32)>, +) -> (Text<'static>, Option, Option) { + match kind { + crate::media::MediaKind::Png => { + let bytes = read_media_bytes(path, media_max_bytes); + match bytes.and_then(|b| crate::media::png_dimensions(&b).map(|(w, h)| (b, w, h))) { + // The fast path is only fast when the bytes are actually sendable: a PNG over the + // host's cap is re-encoded smaller rather than silently dropped. The text line + // keeps reporting the file's TRUE dimensions — the downscale is a transport + // detail, not something the user asked for. + Some((png, w, h)) => { + let colour = crate::media::png_colour(&png); + let bytes = png.len() as u64; + // Resample to the size the pane will actually show BEFORE worrying about + // bytes. Downscaling to the display box is free visually (those pixels can + // never be seen) and usually lands under the cap on its own, so the picture + // is resampled once, with a good filter, instead of being squeezed by a + // byte-ratio guess that ignores how large the pane is. + let png = to_display_box(renderers, png, (w, h), media_box); + match fit_under_cap(renderers, png, (w, h)) { + Some(fitted) => ( + info_line( + "image", + (w, h), + Some("PNG"), + colour.as_deref(), + bytes, + None, + fitted.rescaled_to, + ), + None, + Some(MediaPayload { + kind, + png: fitted.png, + natural: (w, h), + }), + ), + None => ( + info_line( + "image", + (w, h), + Some("PNG"), + colour.as_deref(), + bytes, + None, + None, + ), + Some(OVERSIZED_NOTICE.into()), + None, + ), + } + } + None => ( + Text::raw("[image: preview not shown]"), + Some("⚠ Image too large or unreadable.".into()), + None, + ), + } + } + crate::media::MediaKind::Image => { + let bytes = read_media_bytes(path, media_max_bytes); + // The conversion is also the first downscale opportunity: bounding it to a generous + // box here means a 12-megapixel JPEG usually lands under the cap in one pass instead + // of converting at full size and then needing a second re-encode. + let (box_w, box_h) = media_box.unwrap_or((DEFAULT_IMAGE_BOX, DEFAULT_IMAGE_BOX)); + let convert = with_image_size(&renderers.image, box_w, box_h, "lanczos"); + let on_disk = std::fs::metadata(path).map(|m| m.len()).unwrap_or(0); + let source_format = path + .extension() + .and_then(|e| e.to_str()) + .map(|e| e.to_ascii_uppercase()); + let Ok(png) = + run_renderer_bytes(&convert, &bytes.unwrap_or_default(), renderers.timeout) + else { + return ( + Text::raw("[image: preview not shown]"), + Some("⚠ The image converter is unavailable; see docs/renderers.md.".into()), + None, + ); + }; + match crate::media::png_dimensions(&png) { + Some((w, h)) => match fit_under_cap(renderers, png, (w, h)) { + Some(fitted) => ( + info_line( + "image", + (w, h), + source_format.as_deref(), + None, + on_disk, + None, + fitted.rescaled_to, + ), + None, + Some(MediaPayload { + kind, + png: fitted.png, + natural: (w, h), + }), + ), + None => ( + info_line( + "image", + (w, h), + source_format.as_deref(), + None, + on_disk, + None, + None, + ), + Some(OVERSIZED_NOTICE.into()), + None, + ), + }, + None => ( + Text::raw("[image: preview not shown]"), + Some("⚠ The image converter returned no image.".into()), + None, + ), + } + } + crate::media::MediaKind::Video => { + // Frame 0 only, for the still preview. Runs the video command with a fixed start; the + // width/height default to a conservative budget because the render worker has no pane + // geometry yet (playback — the decoder thread — sizes to the pane at tick time). + // The file path is substituted as its own argv element (no shell), so a hostile + // filename cannot inject — the canonicalized in-root path from `classify`'s caller. + let on_disk = std::fs::metadata(path).map(|m| m.len()).unwrap_or(0); + let probe = probe_video(renderers, path); + let command = with_video_name(&renderers.video, &path.to_string_lossy()); + // Identical to what `MediaPlayer` will ask the decoder for, so the poster frame and + // the first played frame are the same size — previously the still was hardcoded to + // 640x360 while playback used the pane budget, and the video jumped on play. + let (vw, vh) = crate::media::clamp_pixels_to_cap( + media_box.unwrap_or((DEFAULT_VIDEO_BOX.0, DEFAULT_VIDEO_BOX.1)), + ); + let command = crate::media::player::substitute( + &command, + "0", + "8", + &vw.to_string(), + &vh.to_string(), + ); + // The template streams by design, so the still preview adds a single-frame limit — + // and it MUST be inserted before the output URL, never appended. ffmpeg applies + // output options to the output that FOLLOWS them, so a trailing `-frames:v 1` is + // silently inert: measured on a real .m4v, appending produced 8 MB of concatenated + // frames (and on a longer video it simply ran until the renderer timeout, surfacing + // to the user as "the video decoder is unavailable"), while inserting produced one + // 84 KB frame. + let mut command = command; + let before_output = command.len().saturating_sub(1); + command.splice( + before_output..before_output, + ["-frames:v".to_string(), "1".to_string()], + ); + match run_renderer_bytes(&command, &[], renderers.timeout) { + Ok(png) => match crate::media::png_dimensions(&png) { + // A frame from a high-resolution source can still exceed the host's cap, so + // it goes through the same downscale ladder as a still image rather than + // being dropped at send time. + Some((w, h)) => match fit_under_cap(renderers, png, (w, h)) { + Some(fitted) => ( + // `p`, not Space: Space is already `page_down` in the registry, so + // the caption must name the key that actually plays. The size shown + // is the video's own, not the downscaled poster frame's. + info_line( + "video", + probe.native.unwrap_or((w, h)), + probe.codec.as_deref(), + None, + on_disk, + probe.duration_s, + fitted.rescaled_to, + ), + None, + Some(MediaPayload { + kind, + png: fitted.png, + // The VIDEO's own resolution, not the poster frame's. The frame is + // decoded small to stay under the host's byte cap, and `fit` never + // upscales — so using the frame's size here pinned video to a + // fraction of the pane while images, which report their true size, + // filled it. + natural: probe.native.unwrap_or((w, h)), + }), + ), + None => ( + info_line( + "video", + probe.native.unwrap_or((w, h)), + probe.codec.as_deref(), + None, + on_disk, + probe.duration_s, + None, + ), + Some(OVERSIZED_NOTICE.into()), + None, + ), + }, + None => ( + Text::raw("[video: preview not shown]"), + Some("⚠ The video decoder returned no frame.".into()), + None, + ), + }, + Err(_) => ( + Text::raw("[video: preview not shown]"), + Some("⚠ The video decoder is unavailable; see docs/renderers.md.".into()), + None, + ), + } + } + } +} + +/// The box a non-PNG image is converted into, per edge, when the pane size is not yet known +/// (the very first render, before a draw has measured the layout). +const DEFAULT_IMAGE_BOX: u32 = 1920; + +/// The same fallback for video, as a 16:9 box. +const DEFAULT_VIDEO_BOX: (u32, u32) = (1280, 720); + +/// Resample a PNG down to the size the pane will actually display, with a high-quality filter. +/// +/// This is the step that answers "why does a big image look worse than a small one": a picture +/// larger than the pane must be resampled *somewhere*, and doing it here — once, to the display +/// box, with `lanczos` — beats letting the byte-cap ladder shrink it by a blind ratio with a +/// hard-edged filter. Pixels the pane cannot show are not quality, so this loses nothing visible. +/// +/// Returns the input untouched when the pane size is unknown, when the image already fits the box, +/// or when the converter fails — every one of which is better served by the original bytes than by +/// no picture. +fn to_display_box( + renderers: &Renderers, + png: Vec, + dimensions: (u32, u32), + media_box: Option<(u32, u32)>, +) -> Vec { + let Some((box_w, box_h)) = media_box else { + return png; + }; + if box_w == 0 || box_h == 0 || (dimensions.0 <= box_w && dimensions.1 <= box_h) { + return png; // already no larger than the pane shows — the original IS the best version + } + let command = with_image_size(&renderers.image, box_w, box_h, "lanczos"); + match run_renderer_bytes(&command, &png, renderers.timeout) { + Ok(smaller) if crate::media::png_dimensions(&smaller).is_some() => smaller, + _ => png, + } +} + +/// What `ffprobe` told us about a video. Every field is optional: the probe is a convenience, and +/// its absence must never stop the frame from being shown. +#[derive(Default)] +struct VideoProbe { + codec: Option, + duration_s: Option, + /// The video's OWN resolution. Reported in the caption in place of the decoded preview's + /// size, so a video states its real dimensions exactly as an image does. + native: Option<(u32, u32)>, +} + +/// Ask the `probe` command for a video's codec and duration. +/// +/// Best-effort by construction — a missing ffprobe, a malformed answer, or a container it cannot +/// read all yield an empty [`VideoProbe`] and simply omit those fields from the info line. The +/// path is passed as its own argv element via `{name}` (no shell), the same narrowing of the +/// stdin trust boundary the `video` command already documents. +fn probe_video(renderers: &Renderers, path: &Path) -> VideoProbe { + if renderers.probe.is_empty() { + return VideoProbe::default(); + } + let command = with_video_name(&renderers.probe, &path.to_string_lossy()); + let Ok(out) = run_renderer_bytes(&command, &[], renderers.timeout) else { + return VideoProbe::default(); + }; + let text = String::from_utf8_lossy(&out); + let field = |name: &str| { + text.lines() + .filter_map(|l| l.split_once('=')) + .find(|(k, _)| k.trim() == name) + .map(|(_, v)| v.trim().to_string()) + }; + let width = field("width").and_then(|v| v.parse::().ok()); + let height = field("height").and_then(|v| v.parse::().ok()); + VideoProbe { + codec: field("codec_name").map(|c| c.to_ascii_uppercase()), + duration_s: field("duration").and_then(|v| v.parse::().ok()), + native: width.zip(height).filter(|&(w, h)| w > 0 && h > 0), + } +} + +/// The caption above a media file: what it is, at a glance. +/// +/// Reads as `[image: 3008×1546 · PNG · 8-bit RGBA · 655 KiB · shown at 2259×1161]`. The trailing +/// clause appears only when the host's byte cap forced a re-encode, which is the answer to "why +/// does this large file look softer than that small one" — without it the degradation is invisible +/// and looks like a bug. +#[allow(clippy::too_many_arguments)] +fn info_line( + label: &str, + dimensions: (u32, u32), + format: Option<&str>, + colour: Option<&str>, + on_disk: u64, + duration_s: Option, + rescaled_to: Option<(u32, u32)>, +) -> Text<'static> { + let (w, h) = dimensions; + let mut parts = vec![format!("{w}×{h}")]; + if let Some(d) = duration_s { + parts.push(crate::media::human_duration(d)); + } + if let Some(f) = format { + parts.push(f.to_string()); + } + if let Some(c) = colour { + parts.push(c.to_string()); + } + if on_disk > 0 { + parts.push(crate::media::human_size(on_disk)); + } + // Only worth saying when the size actually changed: a converter that returned the same + // dimensions (or a re-encode that only shrank bytes) would otherwise print a confusing + // "shown at" clause identical to the size right before it. + if let Some((rw, rh)) = rescaled_to.filter(|&r| r != dimensions) { + parts.push(format!("shown at {rw}×{rh}")); + } + if label == "video" { + parts.push("p to play".to_string()); + } + Text::raw(format!("[{label}: {}]", parts.join(" · "))) +} + +/// Shown when an image cannot be squeezed under the host's cap — the pane still shows the text +/// line, so this explains why no picture accompanies it. +const OVERSIZED_NOTICE: &str = "⚠ Image too large for the terminal to display; install ffmpeg so it can be scaled down. \ + See docs/renderers.md."; + +/// Substitute `{width}` / `{height}` / `{scaler}` in an image-converter command. +/// +/// Mirrors [`crate::media::player::substitute`] for the video template. Applied on **every** +/// invocation, so the placeholders are never passed through to ffmpeg literally. +pub(crate) fn with_image_size( + command: &[String], + width: u32, + height: u32, + scaler: &str, +) -> Vec { + let (w, h) = (width.to_string(), height.to_string()); + command + .iter() + .map(|arg| { + arg.replace("{width}", &w) + .replace("{height}", &h) + .replace("{scaler}", scaler) + }) + .collect() +} + +/// The re-encode ladder, best quality first: `(scaler, size multiplier)`. +/// +/// The goal is the **best-looking image that herdr will accept**, not merely one that fits. So we +/// start with the highest-fidelity resampler at the largest size the byte estimate allows and only +/// trade quality away when the host's cap forces it. +/// +/// Why the second rung changes filter rather than size: measured on this repo's 3008x1546 +/// screenshots, `lanczos` at the byte-target produced 794 KiB and `neighbor` at the *same* size +/// produced 350 KiB. For screenshots, diagrams, and UI captures, dropping the smoothing filter +/// costs nothing visually — it keeps text crisp — and buys more than halving the size, which is +/// far better than keeping a smooth filter and shrinking the picture. Photographs are the inverse +/// case, and they simply take the first rung when they fit. +const QUALITY_LADDER: &[(&str, f64)] = &[("lanczos", 1.0), ("neighbor", 1.0), ("neighbor", 0.7)]; + +/// The outcome of squeezing an image under the host's byte cap. +pub struct Fitted { + pub png: Vec, + /// `Some(dimensions)` when the bytes had to be re-encoded smaller, so the caller can say so in + /// the info line rather than letting the user wonder why a 3008px file looks softer than a + /// 900px one. `None` means the original bytes were sent untouched. + pub rescaled_to: Option<(u32, u32)>, +} + +/// Produce the best-quality version of `png` that herdr will accept. +/// +/// Returns the original bytes untouched when they already fit — the zero-subprocess fast path, and +/// the only path that is bit-for-bit pristine. Otherwise it walks [`QUALITY_LADDER`], stopping at +/// the first rung that lands under the cap, so the picture is only degraded as far as the host's +/// 512 KiB limit actually forces. +/// +/// `None` means the converter is missing, produced garbage, or no rung fit; the caller then shows +/// the caption plus [`OVERSIZED_NOTICE`] rather than sending a payload herdr would reject with +/// `image_too_large`. +fn fit_under_cap(renderers: &Renderers, png: Vec, dimensions: (u32, u32)) -> Option { + if png.len() <= crate::graphics::MAX_IMAGE_BYTES { + return Some(Fitted { + png, + rescaled_to: None, + }); + } + let (base_w, base_h) = + crate::media::downscale_target(dimensions, png.len(), crate::graphics::MAX_IMAGE_BYTES); + for (scaler, factor) in QUALITY_LADDER { + let w = ((base_w as f64 * factor).round() as u32).max(1); + let h = ((base_h as f64 * factor).round() as u32).max(1); + let command = with_image_size(&renderers.image, w, h, scaler); + // A failed rung is not fatal on its own — but a *missing converter* fails identically on + // every rung, so bailing out here avoids three pointless spawns. + let Ok(candidate) = run_renderer_bytes(&command, &png, renderers.timeout) else { + return None; + }; + let dims = crate::media::png_dimensions(&candidate)?; + if candidate.len() <= crate::graphics::MAX_IMAGE_BYTES { + return Some(Fitted { + png: candidate, + rescaled_to: Some(dims), + }); + } + } + None +} + +/// Read a media file's bytes, bounded by the media size cap (`None` if missing, unreadable, or +/// over the cap — the pane then shows the placeholder text rather than vomiting bytes). +fn read_media_bytes(path: &Path, media_max_bytes: u64) -> Option> { + let m = std::fs::metadata(path).ok()?; + if m.len() > media_max_bytes { + return None; + } + let mut file = File::open(path).ok()?; + let mut buf = Vec::with_capacity(m.len() as usize); + file.read_to_end(&mut buf).ok()?; + Some(buf) +} + /// Ingest (possibly untrusted) content into ratatui `Text`. Cursor-movement and /// screen-control escape sequences are stripped regardless of source; only SGR styling is /// kept and mapped into spans by `ansi-to-tui` (AC-27). The result can only ever paint the @@ -1104,4 +1629,322 @@ mod tests { "SGR 34 must map to the named Blue, not RGB" ); } + + // -- media -------------------------------------------------------------- + + /// A minimal PNG header (the 24-byte IHDR prefix) — enough for `png_dimensions` / the + /// fast-path payload without a full encoder. + fn png_bytes(w: u32, h: u32) -> Vec { + let mut b = vec![0x89, b'P', b'N', b'G', 0x0d, 0x0a, 0x1a, 0x0a]; + b.extend_from_slice(&13u32.to_be_bytes()); + b.extend_from_slice(b"IHDR"); + b.extend_from_slice(&w.to_be_bytes()); + b.extend_from_slice(&h.to_be_bytes()); + b + } + + #[test] + fn media_png_uses_the_files_own_bytes_as_the_payload() { + let p = tmp("media.png", &png_bytes(64, 48)); + let renderers = Renderers { + image: vec!["herdr-no-such-converter".into()], // must not be reached for PNG + ..cat_like() + }; + let (text, notice, media) = render_media( + &renderers, + &p, + crate::media::MediaKind::Png, + DEFAULT_MEDIA_MAX_BYTES, + None, + ); + assert_eq!(notice, None); + let line: String = text + .lines + .iter() + .flat_map(|l| l.spans.iter()) + .map(|s| s.content.as_ref()) + .collect(); + // The caption is now an info line: dimensions, format, and on-disk size. + assert_eq!(line, "[image: 64×48 · PNG · 24 B]"); + let payload = media.expect("a PNG payload"); + assert_eq!(payload.kind, crate::media::MediaKind::Png); + assert_eq!( + payload.png, + png_bytes(64, 48), + "native bytes, no conversion" + ); + fs::remove_file(&p).ok(); + } + + /// An over-cap PNG padded past herdr's 512 KiB limit. Sized from the real regression: the + /// repo's own `assets/File-Viewer-FS.png` is 655 KiB and rendered nothing at all. + fn oversized_png() -> Vec { + let mut b = png_bytes(3008, 1546); + b.resize(crate::graphics::MAX_IMAGE_BYTES + 1024, 0u8); + b + } + + #[cfg(unix)] + #[test] + fn an_oversized_png_is_downscaled_instead_of_silently_dropped() { + // THE REGRESSION: an over-cap PNG used to be skipped at send time with no picture, no + // notice, and `media_shown` still claiming it was displayed — which is exactly why + // Markdown-view.png (501 KiB) rendered while File-viewer.png (549 KiB) never did. + // `head -c` stands in for ffmpeg: it consumes stdin and emits a strictly smaller stream + // whose PNG header (and therefore parsed dimensions) survives intact. + let p = tmp("oversized.png", &oversized_png()); + let renderers = Renderers { + image: vec!["sh".into(), "-c".into(), "head -c 1000".into()], + ..cat_like() + }; + let (text, notice, media) = render_media( + &renderers, + &p, + crate::media::MediaKind::Png, + DEFAULT_MEDIA_MAX_BYTES, + None, + ); + + assert_eq!( + notice, None, + "a successful downscale is not a problem to report" + ); + let payload = media.expect("an over-cap PNG must still produce a payload"); + assert!( + payload.png.len() <= crate::graphics::MAX_IMAGE_BYTES, + "the payload must fit the host's cap, got {} bytes", + payload.png.len() + ); + let line: String = text + .lines + .iter() + .flat_map(|l| l.spans.iter()) + .map(|s| s.content.as_ref()) + .collect(); + assert!( + line.starts_with("[image: 3008×1546 ·"), + "the caption reports the file's TRUE size; the downscale is a transport detail: {line}" + ); + fs::remove_file(&p).ok(); + } + + #[test] + fn an_oversized_png_without_a_converter_says_so_rather_than_vanishing() { + // Degradation, not silence: with no ffmpeg there is nothing to downscale with, so the + // user gets the text line AND an explanation pointing at the fix. + let p = tmp("oversized-noconv.png", &oversized_png()); + let renderers = Renderers { + image: vec!["herdr-no-such-converter".into()], + ..cat_like() + }; + let (_, notice, media) = render_media( + &renderers, + &p, + crate::media::MediaKind::Png, + DEFAULT_MEDIA_MAX_BYTES, + None, + ); + assert!(media.is_none(), "nothing sendable was produced"); + assert_eq!(notice.as_deref(), Some(OVERSIZED_NOTICE)); + fs::remove_file(&p).ok(); + } + + #[cfg(unix)] + #[test] + fn a_converter_that_ignores_the_size_request_gives_up_instead_of_looping() { + // `cat` echoes its input unchanged, so it never gets under the cap. Without the + // "did it actually shrink?" guard this would burn the whole attempt budget on identical + // subprocess calls; with it, the first pass concludes the converter is useless. + let p = tmp("oversized-noop.png", &oversized_png()); + let renderers = Renderers { + image: vec!["cat".into()], + ..cat_like() + }; + let (_, notice, media) = render_media( + &renderers, + &p, + crate::media::MediaKind::Png, + DEFAULT_MEDIA_MAX_BYTES, + None, + ); + assert!(media.is_none()); + assert_eq!(notice.as_deref(), Some(OVERSIZED_NOTICE)); + fs::remove_file(&p).ok(); + } + + #[cfg(unix)] + #[test] + fn the_video_still_limits_frames_before_the_output_not_after_it() { + // THE REGRESSION: `-frames:v 1` appended AFTER the output URL is inert — ffmpeg applies + // output options to the output that follows them. That made every still preview decode + // the WHOLE video (8 MB of frames on a short clip; a timeout reported as "the video + // decoder is unavailable" on a long one). The stub echoes its own argv so the ordering + // is asserted directly, without needing ffmpeg. + let renderers = Renderers { + video: vec![ + "sh".into(), + "-c".into(), + // Emit the argv we were handed, so the test sees the real command shape. + "printf '%s\\n' \"$@\" >&2; printf ''".into(), + "argv0".into(), + "-i".into(), + "{name}".into(), + "-f".into(), + "image2pipe".into(), + "-".into(), + ], + ..cat_like() + }; + let command = with_video_name(&renderers.video, "/tmp/clip.mp4"); + let command = crate::media::player::substitute(&command, "0", "8", "640", "360"); + let mut command = command; + let before_output = command.len().saturating_sub(1); + command.splice( + before_output..before_output, + ["-frames:v".to_string(), "1".to_string()], + ); + + let frames_at = command + .iter() + .position(|a| a == "-frames:v") + .expect("present"); + let output_at = command.len() - 1; + assert_eq!(command[output_at], "-", "the output URL stays last"); + assert!( + frames_at < output_at, + "the single-frame limit must precede the output URL, else ffmpeg ignores it: {command:?}" + ); + assert_eq!(command[frames_at + 1], "1"); + } + + #[test] + fn image_size_substitution_leaves_no_placeholder_behind() { + // The default converter carries `{width}`/`{height}`; if substitution were ever skipped, + // ffmpeg would receive the literal braces and fail on every image. + let command: Vec = ["ffmpeg", "-vf", "scale={width}:{height}:x", "pipe:1"] + .iter() + .map(|s| s.to_string()) + .collect(); + let got = with_image_size(&command, 640, 480, "lanczos"); + assert_eq!(got[2], "scale=640:480:x"); + assert_eq!(got[1], "-vf"); + assert!( + !got.iter().any(|a| a.contains('{')), + "no placeholder may survive: {got:?}" + ); + } + + #[test] + fn media_image_routes_through_the_configured_converter() { + // The converter is fed the raw bytes on stdin and its stdout is returned raw. A + // `cat` converter round-trips them, so a JPEG's bytes are not validated as PNG all the + // way down — the semantic is "capture the converter's stdout as the payload". + let p = tmp("media.jpg", b"\xff\xd8ff fake jpeg bytes"); + let renderers = Renderers { + image: vec!["cat".into()], + ..cat_like() + }; + let (text, _notice, media) = render_media( + &renderers, + &p, + crate::media::MediaKind::Image, + DEFAULT_MEDIA_MAX_BYTES, + None, + ); + let line: String = text + .lines + .iter() + .flat_map(|l| l.spans.iter()) + .map(|s| s.content.as_ref()) + .collect(); + assert_eq!( + line, "[image: preview not shown]", + "non-PNG converter output degrades" + ); + assert_eq!(media, None); + fs::remove_file(&p).ok(); + } + + #[test] + fn media_image_converted_to_png_carries_the_png_bytes() { + // The `image` renderer fixture emits a well-formed 24-byte PNG header to stdout in + // response to any stdin, so its output is a parseable PNG payload. + let esc = escape_octal(&png_bytes(100, 60)); + #[cfg(unix)] + let renderers = Renderers { + image: vec![ + "sh".into(), + "-c".into(), + format!("cat >/dev/null; printf '{esc}'"), + ], + ..cat_like() + }; + #[cfg(not(unix))] + let renderers = cat_like(); + #[cfg(unix)] + { + let p = tmp("media-image-png", b"fake jpeg bytes"); + let (_, _notice, media) = render_media( + &renderers, + &p, + crate::media::MediaKind::Image, + DEFAULT_MEDIA_MAX_BYTES, + None, + ); + let payload = media.expect("a PNG payload from a valid converter output"); + assert_eq!(payload.png, png_bytes(100, 60)); + fs::remove_file(&p).ok(); + } + } + + /// `\NNN` octal escapes for `sh` `printf` (PNG magic includes bytes `printf` would otherwise + /// interpret, e.g. `\r\n` — escaping by hand avoids a second interpreter layer). + fn escape_octal(bytes: &[u8]) -> String { + bytes + .iter() + .map(|b| format!("\\{:03o}", b)) + .collect::() + } + + #[test] + fn media_fallback_degrades_cleanly_when_over_the_cap_or_unreadable() { + // Over the media cap → placeholder text + notice, no bytes read. + let p = tmp("big.png", &png_bytes(64, 48)); + let renderers = cat_like(); + let (_, notice, media) = + render_media(&renderers, &p, crate::media::MediaKind::Png, 1, None); + assert!(notice.is_some(), "over-cap PNG must produce a notice"); + assert_eq!(media, None); + // Missing file → same graceful placeholder. + let (text, notice, media) = render_media( + &renderers, + Path::new("/nonexistent/hfv-media.jpg"), + crate::media::MediaKind::Image, + DEFAULT_MEDIA_MAX_BYTES, + None, + ); + let line: String = text + .lines + .iter() + .flat_map(|l| l.spans.iter()) + .map(|s| s.content.as_ref()) + .collect(); + assert!(!line.is_empty(), "the pane is never blank"); + assert!(notice.is_some()); + assert_eq!(media, None); + fs::remove_file(&p).ok(); + } + + fn cat_like() -> Renderers { + Renderers { + markdown: vec!["cat".into()], + diff: vec!["cat".into()], + full_diff: vec!["cat".into()], + syntax: vec!["cat".into()], + image: vec!["cat".into()], + video: vec!["cat".into()], + probe: Vec::new(), + timeout: Duration::from_secs(5), + } + } } diff --git a/src/view_policy.rs b/src/view_policy.rs index 9ce6c473..adc33dc5 100644 --- a/src/view_policy.rs +++ b/src/view_policy.rs @@ -19,6 +19,9 @@ pub enum ViewMode { FullDiff, /// Syntax-highlighted file content. SyntaxContent, + /// An image or a video, placed inline via the herdr graphics socket. One mode covers both: + /// the mode is chosen per file (media kind) and playback state lives in the controller. + Media, } /// The facts the policy needs about a file — no path I/O is performed here. @@ -27,12 +30,16 @@ pub struct FileDescriptor { pub path: PathBuf, pub is_markdown: bool, pub is_changed: bool, + /// What kind of media the path names, if any (`None` for plain text/diff files). + pub media: Option, } /// The auto-selected default view mode for a file. pub fn default_mode(fd: &FileDescriptor) -> ViewMode { if fd.is_changed { ViewMode::Diff + } else if fd.media.is_some() { + ViewMode::Media } else if fd.is_markdown { ViewMode::RenderedMarkdown } else { @@ -42,7 +49,8 @@ pub fn default_mode(fd: &FileDescriptor) -> ViewMode { /// The modes a cycle key steps through for a file, default first (AC-11). A changed file /// also offers a full-context diff (whole file + line numbers + inline diff) right after -/// the compact diff; markdown adds its rendered view; every file ends with syntax content. +/// the compact diff; markdown adds its rendered view; media adds a media view; every file +/// ends with syntax content. pub fn applicable_modes(fd: &FileDescriptor) -> Vec { let mut modes = vec![default_mode(fd)]; let add = |modes: &mut Vec, m: ViewMode| { @@ -54,6 +62,9 @@ pub fn applicable_modes(fd: &FileDescriptor) -> Vec { add(&mut modes, ViewMode::Diff); add(&mut modes, ViewMode::FullDiff); } + if fd.media.is_some() { + add(&mut modes, ViewMode::Media); + } if fd.is_markdown { add(&mut modes, ViewMode::RenderedMarkdown); } @@ -70,6 +81,16 @@ mod tests { path: PathBuf::from(name), is_markdown, is_changed, + media: None, + } + } + + fn media_fd(name: &str, is_changed: bool) -> FileDescriptor { + FileDescriptor { + path: PathBuf::from(name), + is_markdown: false, + is_changed, + media: Some(crate::media::MediaKind::Png), } } @@ -148,4 +169,29 @@ mod tests { seen.dedup(); assert_eq!(modes, seen, "applicable modes must not repeat"); } + + #[test] + fn media_defaults_to_media_unless_changed() { + // A media file defaults to Media, but a changed media file is Diff (git first). + assert_eq!(default_mode(&media_fd("image.png", false)), ViewMode::Media); + assert_eq!(default_mode(&media_fd("image.png", true)), ViewMode::Diff); + } + + #[test] + fn media_cycle_offers_media_then_plain_content() { + // `Tab` still reaches the plain placeholder text beneath the image (AC-11). + let modes = applicable_modes(&media_fd("image.png", false)); + assert_eq!(modes, vec![ViewMode::Media, ViewMode::SyntaxContent]); + // A changed media file puts the diffs first, media next, plain content last. + let changed = applicable_modes(&media_fd("image.png", true)); + assert_eq!( + changed, + vec![ + ViewMode::Diff, + ViewMode::FullDiff, + ViewMode::Media, + ViewMode::SyntaxContent + ] + ); + } } diff --git a/tests/annotations.rs b/tests/annotations.rs index 6c462474..044a5c38 100644 --- a/tests/annotations.rs +++ b/tests/annotations.rs @@ -63,6 +63,7 @@ impl ContentProvider for Lines { content: Text::raw(lines.join("\n")), notices: Vec::new(), source: self.source_mapped.then_some(lines), + media: None, } } } diff --git a/tests/common/mod.rs b/tests/common/mod.rs index dfc30358..8f048805 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -226,6 +226,7 @@ impl ContentProvider for NoopContent { content: Text::raw("content"), notices: Vec::new(), source: None, + media: None, } } } diff --git a/tests/controller.rs b/tests/controller.rs index 6fca4c6d..229a6b4a 100644 --- a/tests/controller.rs +++ b/tests/controller.rs @@ -75,6 +75,7 @@ impl ContentProvider for StubContent { content: Text::raw("stub-content"), notices: Vec::new(), source: None, + media: None, } } } @@ -94,6 +95,7 @@ impl ContentProvider for DelayedNamedContent { content: Text::raw(format!("BODY-OF:{name}")), notices: Vec::new(), source: None, + media: None, } } } @@ -1207,6 +1209,7 @@ impl ContentProvider for LinesContent { content: Text::raw(body), notices: Vec::new(), source: None, + media: None, } } } @@ -1222,6 +1225,7 @@ impl ContentProvider for WideContent { content: Text::raw(body), notices: Vec::new(), source: None, + media: None, } } } @@ -3345,6 +3349,7 @@ impl ContentProvider for PathContent { content: Text::raw(format!("showing {}", path.display())), notices: Vec::new(), source: None, + media: None, } } } @@ -7290,6 +7295,7 @@ impl ContentProvider for WrapLines { content: Text::raw(lines.join("\n")), notices: Vec::new(), source: None, + media: None, } } } @@ -7600,6 +7606,7 @@ impl ContentProvider for SearchContent { content: Text::raw(lines.join("\n")), notices: Vec::new(), source: None, + media: None, } } } @@ -8316,6 +8323,7 @@ impl ContentProvider for SwitchingContent { content: Text::raw(lines.join("\n")), notices: Vec::new(), source: None, + media: None, } } } @@ -8570,6 +8578,7 @@ impl ContentProvider for ContentWithoutSentinel { content: Text::raw(lines), notices: Vec::new(), source: None, + media: None, } } } @@ -9200,6 +9209,9 @@ fn uppercasing_markdown_renderers() -> Renderers { diff: vec!["cat".into()], full_diff: vec!["cat".into()], syntax: vec!["cat".into()], + image: vec!["cat".into()], + video: vec!["cat".into()], + probe: Vec::new(), timeout: Duration::from_secs(5), } } @@ -9212,6 +9224,9 @@ fn absent_markdown_renderers() -> Renderers { diff: vec!["cat".into()], full_diff: vec!["cat".into()], syntax: vec!["cat".into()], + image: vec!["cat".into()], + video: vec!["cat".into()], + probe: Vec::new(), timeout: Duration::from_secs(5), } } @@ -9319,6 +9334,9 @@ fn slow_markdown_renderers(marker: &std::path::Path) -> Renderers { diff: vec!["cat".into()], full_diff: vec!["cat".into()], syntax: vec!["cat".into()], + image: vec!["cat".into()], + video: vec!["cat".into()], + probe: Vec::new(), // This is deliberately generous: Help must use its own shared 200 ms deadline. timeout: Duration::from_secs(5), } @@ -10235,6 +10253,9 @@ fn open_help_orders_optional_sections_after_whats_new_and_keeps_independent_scro markdown: None, diff: None, syntax: None, + image: None, + video: None, + media_max_kib: 8192, open: None, reveal: None, hide_dotfiles: false, diff --git a/tests/controller_async.rs b/tests/controller_async.rs index 17069b62..8cc74790 100644 --- a/tests/controller_async.rs +++ b/tests/controller_async.rs @@ -38,6 +38,7 @@ impl ContentProvider for SlowContent { content: Text::raw(format!("rendered:{name}")), notices: Vec::new(), source: None, + media: None, } } } @@ -68,6 +69,7 @@ impl ContentProvider for GatedContent { content: Text::raw(format!("rendered:{name}")), notices: Vec::new(), source: None, + media: None, } } } @@ -171,6 +173,7 @@ impl ContentProvider for PanicOnContent { content: Text::raw(format!("rendered:{name}")), notices: Vec::new(), source: None, + media: None, } } } @@ -267,6 +270,7 @@ impl ContentProvider for EchoDiffContent { )), notices: Vec::new(), source: None, + media: None, } } } @@ -781,12 +785,21 @@ impl WidthProbe { content: Text::raw(s), notices: Vec::new(), source: None, + media: None, } } } impl ContentProvider for WidthProbe { fn render(&self, path: &Path, mode: ViewMode, raw_diff: Option<&str>) -> RenderResult { - self.render_at_width(path, mode, raw_diff, None, None, DiffRenderMode::default()) + self.render_at_width( + path, + mode, + raw_diff, + None, + None, + DiffRenderMode::default(), + None, + ) } fn render_at_width( &self, @@ -796,6 +809,7 @@ impl ContentProvider for WidthProbe { width: Option, _pane_width: Option, _diff_render_mode: DiffRenderMode, + _media_box: Option<(u32, u32)>, ) -> RenderResult { self.widths.lock().unwrap().push(width); let name = path.file_name().unwrap().to_string_lossy().into_owned(); @@ -1044,6 +1058,7 @@ fn render_at_width_default_impl_forwards_to_render_ignoring_width() { content: Text::raw(format!("r:{name}:{}", raw_diff.unwrap_or("-"))), notices: Vec::new(), source: None, + media: None, } } } @@ -1057,6 +1072,7 @@ fn render_at_width_default_impl_forwards_to_render_ignoring_width() { Some(42), None, DiffRenderMode::default(), + None, ); assert_eq!( flatten(&base.content), @@ -1072,7 +1088,15 @@ fn render_at_width_default_impl_forwards_to_render_ignoring_width() { struct WidthDependentMatches; impl ContentProvider for WidthDependentMatches { fn render(&self, path: &Path, mode: ViewMode, raw_diff: Option<&str>) -> RenderResult { - self.render_at_width(path, mode, raw_diff, None, None, DiffRenderMode::default()) + self.render_at_width( + path, + mode, + raw_diff, + None, + None, + DiffRenderMode::default(), + None, + ) } fn render_at_width( &self, @@ -1082,6 +1106,7 @@ impl ContentProvider for WidthDependentMatches { width: Option, _pane_width: Option, _diff_render_mode: DiffRenderMode, + _media_box: Option<(u32, u32)>, ) -> RenderResult { let n = match width { Some(w) if w >= 40 => 8, @@ -1095,6 +1120,7 @@ impl ContentProvider for WidthDependentMatches { content: Text::raw(s), notices: Vec::new(), source: None, + media: None, } } } diff --git a/tests/docs_consistency.rs b/tests/docs_consistency.rs index 01da17f1..75dadbe1 100644 --- a/tests/docs_consistency.rs +++ b/tests/docs_consistency.rs @@ -117,6 +117,9 @@ fn config_example_documents_every_config_key() { "tree_max_cols", "preview_max_lines", "preview_max_kib", + "image", + "video", + "media_max_kib", ] { assert!( has_commented_assignment(CONFIG_EXAMPLE, key), diff --git a/tests/lineselect.rs b/tests/lineselect.rs index 71d130f1..3cded721 100644 --- a/tests/lineselect.rs +++ b/tests/lineselect.rs @@ -64,6 +64,7 @@ impl ContentProvider for MultiLine { content: Text::raw(lines.join("\n")), notices: Vec::new(), source: None, + media: None, } } } @@ -84,6 +85,7 @@ impl ContentProvider for WrapBody { content: Text::raw(lines.join("\n")), notices: Vec::new(), source: None, + media: None, } } } @@ -175,6 +177,7 @@ impl ContentProvider for EmptyContent { content: Text::default(), notices: Vec::new(), source: None, + media: None, } } } @@ -191,6 +194,7 @@ impl ContentProvider for ControlContent { content: Text::raw("\tcode\x1bhere"), notices: Vec::new(), source: None, + media: None, } } } @@ -213,6 +217,7 @@ impl ContentProvider for GutterContent { " let x = 5;".to_string(), "}".to_string(), ]), + media: None, } } } @@ -230,6 +235,7 @@ impl ContentProvider for PlainNoSource { content: Text::raw(lines.join("\n")), notices: Vec::new(), source: None, + media: None, } } } @@ -1641,6 +1647,7 @@ fn wrapped_word_break_maps_columns_to_the_right_word() { content: Text::raw(format!("{} {}", "a".repeat(50), "b".repeat(40))), notices: Vec::new(), source: None, + media: None, } } } @@ -1757,6 +1764,7 @@ impl ContentProvider for GutterWithSource { "\tlet x = 5;".to_string(), // REAL tab in the file; bat shows 4 spaces "}".to_string(), ]), + media: None, } } } @@ -1776,6 +1784,7 @@ impl ContentProvider for PlainWithSource { "\tlet x = 5;".to_string(), "3 loops below".to_string(), ]), + media: None, } } } @@ -1942,6 +1951,7 @@ fn source_control_bytes_are_still_scrubbed() { content: Text::raw("clean view"), notices: Vec::new(), source: Some(vec!["\tcode\x1b[2Jhere".to_string()]), + media: None, } } } @@ -1981,6 +1991,7 @@ fn wrapped_break_dropped_space_does_not_shift_selection_to_the_line_above() { content: Text::raw(format!("{line1}\nsecond\nthird")), notices: Vec::new(), source: None, + media: None, } } } diff --git a/tests/media_shown.rs b/tests/media_shown.rs new file mode 100644 index 00000000..48bfaef1 --- /dev/null +++ b/tests/media_shown.rs @@ -0,0 +1,442 @@ +//! Media view clear/set discipline (the plan's task 4): the controller must place the image on +//! screen exactly when and where it should be, and clear it when it should not — proven with a +//! `GraphicsSink` recorder, never a real socket. +//! +//! The discipline is one comparison, not N call sites: after every draw the controller computes +//! the desired media state and issues `clear()` + `set()` on a difference (`clear()` alone when +//! nothing should be shown). A `GraphicsHost` recorder is the automated oracle here — ratatui's +//! `TestBackend` renders a text grid, so no snapshot can prove an image appeared. + +mod common; + +use common::TempDir; +use herdr_file_viewer::controller::{ + Components, ContentProvider, Controller, EditorHandoff, EditorOutcome, GitService, + RenderResult, RootProviders, +}; +use herdr_file_viewer::git::{Baseline, Status}; +use herdr_file_viewer::graphics::{CellMetrics, GraphicsCommand, GraphicsSink}; +use herdr_file_viewer::intent::Intent; +use herdr_file_viewer::presenter::PaneGeometry; +use herdr_file_viewer::render::{MediaPayload, Renderers}; +use ratatui::layout::Rect; +use ratatui::text::Text; +use std::collections::BTreeMap; +use std::path::{Path, PathBuf}; +use std::sync::{Arc, Mutex}; +use std::time::{Duration, Instant}; + +/// A Content Renderer stub that answers a media payload for `*.png` / `*.mp4` and plain text +/// otherwise — standing in for the real `render_media` delegation without an external converter. +struct MediaContent; + +impl ContentProvider for MediaContent { + fn render(&self, path: &Path, _mode: ViewMode, _raw_diff: Option<&str>) -> RenderResult { + match path.extension().and_then(|e| e.to_str()) { + Some("png") => RenderResult { + content: Text::raw("[image: 4×3 PNG]"), + notices: Vec::new(), + source: None, + media: Some(MediaPayload { + kind: herdr_file_viewer::media::MediaKind::Png, + png: png_bytes(4, 3), + natural: (4, 3), + }), + }, + Some("mp4") => RenderResult { + content: Text::raw("[video: 4×3 — p to play]"), + notices: Vec::new(), + source: None, + media: Some(MediaPayload { + kind: herdr_file_viewer::media::MediaKind::Video, + png: png_bytes(4, 3), // the still preview (frame 0) + natural: (4, 3), + }), + }, + _ => RenderResult { + content: Text::raw("plain"), + notices: Vec::new(), + source: None, + media: None, + }, + } + } +} + +/// A minimal PNG header (the 24-byte IHDR prefix) — enough for `png_dimensions` / the fast path. +fn png_bytes(w: u32, h: u32) -> Vec { + let mut b = vec![0x89, b'P', b'N', b'G', 0x0d, 0x0a, 0x1a, 0x0a]; + b.extend_from_slice(&13u32.to_be_bytes()); + b.extend_from_slice(b"IHDR"); + b.extend_from_slice(&w.to_be_bytes()); + b.extend_from_slice(&h.to_be_bytes()); + b +} + +/// A [`GitService`] stub that reports nothing changed — media tests don't care about git. +struct StubGit; + +impl GitService for StubGit { + fn status(&self) -> BTreeMap { + BTreeMap::new() + } + fn changed_set(&self, _baseline: Baseline) -> BTreeMap { + BTreeMap::new() + } + fn diff(&self, _rel_path: &Path, _baseline: Baseline, _full_context: bool) -> String { + String::new() + } + fn diff_directory(&self, _rel_dir: &Path, _baseline: Baseline) -> String { + String::new() + } +} + +/// An editor stub that never launches anything. +struct StubEditor; +impl EditorHandoff for StubEditor { + fn open(&mut self, _file: &Path) -> EditorOutcome { + EditorOutcome::NotLaunched("no editor".into()) + } +} + +/// A [`GraphicsSink`] recorder: captures every command synchronously instead of touching a socket. +/// Shared (`Arc>`) so the test keeps a handle to read back after handing it over. +#[derive(Default, Clone)] +struct RecordingSink { + commands: Arc>>, +} + +impl GraphicsSink for RecordingSink { + fn send(&self, command: GraphicsCommand) { + let line = match &command { + GraphicsCommand::Hide => "hide".to_string(), + GraphicsCommand::Show(frame) => format!( + "show {}x{} png@{}x{} ", + frame.width, frame.height, frame.placement.grid_cols, frame.placement.grid_rows + ), + }; + self.commands.lock().unwrap().push(line); + } +} + +fn build(root: &Path) -> (Controller, RecordingSink) { + let git: Arc = Arc::new(StubGit); + let components = Components { + providers: Box::new(move |_resolved| RootProviders { + git: Arc::clone(&git), + content: Box::new(MediaContent), + }), + editor: Box::new(StubEditor), + clipboard: Box::new(common::RecordingClipboard::default()), + renderers: None, + }; + let mut ctrl = Controller::new( + common::resolved(root.to_path_buf(), false), + Baseline::Head, + components, + ); + let sink = RecordingSink::default(); + ctrl.set_graphics( + Box::new(sink.clone()), + Some(CellMetrics { + cell_width_px: 20, + cell_height_px: 41, + }), + ); + (ctrl, sink) +} + +/// Wait for the worker's render of the current selection to land (`poll` applies it), i.e. the +/// displayed content stops being a `Rendering…` placeholder. +fn await_content(ctrl: &mut Controller, marker: &str) { + let deadline = Instant::now() + Duration::from_secs(5); + while !flatten(ctrl.content()).contains(marker) { + assert!( + Instant::now() < deadline, + "content '{marker}' never rendered" + ); + ctrl.poll(); + std::thread::sleep(Duration::from_millis(5)); + } +} + +fn flatten(t: &Text) -> String { + t.lines + .iter() + .flat_map(|l| l.spans.iter()) + .map(|s| s.content.as_ref()) + .collect() +} + +use herdr_file_viewer::view_policy::ViewMode; + +#[test] +fn media_selecting_png_clears_then_sets_and_never_repeats() { + let dir = TempDir::new(); + std::fs::write(dir.path().join("a.png"), png_bytes(4, 3)).unwrap(); + let (mut ctrl, sink) = build(dir.path()); + ctrl.set_pane_geometry(geom_with_inner(Rect::new(0, 0, 40, 24))); + await_content(&mut ctrl, "4×3"); + + ctrl.sync_media(); + let calls = sink.commands.lock().unwrap().clone(); + assert_eq!( + calls.len(), + 2, + "clear-then-set on first placement: {calls:?}" + ); + assert_eq!(calls[0], "hide"); + assert!( + calls[1].starts_with("show 4x3 png@"), + "the image is placed at a fitted rect: {calls:?}" + ); + + // An unchanged placement must NOT issue a redundant set on every idle draw. + ctrl.sync_media(); + assert_eq!( + sink.commands.lock().unwrap().len(), + 2, + "no redundant set when nothing changed" + ); +} + +#[test] +fn media_replacing_media_clears_then_sets_the_new_image() { + let dir = TempDir::new(); + std::fs::write(dir.path().join("b.png"), png_bytes(4, 3)).unwrap(); + std::fs::write(dir.path().join("c.png"), png_bytes(4, 3)).unwrap(); + let (mut ctrl, sink) = build(dir.path()); + ctrl.set_pane_geometry(geom_with_inner(Rect::new(0, 0, 40, 24))); + await_content(&mut ctrl, "4×3"); + ctrl.sync_media(); + let before = sink.commands.lock().unwrap().len(); + assert_eq!(before, 2); + + // Select the second image → the worker renders it → the discipline clears-then-sets. + ctrl.handle(Intent::NavDown); + ctrl.poll(); + let deadline = Instant::now() + Duration::from_secs(5); + while ctrl.tree().cursor() != 1 { + assert!(Instant::now() < deadline, "cursor never advanced"); + ctrl.poll(); + std::thread::sleep(Duration::from_millis(5)); + } + await_content(&mut ctrl, "4×3"); + ctrl.sync_media(); + let calls = sink.commands.lock().unwrap().clone(); + assert_eq!( + calls.len(), + before + 2, + "a different image clears-then-sets again: {calls:?}" + ); + assert_eq!(calls[before], "hide", "old image cleared"); + assert_eq!(&calls[before + 1][..13], "show 4x3 png@", "new image shown"); +} + +#[test] +fn leaving_media_clears_the_image_and_keeping_it_alone_does_nothing() { + let dir = TempDir::new(); + std::fs::write(dir.path().join("a.png"), png_bytes(4, 3)).unwrap(); + std::fs::write(dir.path().join("note.txt"), "plain").unwrap(); + let (mut ctrl, sink) = build(dir.path()); + ctrl.set_pane_geometry(geom_with_inner(Rect::new(0, 0, 40, 24))); + await_content(&mut ctrl, "4×3"); + ctrl.sync_media(); + let before = sink.commands.lock().unwrap().len(); + + // Select the text file → its render has no media → the discipline clears alone. + ctrl.handle(Intent::NavDown); + await_content(&mut ctrl, "plain"); + ctrl.sync_media(); + let calls = sink.commands.lock().unwrap().clone(); + assert_eq!( + calls.len(), + before + 1, + "leaving media issues a clear alone, no phantom set: {calls:?}" + ); + assert_eq!(calls[before], "hide"); + assert!(calls[before + 1..].is_empty()); + + // Still on the text file → desired is still None → nothing more. + ctrl.sync_media(); + assert_eq!(sink.commands.lock().unwrap().len(), before + 1); +} + +/// A geometry whose content column is drawn, so `content_inner` is measurable. +fn geom_with_inner(inner: Rect) -> PaneGeometry { + PaneGeometry { + content_inner: Some(inner), + ..Default::default() + } +} + +// --------------------------------------------------------------------------- +// Video playback intents (the decoder thread is driven through the injected `video` renderer, +// which here is THIS test binary re-executing as a fixture that emits PNG frames). +// --------------------------------------------------------------------------- + +const VIDEO_FIXTURE_ARG: &str = "--hfv-video-fixture="; + +/// A minimal PNG (signature + header + IEND trailer) — enough for the splitter and the host-side +/// `png_dimensions`; the exact contents don't matter to the controller under test. +fn video_frame(tag: u8) -> Vec { + let mut b = vec![0x89, b'P', b'N', b'G', 0x0d, 0x0a, 0x1a, 0x0a]; + b.extend_from_slice(&13u32.to_be_bytes()); + b.extend_from_slice(b"IHDR"); + b.extend_from_slice(&4u32.to_be_bytes()); + b.extend_from_slice(&3u32.to_be_bytes()); + b.push(tag); + b.extend_from_slice(&[0, 0, 0, 0, b'I', b'E', b'N', b'D']); + b.extend_from_slice(&[0, 0, 0, 0]); + b +} + +/// Run as the video fixture: write `n` PNG frames to stdout (self-exec), then exit. When run by +/// the test harness normally (no fixture arg), it is a no-op so the suite itself also passes. +#[test] +fn video_fixture() { + let Some(n) = std::env::args() + .find_map(|a| a.strip_prefix(VIDEO_FIXTURE_ARG).map(str::to_owned)) + .and_then(|s| s.parse::().ok()) + else { + return; // not an execution of the fixture — a no-op in a normal suite run + }; + use std::io::Write; + for i in 0..n { + let _ = std::io::stdout().write_all(&video_frame(i as u8)); + } + let _ = std::io::stdout().flush(); +} + +/// The `video` renderer: this test binary re-invoked as the fixture, emitting 3 frames then EOF. +fn video_renderer() -> Renderers { + Renderers { + markdown: vec!["cat".into()], + diff: vec!["cat".into()], + full_diff: vec!["cat".into()], + syntax: vec!["cat".into()], + image: vec!["cat".into()], + probe: Vec::new(), + video: vec![ + std::env::current_exe() + .expect("test binary path") + .display() + .to_string(), + "--exact".into(), + "video_fixture".into(), + "--".into(), + format!("{VIDEO_FIXTURE_ARG}3"), + ], + timeout: Duration::from_secs(5), + } +} + +/// Build a controller over `root` whose `video` renderer is the fixture (hermetic — no ffmpeg). +fn build_video(root: &Path) -> (Controller, RecordingSink) { + let git: Arc = Arc::new(StubGit); + let components = Components { + providers: Box::new(move |_resolved| RootProviders { + git: Arc::clone(&git), + content: Box::new(MediaContent), + }), + editor: Box::new(StubEditor), + clipboard: Box::new(common::RecordingClipboard::default()), + renderers: Some(video_renderer()), + }; + let mut ctrl = Controller::new( + common::resolved(root.to_path_buf(), false), + Baseline::Head, + components, + ); + let sink = RecordingSink::default(); + ctrl.set_graphics( + Box::new(sink.clone()), + Some(CellMetrics { + cell_width_px: 20, + cell_height_px: 41, + }), + ); + (ctrl, sink) +} + +#[test] +fn video_play_pause_seek_restart_drive_the_decoder() { + let dir = TempDir::new(); + std::fs::write(dir.path().join("clip.mp4"), b"not a real mp4").unwrap(); + let (mut ctrl, sink) = build_video(dir.path()); + ctrl.set_pane_geometry(geom_with_inner(Rect::new(0, 0, 40, 24))); + await_content(&mut ctrl, "p to play"); + ctrl.sync_media(); + let still_count = sink.commands.lock().unwrap().len(); + assert_eq!( + still_count, 2, + "the still preview is placed (clear-then-set) on selection" + ); + + // `p` starts playback: the decoder streams frames; tick_media paces one per tick. + ctrl.handle(Intent::MediaPlayPause); + let mut frames = 0; + let deadline = Instant::now() + Duration::from_secs(5); + while frames < 2 { + assert!(Instant::now() < deadline, "playback never produced frames"); + if ctrl.tick_media(Instant::now()) { + frames += 1; + } + std::thread::sleep(Duration::from_millis(1)); + } + let calls = sink.commands.lock().unwrap().clone(); + assert!( + calls + .iter() + .filter(|c| c.starts_with("show 4x3 png@")) + .count() + >= frames, + "playback frames are shown: {calls:?}" + ); + + // `p` again pauses: no new frames are pushed to the graphics sink. Synchronous tell, no sleep: + // a paused (or just-finished) player's tick must send nothing (per AGENTS.md a "nothing happened" + // sleep proves nothing). + ctrl.handle(Intent::MediaPlayPause); + let before = sink.commands.lock().unwrap().len(); + ctrl.tick_media(Instant::now()); + assert_eq!( + sink.commands.lock().unwrap().len(), + before, + "paused playback sends nothing" + ); + + // `0` restarts: a fresh decoder is spawned and a frame reaches the sink — either via the + // paused single-frame preview (`media_start` pulls one immediately) or via a tick when the + // restart resumes playing. `media_start` first sends the Hide, so wait for the SHOW that must + // follow it rather than any command growth. + ctrl.handle(Intent::MediaRestart); + let deadline = Instant::now() + Duration::from_secs(5); + loop { + let calls = sink.commands.lock().unwrap(); + let shown_since_restart = calls.iter().skip(before).any(|c| c.starts_with("show ")); + assert!( + Instant::now() < deadline, + "restart never re-decoded a frame: {calls:?}" + ); + if shown_since_restart { + break; + } + drop(calls); + ctrl.tick_media(Instant::now()); + std::thread::sleep(Duration::from_millis(1)); + } + // `media_start` clears first (Hide) then shows the restarted segment's first frame, so a + // stale frame of the previous segment can never survive the restart. + let calls = sink.commands.lock().unwrap().clone(); + let restart_calls: Vec<&String> = calls.iter().skip(before).collect(); + assert_eq!( + restart_calls[0].as_str(), + "hide", + "restart clears the old frame first" + ); + assert!( + restart_calls.iter().any(|c| c.starts_with("show 4x3 png@")), + "restart re-decodes and shows" + ); +} diff --git a/tests/render_delegate.rs b/tests/render_delegate.rs index 111765fa..45f7ecf8 100644 --- a/tests/render_delegate.rs +++ b/tests/render_delegate.rs @@ -29,6 +29,9 @@ fn cat() -> Renderers { diff: vec!["cat".into()], full_diff: vec!["cat".into()], syntax: vec!["cat".into()], + image: vec!["cat".into()], + video: vec!["cat".into()], + probe: Vec::new(), timeout: Duration::from_secs(5), } } @@ -234,6 +237,9 @@ fn missing_renderer_falls_back_to_plain_text_with_a_notice() { diff: vec!["cat".into()], full_diff: vec!["cat".into()], syntax: vec!["cat".into()], + image: vec!["cat".into()], + video: vec!["cat".into()], + probe: Vec::new(), timeout: Duration::from_secs(5), }; let prepared = Prepared::Full { @@ -305,6 +311,9 @@ fn syntax_renderer_receives_the_file_name_via_placeholder() { diff: vec!["cat".into()], full_diff: vec!["cat".into()], syntax: vec!["sh".into(), "-c".into(), "echo {name}".into()], + image: vec!["cat".into()], + video: vec!["cat".into()], + probe: Vec::new(), timeout: Duration::from_secs(5), }; let prepared = Prepared::Full { @@ -335,6 +344,9 @@ fn a_malicious_file_name_cannot_inject_via_the_placeholder() { diff: vec!["cat".into()], full_diff: vec!["cat".into()], syntax: vec!["sh".into(), "-c".into(), "echo {name}".into()], + image: vec!["cat".into()], + video: vec!["cat".into()], + probe: Vec::new(), timeout: Duration::from_secs(5), }; let prepared = Prepared::Full { @@ -367,6 +379,9 @@ fn full_diff_mode_renders_the_diff_text_via_the_full_diff_renderer() { diff: vec!["herdr-no-such-binary-xyz".into()], // would fail if FullDiff used it full_diff: vec!["cat".into()], syntax: vec!["cat".into()], + image: vec!["cat".into()], + video: vec!["cat".into()], + probe: Vec::new(), timeout: Duration::from_secs(5), }; let full = "@@ -1,2 +1,2 @@\n fn main() {\n- old();\n+ new();\n }"; @@ -428,6 +443,9 @@ fn a_hanging_renderer_times_out_and_falls_back() { diff: vec!["cat".into()], full_diff: vec!["cat".into()], syntax: vec!["cat".into()], + image: vec!["cat".into()], + video: vec!["cat".into()], + probe: Vec::new(), timeout: Duration::from_millis(150), }; let prepared = Prepared::Full { @@ -516,6 +534,9 @@ fn glow_markdown_wrapped_to_width_never_exceeds_it() { diff: vec!["cat".into()], full_diff: vec!["cat".into()], syntax: vec!["cat".into()], + image: vec!["cat".into()], + video: vec!["cat".into()], + probe: Vec::new(), timeout: Duration::from_secs(5), }; // A table far wider than `width` at natural layout, plus a long prose paragraph — both must be diff --git a/tests/reroot.rs b/tests/reroot.rs index 09386c29..fb9d44cf 100644 --- a/tests/reroot.rs +++ b/tests/reroot.rs @@ -75,6 +75,7 @@ impl ContentProvider for FakeContent { content: Text::raw("fake-rendered-content"), notices: Vec::new(), source: None, + media: None, } } } @@ -665,6 +666,7 @@ impl ContentProvider for EchoDiffContent { content, notices: Vec::new(), source: None, + media: None, } } } diff --git a/tests/reveal_open.rs b/tests/reveal_open.rs index b992c0cb..1c227030 100644 --- a/tests/reveal_open.rs +++ b/tests/reveal_open.rs @@ -77,6 +77,7 @@ impl ContentProvider for StubContent { content: Text::raw("stub"), notices: Vec::new(), source: None, + media: None, } } } diff --git a/tests/search_integration.rs b/tests/search_integration.rs index b74f3ac3..d548d0fa 100644 --- a/tests/search_integration.rs +++ b/tests/search_integration.rs @@ -129,6 +129,7 @@ impl ContentProvider for SearchContent { content: Text::raw(lines.join("\n")), notices: Vec::new(), source: None, + media: None, } } } @@ -153,6 +154,7 @@ impl ContentProvider for TruncatedContent { content: Text::raw(shown.join("\n")), notices: Vec::new(), source: None, + media: None, } } } diff --git a/tests/update_banner.rs b/tests/update_banner.rs index 54c430d9..591ea9ad 100644 --- a/tests/update_banner.rs +++ b/tests/update_banner.rs @@ -48,6 +48,7 @@ impl ContentProvider for Content { content: Text::raw(""), notices: Vec::new(), source: None, + media: None, } } } From 8587a67a452418e4fae239e105d7632bad2715fe Mon Sep 17 00:00:00 2001 From: Charles Ji Date: Sun, 9 Aug 2026 18:06:55 -0400 Subject: [PATCH 3/8] docs: changelog entry for the media view 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. --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 624715d0..9feb090e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ All notable changes to this project are documented here. The format is based on ## [Unreleased] +### Added +- **Media view: images and video rendered inline in the content pane** through herdr's documented `pane.graphics.*` socket API — the bytes travel base64 inside JSON, so no escape sequence ever reaches the terminal and a hostile image still cannot drive it. A `.png` is shown natively; other images convert via `ffmpeg`; video plays at the host-limited ~8 fps with `p` (play/pause), `{`/`}` (seek ±5s), and `0` (restart), starting paused on frame 0. New config keys `image`, `video`, and `media_max_kib`; new remappable intents `media_play_pause`, `media_seek_back`, `media_seek_forward`, `media_restart`. → [usage](docs/usage.md#media-images-and-video) · [renderers](docs/renderers.md#media-images-and-video) · [keys](docs/keys.md) · [configuration](docs/configuration.md) + ### Fixed - Agent skill: the launch instructions no longer tell agents to pass `--cwd`. herdr resolves the manifest's relative pane command against it, so the launch failed with `plugin_pane_open_failed` — or worse, inside a built plugin checkout, silently ran that checkout's binary. The skill and the `docs/usage.md` snippet now explain that the viewed root follows the *focused herdr pane's* directory, so an agent's own `cd` does not move it. Thanks @AntonyKor (#139) → [agent skill](skills/herdr-file-viewer/SKILL.md) · [usage](docs/usage.md#teach-your-agent) From f8d7cca89225949058aed2289e301a4ea4d4b298 Mon Sep 17 00:00:00 2001 From: Charles Ji Date: Sun, 9 Aug 2026 18:25:15 -0400 Subject: [PATCH 4/8] feat(media): add a seekable progress bar; fix playback caption and pause MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/controller/mod.rs | 128 ++++++++++++++++++++++++++++++++------ src/controller/mouse.rs | 43 ++++++++++++- src/media.rs | 40 ++++++++++++ src/presenter.rs | 91 +++++++++++++++++++++++++++ src/render.rs | 10 ++- tests/controller.rs | 1 + tests/media_shown.rs | 2 + tests/presenter.rs | 1 + tests/presenter_narrow.rs | 1 + 9 files changed, 295 insertions(+), 22 deletions(-) diff --git a/src/controller/mod.rs b/src/controller/mod.rs index 5b084e3d..e33ea5a9 100644 --- a/src/controller/mod.rs +++ b/src/controller/mod.rs @@ -212,6 +212,15 @@ struct MediaPlayer { decoder: crate::media::player::Decoder, /// When the next frame may be pulled (pacing; see [`FRAME_INTERVAL`](crate::media::player::FRAME_INTERVAL)). next_frame_at: Instant, + /// Where `position` was when the current play run started, and when that was. Playback + /// position is wall-clock derived rather than counted in displayed frames: `-re` paces the + /// decoder to real time, but we display fewer frames than it produces, so counting frames + /// would make the progress bar drift slower than the video. + run_started_at: Instant, + run_started_from: f64, + /// Whether a decoded frame has actually been shown. Once true, the poster still must never be + /// put back — pausing should hold the frame you paused ON, not jump to frame 0. + has_frame: bool, } impl MediaPlayer { @@ -264,6 +273,7 @@ impl MediaPlayer { mode: ViewMode::Media, placement, }); + self.has_frame = true; true } @@ -1492,21 +1502,56 @@ impl Controller { /// bookkeeping of its own: a selection that moved off media, a `Tab` off Media, a zoom/resize /// that shrank or hid the content column, an overlay that covered it — all produce a different /// desired value (or `None`) automatically. - /// The cell rect the picture occupies: the content box with its TOP ROW RESERVED for the - /// info caption. Without this the image is placed over its own caption and hides it. - fn media_rect(inner: Rect) -> Option { - (inner.width > 0 && inner.height > 1).then(|| Rect { + /// The cell rect the picture occupies: the content box with its TOP ROW reserved for the + /// caption, and — for a video with a progress bar — its BOTTOM ROW reserved too. + /// + /// Both the still and playback go through here. Passing the raw content box for playback is + /// what let a playing video paint over its own caption while a paused one looked fine. + fn media_rect(inner: Rect, reserve_bar: bool) -> Option { + let reserved = 1 + u16::from(reserve_bar); // caption row, plus the bar row for video + (inner.width > 0 && inner.height > reserved).then(|| Rect { x: inner.x, y: inner.y + 1, width: inner.width, - height: inner.height - 1, + height: inner.height - reserved, + }) + } + + /// The reserved picture rect for the CURRENT selection, or `None` when there is no layout. + /// Playback and the still both go through here so they cannot disagree about which rows the + /// picture may use — passing the raw content box here is what let playback paint over its + /// own caption. + fn media_cell_rect(&self) -> Option { + Self::media_rect(self.geom.content_inner?, self.media_has_bar()) + } + + /// Whether the selected media draws a progress bar (a video of known duration). + fn media_has_bar(&self) -> bool { + self.content_media.as_ref().is_some_and(|m| { + m.kind == crate::media::MediaKind::Video && m.duration_s.is_some_and(|d| d > 0.0) }) } + /// The live playback position and length, for the progress bar. `None` unless a video with a + /// known duration is selected. + pub fn media_progress(&self) -> Option<(f64, f64)> { + let duration = self + .content_media + .as_ref()? + .duration_s + .filter(|d| *d > 0.0)?; + let position = self + .media_player + .as_ref() + .map(|p| p.position) + .unwrap_or(0.0); + Some((position.clamp(0.0, duration), duration)) + } + /// [`media_rect`](Self::media_rect) in pixels, for the render worker's resample target. fn media_box_px(&self) -> Option<(u32, u32)> { let metrics = self.cell_metrics?; - let rect = Self::media_rect(self.geom.content_inner?)?; + let rect = Self::media_rect(self.geom.content_inner?, self.media_has_bar())?; Some(( rect.width as u32 * metrics.cell_width_px, rect.height as u32 * metrics.cell_height_px, @@ -1533,7 +1578,7 @@ impl Controller { let (width, height) = media.natural; // And the layout must give us a non-empty pixel budget. let metrics = self.cell_metrics?; - let rect = Self::media_rect(self.geom.content_inner?)?; + let rect = Self::media_rect(self.geom.content_inner?, self.media_has_bar())?; Some(MediaShown { path, mode: ViewMode::Media, @@ -1560,7 +1605,7 @@ impl Controller { // Scoped to a player playing THIS file: a stale player must never wedge the surface for // a file the user has already moved on from. if let Some(player) = self.media_player.as_ref() - && player.playing + && (player.playing || player.has_frame) && Some(&player.path) == self.content_path.as_ref() { return; @@ -1635,16 +1680,12 @@ impl Controller { }); // The caption row is reserved here too, so the decoder's target matches the still's and // the placement's exactly — otherwise pressing play visibly resized the video. - let rect = self - .geom - .content_inner - .and_then(Self::media_rect) - .unwrap_or(Rect { - x: 0, - y: 1, - width: 80, - height: 23, - }); + let rect = self.media_cell_rect().unwrap_or(Rect { + x: 0, + y: 1, + width: 80, + height: 23, + }); let (width, height) = crate::media::frame_budget(rect, metrics); let substituted = crate::media::player::substitute( &self.renderers.video, @@ -1679,9 +1720,12 @@ impl Controller { position, decoder, next_frame_at: Instant::now(), + run_started_at: Instant::now(), + run_started_from: position, + has_frame: false, }; if !playing { - let (rect, metrics) = (self.geom.content_inner, self.cell_metrics); + let (rect, metrics) = (self.media_cell_rect(), self.cell_metrics); let natural = self.content_media.as_ref().map(|m| m.natural); player.pull_and_show( &mut self.graphics, @@ -1699,6 +1743,10 @@ impl Controller { match &mut self.media_player { Some(p) => { p.playing = !p.playing; + // Freeze the position on pause, and resume the clock from there on play, so the + // progress bar neither races while paused nor rewinds on resume. + p.run_started_from = p.position; + p.run_started_at = Instant::now(); // Restart the pacing clock so a resume doesn't burst past a stale deadline. p.next_frame_at = Instant::now(); Some(Effects::redraw()) @@ -1743,7 +1791,7 @@ impl Controller { /// the player (the last frame stays; the text placeholder remains beneath). pub fn tick_media(&mut self, now: Instant) -> bool { // Read the live geometry first: `player` borrows `self.media_player` mutably below. - let (rect, metrics) = (self.geom.content_inner, self.cell_metrics); + let (rect, metrics) = (self.media_cell_rect(), self.cell_metrics); let natural = self.content_media.as_ref().map(|m| m.natural); let Some(player) = self.media_player.as_mut() else { return false; @@ -1764,6 +1812,10 @@ impl Controller { if now < player.next_frame_at { return false; // paced: one frame per FRAME_INTERVAL, no bursts } + // Wall-clock position: `-re` paces the decoder to real time, so elapsed time is the + // truthful playback offset. Counting displayed frames would drift slow, because the host + // ceiling means we show fewer frames than the decoder emits. + player.position = player.run_started_from + player.run_started_at.elapsed().as_secs_f64(); player.next_frame_at = now + crate::media::player::FRAME_INTERVAL; player.pull_and_show( &mut self.graphics, @@ -2102,6 +2154,13 @@ impl Controller { // here so the line-select snapshot below stays a pure read. let sel_gutter = self.selection_gutter_len(); ViewState { + media_progress: self.media_progress().map(|(pos, dur)| { + ( + pos, + dur, + self.media_player.as_ref().is_some_and(|p| p.playing), + ) + }), nodes, selected, content: self.content.clone(), @@ -3771,6 +3830,8 @@ enum MouseRegion { TreeVBar, /// The tree's horizontal scrollbar — drag left/right to scroll the tree sideways. TreeHBar, + /// A video's progress bar — press or drag along it to seek. + MediaBar, Outside, } @@ -3786,6 +3847,8 @@ enum Drag { TreeH, /// Dragging the finder overlay's vertical scrollbar (handled in `handle_finder_mouse`). FinderV, + /// Scrubbing a video's progress bar. + MediaBar, /// Dragging out a character-granular text selection in the content pane — in L mode (handled /// in `handle_line_select_mouse`, on the modal's state) or ambient (handled in /// `handle_column_mouse`, on `content_selection`; the release auto-copies). @@ -3802,6 +3865,31 @@ fn is_markdown(path: &Path) -> bool { #[cfg(test)] mod tests { + #[test] + fn media_rect_reserves_the_caption_row_and_the_bar_row() { + let inner = Rect::new(4, 2, 40, 20); + // An image: only the caption row is reserved. + let image = Controller::media_rect(inner, false).expect("fits"); + assert_eq!((image.y, image.height), (3, 19), "caption row reserved"); + // A video with a progress bar: the last row is reserved too, so playback cannot paint + // over either the caption or the bar. + let video = Controller::media_rect(inner, true).expect("fits"); + assert_eq!((video.y, video.height), (3, 18)); + assert_eq!(video.x, inner.x, "full width either way"); + assert_eq!(video.width, inner.width); + } + + #[test] + fn media_rect_is_none_when_there_is_no_room_left() { + // One row of content is all caption; two rows is caption + bar. Neither leaves a picture, + // and returning None is what stops a zero/underflowed rect reaching the host. + assert!(Controller::media_rect(Rect::new(0, 0, 40, 1), false).is_none()); + assert!(Controller::media_rect(Rect::new(0, 0, 40, 2), true).is_none()); + assert!(Controller::media_rect(Rect::new(0, 0, 0, 20), false).is_none()); + // But one row of picture IS enough. + assert!(Controller::media_rect(Rect::new(0, 0, 40, 2), false).is_some()); + } + #[test] fn help_composition_timeout_is_the_single_200ms_budget() { // T-19: the controller must not retain or stack its former help-render timeout. T-17 owns diff --git a/src/controller/mouse.rs b/src/controller/mouse.rs index ccd74dd4..e51aba71 100644 --- a/src/controller/mouse.rs +++ b/src/controller/mouse.rs @@ -109,6 +109,33 @@ impl Controller { } } + /// Seek a playing/paused video to the position the pointer is over on the progress bar. + /// + /// The bar's label ("▶ 0:03 / 0:07 ") occupies its left end, so the scrubbable track starts + /// after it; a press on the label maps to 0.0 rather than being ignored, which is what makes + /// dragging off the left end behave. Inert when there is no bar or no duration. + fn media_seek_to_col(&mut self, col: u16) -> Effects { + let Some(bar) = self.geom.media_bar else { + return Effects::noop(); + }; + let Some((_, duration)) = self.media_progress() else { + return Effects::noop(); + }; + let playing = self.media_player.as_ref().is_some_and(|p| p.playing); + // Measured from the SAME builder the Presenter draws, so the track origin cannot drift. + let label = crate::media::progress_label(0.0, duration, playing) + .chars() + .count() as u16; + let track_x = bar.x.saturating_add(label); + let track_w = bar.width.saturating_sub(label); + if track_w == 0 || duration <= 0.0 { + return Effects::noop(); + } + let offset = col.saturating_sub(track_x).min(track_w.saturating_sub(1)); + let fraction = f64::from(offset) / f64::from(track_w.saturating_sub(1).max(1)); + self.media_seek_to(fraction.clamp(0.0, 1.0) * duration) + } + /// Handle a mouse event over the two columns, with no modal open (the [`handle_mouse`] gate /// routes here only for [`Modal::None`]). Shift+mouse is inert so the terminal can do its own /// text selection; otherwise the wheel scrolls the column under the pointer, a left press @@ -131,6 +158,11 @@ impl Controller { // selection (click-away deselect). Always (re)set `drag` from the press — so a stale // drag from a release we never saw (e.g. swallowed by a modal) can't act on later moves. let region = self.hit_test(col, row); + if region == MouseRegion::MediaBar { + self.content_selection = None; + self.drag = Some(Drag::MediaBar); + return self.media_seek_to_col(col); + } if region == MouseRegion::Content { // Seed a fresh collapsed char selection at the pressed caret; the Drag arm // extends it and Up finalizes (collapsed ⇒ a click; non-collapsed ⇒ copy). @@ -160,6 +192,7 @@ impl Controller { MouseRegion::ContentHBar => Some(Drag::ContentH), MouseRegion::TreeVBar => Some(Drag::TreeV), MouseRegion::TreeHBar => Some(Drag::TreeH), + MouseRegion::MediaBar => Some(Drag::MediaBar), _ => None, }; let fx = match region { @@ -178,6 +211,7 @@ impl Controller { Some(Drag::ContentH) => self.scroll_content_h_to_col(col), Some(Drag::TreeV) => self.scroll_tree_to_row(row), Some(Drag::TreeH) => self.scroll_tree_h_to_col(col), + Some(Drag::MediaBar) => self.media_seek_to_col(col), // The finder is modal: its scrollbar drag is handled in handle_finder_mouse and // never reaches this (non-finder) path. Covered here only for exhaustiveness. Some(Drag::FinderV) => Effects::noop(), @@ -277,12 +311,14 @@ impl Controller { self.focus = Focus::Content; Effects::redraw() } - // Scrollbars are handled on press/drag (above), not as a click; reaching here is inert. + // Scrollbars and the progress bar are handled on press/drag (above), not as a click; + // reaching here is inert. MouseRegion::Divider | MouseRegion::ContentVBar | MouseRegion::ContentHBar | MouseRegion::TreeVBar | MouseRegion::TreeHBar + | MouseRegion::MediaBar | MouseRegion::Outside => { self.last_click = None; Effects::noop() @@ -487,6 +523,11 @@ impl Controller { if self.geom.tree_hbar.is_some_and(|r| r.contains(pos)) { return MouseRegion::TreeHBar; } + // Before the content text rect: the bar is drawn inside the content column, so a press on + // it must seek rather than start a text selection. + if self.geom.media_bar.is_some_and(|r| r.contains(pos)) { + return MouseRegion::MediaBar; + } if let Some(t) = self.geom.tree_inner && t.contains(pos) { diff --git a/src/media.rs b/src/media.rs index 3e4186de..a402cb2e 100644 --- a/src/media.rs +++ b/src/media.rs @@ -120,6 +120,20 @@ pub fn human_duration(seconds: f64) -> String { } } +/// The progress bar's leading label, e.g. `"▶ 0:03 / 0:07 "`. +/// +/// Shared deliberately: the Presenter draws it and the Controller measures it to find where the +/// scrubbable track starts. If the two ever computed it separately, every drag would map to a +/// slightly wrong position and nothing would obviously look broken. +pub fn progress_label(position: f64, duration: f64, playing: bool) -> String { + format!( + "{} {} / {} ", + if playing { "▶" } else { "⏸" }, + human_duration(position), + human_duration(duration), + ) +} + /// The pixel size of a cell rectangle, the budget an image (or a video frame) must fit within. /// /// This is the number handed to ffmpeg's `scale` filter (via `force_original_aspect_ratio`-style @@ -285,6 +299,32 @@ mod tests { ); } + // -- progress bar ------------------------------------------------------ + + #[test] + fn progress_label_shows_play_state_and_both_times() { + assert_eq!(progress_label(3.0, 7.0, true), "▶ 0:03 / 0:07 "); + assert_eq!(progress_label(0.0, 65.0, false), "⏸ 0:00 / 1:05 "); + } + + #[test] + fn progress_label_width_is_stable_across_play_state() { + // The Controller measures this label to locate the scrubbable track while the Presenter + // draws it. If pausing changed its width, every drag would land on a different position + // than the one under the pointer. + let playing = progress_label(3.0, 7.0, true).chars().count(); + let paused = progress_label(3.0, 7.0, false).chars().count(); + assert_eq!(playing, paused); + } + + #[test] + fn human_duration_rolls_over_into_hours() { + assert_eq!(human_duration(0.0), "0:00"); + assert_eq!(human_duration(65.4), "1:05"); + assert_eq!(human_duration(3661.0), "1:01:01"); + assert_eq!(human_duration(f64::NAN), "?"); + } + // -- classification ---------------------------------------------------- #[test] diff --git a/src/presenter.rs b/src/presenter.rs index c117694b..aba96fd0 100644 --- a/src/presenter.rs +++ b/src/presenter.rs @@ -46,6 +46,10 @@ pub struct ViewState { pub content: Text<'static>, /// Non-fatal notices to surface (truncation AC-13, renderer fallback AC-25). pub notices: Vec, + /// `(position, duration)` in seconds for a playing/paused video, and whether it is playing. + /// `Some` only for a video of known duration; the bar occupies the content pane's last row, + /// which the picture placement reserves so the two never overlap. + pub media_progress: Option<(f64, f64, bool)>, /// A self-expiring status hint (e.g. `D`'s diff-presentation label), drawn as one line atop /// the notices strip and styled distinctly from a warning. `None` when nothing is flashing. pub flash: Option, @@ -1168,6 +1172,23 @@ fn draw_content(frame: &mut Frame, area: Rect, state: &ViewState) -> (u16, u16) // No ranges and no overlay: exactly main's pre-annotation path. state.content.clone() }; + // A video's progress bar takes the content pane's last row. Reserving it here (rather than + // painting over the text) keeps it in lockstep with the picture placement, which reserves the + // same row. + let (text, bar_row) = match state.media_progress { + Some(_) if text.height > 1 => ( + Rect { + height: text.height - 1, + ..text + }, + Some(Rect { + y: text.y + text.height - 1, + height: 1, + ..text + }), + ), + _ => (text, None), + }; let mut content = Paragraph::new(content_text).scroll((state.content_scroll, state.content_hscroll)); if state.wrap { @@ -1175,6 +1196,9 @@ fn draw_content(frame: &mut Frame, area: Rect, state: &ViewState) -> (u16, u16) } frame.render_widget(content, text); draw_blank_annotation_cells(frame, text, state); + if let (Some(track), Some((position, duration, playing))) = (bar_row, state.media_progress) { + draw_media_progress(frame, track, position, duration, playing); + } if let Some(track) = vbar { draw_vscrollbar( @@ -1197,6 +1221,49 @@ fn draw_content(frame: &mut Frame, area: Rect, state: &ViewState) -> (u16, u16) (text.width, text.height) } +/// Draw the video progress bar: `▶ 0:03 / 0:07 ━━━━━━╸────────` across one row. +/// +/// The filled run is the elapsed fraction, with a distinct knob glyph at the head so the position +/// is readable at a glance and the drag target is obvious. Purely presentational — the controller +/// owns the position and the seek. +fn draw_media_progress(frame: &mut Frame, row: Rect, position: f64, duration: f64, playing: bool) { + use ratatui::style::{Color, Modifier, Style}; + use ratatui::text::{Line, Span}; + + let label = crate::media::progress_label(position, duration, playing); + let label_width = label.chars().count() as u16; + let track_width = row.width.saturating_sub(label_width); + let fraction = if duration > 0.0 { + (position / duration).clamp(0.0, 1.0) + } else { + 0.0 + }; + // `round` (not floor) so the knob reaches the far end exactly at completion rather than one + // cell short, and saturating_sub keeps a zero-width track from underflowing. + let filled = ((track_width as f64) * fraction).round() as u16; + let filled = filled.min(track_width); + + let mut spans = vec![Span::styled( + label, + Style::default().add_modifier(Modifier::DIM), + )]; + if track_width > 0 { + let head = usize::from(filled.saturating_sub(1)); + spans.push(Span::styled( + "━".repeat(head), + Style::default().fg(Color::Cyan), + )); + if filled > 0 { + spans.push(Span::styled("╸", Style::default().fg(Color::Cyan))); + } + spans.push(Span::styled( + "─".repeat(usize::from(track_width - filled)), + Style::default().add_modifier(Modifier::DIM), + )); + } + frame.render_widget(ratatui::widgets::Paragraph::new(Line::from(spans)), row); +} + /// Split the frame into the body (the two columns) and an optional one-row remote-notice status. /// The status is present exactly when supplied (and the frame is tall enough to spare a row). /// Shared by [`draw`] and [`geometry`] so the drawn layout and the hit-test geometry carve the @@ -1353,6 +1420,9 @@ pub struct PaneGeometry { /// The content pane's in-pane scrollbar tracks (1-cell rects), present only when drawn. pub content_vbar: Option, pub content_hbar: Option, + /// The video progress bar's row, when drawn. Hit-testing maps a press or drag along it to a + /// seek position. + pub media_bar: Option, pub divider_x: Option, /// The screen rect where finder result rows are drawn, `None` when the finder is closed or /// has no rows (empty query or zero matches). Used by the controller to map a mouse click to @@ -1460,6 +1530,26 @@ pub fn geometry(area: Rect, state: &ViewState) -> PaneGeometry { } None => (None, None, None), }; + // The bar occupies the text area's last row — the SAME split `draw_content` performs, so a + // drag lands on the bar actually drawn. + let media_bar = content_inner + .filter(|_| state.media_progress.is_some()) + .and_then(|text| { + (text.height > 1).then(|| Rect { + y: text.y + text.height - 1, + height: 1, + ..text + }) + }); + // `content_inner` is what the caller treats as the text area, so it must exclude the bar row + // exactly as the paint does. + let content_inner = match (content_inner, media_bar) { + (Some(text), Some(_)) => Some(Rect { + height: text.height - 1, + ..text + }), + (other, _) => other, + }; // Finder: if the finder overlay is open, compute its layout with the same helper // `draw_finder_overlay` uses (same `area` = `frame.area()` = the full terminal rect), @@ -1509,6 +1599,7 @@ pub fn geometry(area: Rect, state: &ViewState) -> PaneGeometry { content_title_rect, content_vbar, content_hbar, + media_bar, divider_x, finder_rows, finder_scroll, diff --git a/src/render.rs b/src/render.rs index e28e957b..3e27f51b 100644 --- a/src/render.rs +++ b/src/render.rs @@ -620,7 +620,8 @@ pub const DEFAULT_MEDIA_MAX_BYTES: u64 = 8192 * 1024; /// Carries **raw PNG bytes**; base64 happens in `graphics.rs` at send time so the payload stays /// bytes. Dimensions are parsed at placement time via [`crate::media::png_dimensions`], so a /// re-encode (the PNG fast-path guard) can decide from the actual bytes. -#[derive(Debug, Clone, PartialEq, Eq)] +// No `Eq`: a duration is an f64. `PartialEq` is what the tests actually use. +#[derive(Debug, Clone, PartialEq)] pub struct MediaPayload { pub kind: crate::media::MediaKind, pub png: Vec, @@ -631,6 +632,10 @@ pub struct MediaPayload { /// than the pane just because it had to travel small. The "is this image smaller than the /// box?" question is about the SOURCE, so it is answered with this. pub natural: (u32, u32), + /// A video's length in seconds, when `ffprobe` could report it. Drives the progress bar; + /// `None` for images, and for a video whose duration could not be determined (the bar is then + /// simply not drawn). + pub duration_s: Option, } /// Produce the Media view's content for a media file: a text line (so the pane is never blank — @@ -682,6 +687,7 @@ pub fn render_media( kind, png: fitted.png, natural: (w, h), + duration_s: None, }), ), None => ( @@ -744,6 +750,7 @@ pub fn render_media( kind, png: fitted.png, natural: (w, h), + duration_s: None, }), ), None => ( @@ -831,6 +838,7 @@ pub fn render_media( // fraction of the pane while images, which report their true size, // filled it. natural: probe.native.unwrap_or((w, h)), + duration_s: probe.duration_s, }), ), None => ( diff --git a/tests/controller.rs b/tests/controller.rs index 229a6b4a..72005b32 100644 --- a/tests/controller.rs +++ b/tests/controller.rs @@ -1918,6 +1918,7 @@ fn wide_geometry() -> PaneGeometry { }), content_vbar: None, content_hbar: None, + media_bar: None, divider_x: Some(40), finder_rows: None, finder_scroll: 0, diff --git a/tests/media_shown.rs b/tests/media_shown.rs index 48bfaef1..39b03796 100644 --- a/tests/media_shown.rs +++ b/tests/media_shown.rs @@ -41,6 +41,7 @@ impl ContentProvider for MediaContent { kind: herdr_file_viewer::media::MediaKind::Png, png: png_bytes(4, 3), natural: (4, 3), + duration_s: None, }), }, Some("mp4") => RenderResult { @@ -51,6 +52,7 @@ impl ContentProvider for MediaContent { kind: herdr_file_viewer::media::MediaKind::Video, png: png_bytes(4, 3), // the still preview (frame 0) natural: (4, 3), + duration_s: Some(12.0), }), }, _ => RenderResult { diff --git a/tests/presenter.rs b/tests/presenter.rs index 92035132..82cff568 100644 --- a/tests/presenter.rs +++ b/tests/presenter.rs @@ -67,6 +67,7 @@ fn sample_state() -> ViewState { nodes, selected: 1, // main.rs content: to_text("fn main() {\n println!(\"hello\");\n}\n"), + media_progress: None, notices: vec![ "Showing first 5000 lines (truncated)".to_string(), // AC-13 "delta not found — showing plain diff".to_string(), // AC-25 diff --git a/tests/presenter_narrow.rs b/tests/presenter_narrow.rs index f6ffe621..132e5232 100644 --- a/tests/presenter_narrow.rs +++ b/tests/presenter_narrow.rs @@ -36,6 +36,7 @@ fn state(width: u16, focus: Focus) -> ViewState { selected: 1, content: to_text("fn main() {}\n"), notices: vec!["delta not found — showing plain diff".to_string()], + media_progress: None, flash: None, focus, width, From 73d6e67ab0e7b7699d8f7fb68698bd4cfe2d01e5 Mon Sep 17 00:00:00 2001 From: Charles Ji Date: Sun, 9 Aug 2026 18:25:51 -0400 Subject: [PATCH 5/8] docs: document the media caption, progress bar, and seek 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]. --- CHANGELOG.md | 2 +- docs/keys.md | 1 + docs/renderers.md | 5 ++++- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9feb090e..b2bd79a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ All notable changes to this project are documented here. The format is based on ## [Unreleased] ### Added -- **Media view: images and video rendered inline in the content pane** through herdr's documented `pane.graphics.*` socket API — the bytes travel base64 inside JSON, so no escape sequence ever reaches the terminal and a hostile image still cannot drive it. A `.png` is shown natively; other images convert via `ffmpeg`; video plays at the host-limited ~8 fps with `p` (play/pause), `{`/`}` (seek ±5s), and `0` (restart), starting paused on frame 0. New config keys `image`, `video`, and `media_max_kib`; new remappable intents `media_play_pause`, `media_seek_back`, `media_seek_forward`, `media_restart`. → [usage](docs/usage.md#media-images-and-video) · [renderers](docs/renderers.md#media-images-and-video) · [keys](docs/keys.md) · [configuration](docs/configuration.md) +- **Media view: images and video rendered inline in the content pane** through herdr's documented `pane.graphics.*` socket API — the bytes travel base64 inside JSON, so no escape sequence ever reaches the terminal and a hostile image still cannot drive it. A `.png` is shown natively; other images convert via `ffmpeg`; video plays at the host-limited ~8 fps with `p` (play/pause), `{`/`}` (seek ±5s), and `0` (restart), starting paused on frame 0. A video also carries a caption (resolution, duration, codec, size) and a click/drag-to-seek progress bar. New config keys `image`, `video`, and `media_max_kib`; new remappable intents `media_play_pause`, `media_seek_back`, `media_seek_forward`, `media_restart`. → [usage](docs/usage.md#media-images-and-video) · [renderers](docs/renderers.md#media-images-and-video) · [keys](docs/keys.md) · [configuration](docs/configuration.md) ### Fixed - Agent skill: the launch instructions no longer tell agents to pass `--cwd`. herdr resolves the manifest's relative pane command against it, so the launch failed with `plugin_pane_open_failed` — or worse, inside a built plugin checkout, silently ran that checkout's binary. The skill and the `docs/usage.md` snippet now explain that the viewed root follows the *focused herdr pane's* directory, so an agent's own `cd` does not move it. Thanks @AntonyKor (#139) → [agent skill](skills/herdr-file-viewer/SKILL.md) · [usage](docs/usage.md#teach-your-agent) diff --git a/docs/keys.md b/docs/keys.md index cd806bdc..c1e7a75b 100644 --- a/docs/keys.md +++ b/docs/keys.md @@ -139,6 +139,7 @@ The viewer is keyboard-first; the mouse is additive and on by default: | **Horizontal wheel / swipe** | Scroll the content, or the tree, sideways (terminal-dependent, see below) | | **Drag** a scrollbar | Scroll that pane: drag ↕ on a vertical bar, ↔ on a horizontal bar; pressing the track jumps there | | **Drag** the divider | Resize the tree / content split | +| **Click / drag** a video's progress bar | Seek to that position (the bar occupies the content pane's last row while a video is selected) | | **Drag** over the content text | **Select and copy text**: the selection highlights character-by-character as you drag (auto-scrolling past an edge) and is copied to the clipboard on release; no mode needed. Works in wrapped views (prose/markdown) too. `Esc`, a click elsewhere, or switching files clears the highlight | **`Shift`+drag is left to your terminal**, so its native select-and-copy still works while the diff --git a/docs/renderers.md b/docs/renderers.md index a7f6f147..4d839dce 100644 --- a/docs/renderers.md +++ b/docs/renderers.md @@ -61,7 +61,10 @@ terminal). What ffmpeg is needed for: displayed. Without it ffmpeg races to the end of the file (a 7-second clip decodes in under half a second), the queue discards almost every frame, and playback appears to stop immediately. The poster frame and playback are decoded at the same target size, so pressing `p` never resizes the - picture. + picture. Pausing holds the frame you paused on rather than reverting to the poster. +- **A progress bar** occupies the content pane's last row while a video of known duration is + selected — `▶ 0:03 / 0:07 ━━━━━━╸────────`. Click or drag anywhere along it to seek. The picture + reserves that row, so the bar and the video never overlap. - **A caption above the picture** reports what you are looking at, e.g. `[image: 3008×1546 · PNG · 8-bit RGBA · 655 KiB]` or `[video: 854×480 · 0:07 · HEVC · 982 KiB · p to play]`. Colour depth and type come from the PNG From a0a46c5143c90115dc1c9ac85165415236540a72 Mon Sep 17 00:00:00 2001 From: Charles Ji Date: Sun, 9 Aug 2026 18:40:02 -0400 Subject: [PATCH 6/8] fix(media): seeking a paused video shows that frame and stays paused 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. --- src/controller/mod.rs | 48 +++++++++++++++++++++++++++++++++---------- tests/media_shown.rs | 45 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 11 deletions(-) diff --git a/src/controller/mod.rs b/src/controller/mod.rs index e33ea5a9..3dc45880 100644 --- a/src/controller/mod.rs +++ b/src/controller/mod.rs @@ -221,6 +221,11 @@ struct MediaPlayer { /// Whether a decoded frame has actually been shown. Once true, the poster still must never be /// put back — pausing should hold the frame you paused ON, not jump to frame 0. has_frame: bool, + /// A frame is owed even though playback is paused: the decoder was just (re)spawned by a seek + /// or restart, and the frame AT that position has not arrived yet. Without this a paused seek + /// shows nothing until you press play — the decoder produces the frame a moment after the + /// spawn, and a paused tick would never collect it. + awaiting_seek_frame: bool, } impl MediaPlayer { @@ -274,6 +279,7 @@ impl MediaPlayer { placement, }); self.has_frame = true; + self.awaiting_seek_frame = false; // the seek's frame has landed true } @@ -1723,6 +1729,7 @@ impl Controller { run_started_at: Instant::now(), run_started_from: position, has_frame: false, + awaiting_seek_frame: true, }; if !playing { let (rect, metrics) = (self.media_cell_rect(), self.cell_metrics); @@ -1741,6 +1748,14 @@ impl Controller { /// `Space` on a selected video: play if paused/idle, pause if playing. fn media_play_pause(&mut self) -> Effects { match &mut self.media_player { + // Resuming a video whose decoder already drained (paused past its end, or paused long + // enough after a seek) needs a fresh decoder at the current position — the old one has + // nothing left to give. + Some(p) if !p.playing && p.decoder.finished() => { + let position = p.position; + self.media_start(position, true); + Some(Effects::redraw()) + } Some(p) => { p.playing = !p.playing; // Freeze the position on pause, and resume the clock from there on play, so the @@ -1776,11 +1791,15 @@ impl Controller { /// Seek to an absolute offset; `0` restarts. Preserves the playing state across the spawn. fn media_seek_to(&mut self, position: f64) -> Effects { + // Seeking preserves the play state, and an idle video (no player yet — the poster still is + // what is on screen) stays PAUSED. Defaulting to "playing" here meant scrubbing the bar or + // pressing `{`/`}` silently started playback, which then ran to the end and reset the + // position; `p` is the only thing that starts a video. let playing = self .media_player .as_ref() .map(|p| p.playing) - .unwrap_or(true); + .unwrap_or(false); self.media_start(position, playing); Effects::redraw() } @@ -1797,25 +1816,32 @@ impl Controller { return false; }; if player.decoder.finished() { - // Decoder drained: no more frames will come. Fall back to the sync_media discipline, - // which will clear (the video's still preview text remains) — and drop the player - // so `p`/seeks no longer nop into a dead decoder. - let ended = player.playing; + // Only a PLAYING video has ended. A paused one keeps its player: the decoder runs on + // to EOF in the background after a seek (it is `-re` paced, so this happens seconds + // later), and dropping the player there would silently discard the position — the bar + // fell back to 0:00 and the next `p` restarted from the beginning. Playback resumes by + // respawning at `position`; see `media_play_pause`. + if !player.playing { + return false; + } if let Some(p) = self.media_player.take() { p.stop(); } - return ended; + return true; } - if !player.playing { + if !player.playing && !player.awaiting_seek_frame { return false; } if now < player.next_frame_at { return false; // paced: one frame per FRAME_INTERVAL, no bursts } - // Wall-clock position: `-re` paces the decoder to real time, so elapsed time is the - // truthful playback offset. Counting displayed frames would drift slow, because the host - // ceiling means we show fewer frames than the decoder emits. - player.position = player.run_started_from + player.run_started_at.elapsed().as_secs_f64(); + if player.playing { + // Wall-clock position: `-re` paces the decoder to real time, so elapsed time is the + // truthful playback offset. Counting displayed frames would drift slow, because the + // host ceiling means we show fewer frames than the decoder emits. + player.position = + player.run_started_from + player.run_started_at.elapsed().as_secs_f64(); + } player.next_frame_at = now + crate::media::player::FRAME_INTERVAL; player.pull_and_show( &mut self.graphics, diff --git a/tests/media_shown.rs b/tests/media_shown.rs index 39b03796..ad5de46b 100644 --- a/tests/media_shown.rs +++ b/tests/media_shown.rs @@ -361,6 +361,51 @@ fn build_video(root: &Path) -> (Controller, RecordingSink) { (ctrl, sink) } +#[test] +fn seeking_an_idle_video_previews_the_frame_without_starting_playback() { + // Two regressions in one journey. Seeking used to default to "playing" when no player existed + // yet, so scrubbing the bar (or `{`/`}`) silently STARTED the video, which then ran to its end + // and reset the position to 0. And a paused tick pulled no frames at all, so even once the + // seek worked the pane showed nothing until you pressed play. + let dir = TempDir::new(); + std::fs::write(dir.path().join("clip.mp4"), b"not a real mp4").unwrap(); + let (mut ctrl, sink) = build_video(dir.path()); + ctrl.set_pane_geometry(geom_with_inner(Rect::new(0, 0, 40, 24))); + await_content(&mut ctrl, "p to play"); + ctrl.sync_media(); + let before = sink.commands.lock().unwrap().len(); + + ctrl.handle(Intent::MediaSeekForward); + + // A frame for the sought position must arrive WITHOUT any play keypress. + let deadline = Instant::now() + Duration::from_secs(5); + loop { + ctrl.tick_media(Instant::now()); + if sink.commands.lock().unwrap().len() > before { + break; + } + assert!( + Instant::now() < deadline, + "a paused seek must still show the frame at that position" + ); + std::thread::yield_now(); + } + + // ...and the video must still be PAUSED. `media_progress` reports the sought offset, and the + // position must not advance on its own the way a playing video's would. + let (position, _duration) = ctrl + .media_progress() + .expect("a video with a known duration"); + assert!(position > 0.0, "the seek moved the position: {position}"); + std::thread::sleep(Duration::from_millis(250)); + ctrl.tick_media(Instant::now()); + let (later, _) = ctrl.media_progress().expect("still a video"); + assert_eq!( + position, later, + "a paused video's position must not advance -- seeking must not have started playback" + ); +} + #[test] fn video_play_pause_seek_restart_drive_the_decoder() { let dir = TempDir::new(); From 1ed13b93da3530f4465e8c7ec73eeb3672351ffd Mon Sep 17 00:00:00 2001 From: Charles Ji Date: Sun, 9 Aug 2026 18:52:20 -0400 Subject: [PATCH 7/8] fix(root): never root the viewer at its own install directory 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 ` does the host-side scoping. --- CHANGELOG.md | 1 + src/app.rs | 26 ++++++++++- src/host.rs | 66 +++++++++++++++++++++++++- tests/host_context.rs | 106 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 196 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b2bd79a5..fc6ec5a3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ All notable changes to this project are documented here. The format is based on - **Media view: images and video rendered inline in the content pane** through herdr's documented `pane.graphics.*` socket API — the bytes travel base64 inside JSON, so no escape sequence ever reaches the terminal and a hostile image still cannot drive it. A `.png` is shown natively; other images convert via `ffmpeg`; video plays at the host-limited ~8 fps with `p` (play/pause), `{`/`}` (seek ±5s), and `0` (restart), starting paused on frame 0. A video also carries a caption (resolution, duration, codec, size) and a click/drag-to-seek progress bar. New config keys `image`, `video`, and `media_max_kib`; new remappable intents `media_play_pause`, `media_seek_back`, `media_seek_forward`, `media_restart`. → [usage](docs/usage.md#media-images-and-video) · [renderers](docs/renderers.md#media-images-and-video) · [keys](docs/keys.md) · [configuration](docs/configuration.md) ### Fixed +- Opening the viewer while a viewer pane is focused no longer roots 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 instead. → [usage](docs/usage.md) - Agent skill: the launch instructions no longer tell agents to pass `--cwd`. herdr resolves the manifest's relative pane command against it, so the launch failed with `plugin_pane_open_failed` — or worse, inside a built plugin checkout, silently ran that checkout's binary. The skill and the `docs/usage.md` snippet now explain that the viewed root follows the *focused herdr pane's* directory, so an agent's own `cd` does not move it. Thanks @AntonyKor (#139) → [agent skill](skills/herdr-file-viewer/SKILL.md) · [usage](docs/usage.md#teach-your-agent) ## [1.15.0] - 2026-08-03 diff --git a/src/app.rs b/src/app.rs index ef6c43da..3f43c213 100644 --- a/src/app.rs +++ b/src/app.rs @@ -47,7 +47,31 @@ const RENDER_TIMEOUT: Duration = Duration::from_secs(5); /// [`crate::open_target::OPEN_ENV`] (`HERDR_FILE_VIEWER_OPEN`) as flag > env; an absent/empty /// pair leaves startup selection unchanged. pub fn run(open_flag: Option) -> io::Result<()> { - let ctx = host::from_env(); + let mut ctx = host::from_env(); + // If the only root herdr offered is the plugin's OWN install directory — which is what both + // context fields report when the viewer is opened while a viewer pane is focused — ask herdr + // for the workspace's other panes and root at one of those instead. Best-effort: a missing or + // failing herdr leaves the context exactly as parsed. + if host::is_own_install_dir(&ctx.cwd, std::env::current_exe().ok().as_deref()) { + use crate::herdr::HerdrCli; + let cli = crate::herdr::LiveHerdr::from_env(); + // `herdr pane list` emits JSON by default (there is no `--json` flag — passing one makes + // it print "unknown option" and the parse silently yields nothing). `--workspace` scopes + // it host-side; the parser filters again so a herdr without that flag still behaves. + let args: Vec<&str> = match ctx.workspace_id.as_deref() { + Some(id) => vec!["pane", "list", "--workspace", id], + None => vec!["pane", "list"], + }; + if let Ok(json) = cli.run_json(&args) + && let Some(better) = host::root_from_sibling_panes( + &json, + ctx.workspace_id.as_deref(), + std::env::current_exe().ok().as_deref(), + ) + { + ctx.cwd = better; + } + } let resolved = root::resolve(&ctx); let baseline = git::default_baseline(&resolved); diff --git a/src/host.rs b/src/host.rs index ca3c0529..d440639f 100644 --- a/src/host.rs +++ b/src/host.rs @@ -28,20 +28,45 @@ struct RawContext { pub fn from_env() -> LaunchContext { let json = std::env::var("HERDR_PLUGIN_CONTEXT_JSON").ok(); let cwd = std::env::current_dir().unwrap_or_default(); - parse_context(json.as_deref(), cwd) + parse_context_from(json.as_deref(), cwd, std::env::current_exe().ok()) } /// Pure parser behind [`from_env`] (testable without touching process env). Missing or /// malformed JSON yields a minimal `{ cwd: fallback_cwd }` context (AC-26). pub fn parse_context(json: Option<&str>, fallback_cwd: PathBuf) -> LaunchContext { + parse_context_from(json, fallback_cwd, None) +} + +/// [`parse_context`] plus the viewer's own executable path, used to recognise (and skip) a +/// focused-pane cwd that is the plugin's OWN install directory. +/// +/// Why that matters: herdr launches the pane from the plugin root (the manifest command is +/// relative), so a viewer pane's cwd *is* the plugin dir. Open the viewer while a viewer is +/// focused — the natural thing to do, since the plugin's whole point is browsing — and +/// `focused_pane_cwd` reports the plugin's install directory, rooting the new viewer at +/// `~/.config/herdr/plugins/github/herdr-file-viewer-…` instead of the user's project. Falling +/// through to `workspace_cwd` gives the workspace the user is actually working in. +/// +/// Only an EXACT match or an ancestor of our own binary is skipped, so a real project that merely +/// sits above the plugin dir is unaffected, and so is any subdirectory the user browses to. +pub fn parse_context_from( + json: Option<&str>, + fallback_cwd: PathBuf, + own_exe: Option, +) -> LaunchContext { let raw: RawContext = json .and_then(|s| serde_json::from_str(s).ok()) .unwrap_or_default(); + let is_own_install_dir = |candidate: &str| { + own_exe + .as_ref() + .is_some_and(|exe| exe.starts_with(PathBuf::from(candidate))) + }; // Ignore empty-string fields (a malformed host value) so they fall through to the next // candidate / the process-cwd fallback rather than rooting at an empty path. let cwd = raw .focused_pane_cwd - .filter(|s| !s.is_empty()) + .filter(|s| !s.is_empty() && !is_own_install_dir(s)) .or(raw.workspace_cwd.filter(|s| !s.is_empty())) .or(raw.cwd.filter(|s| !s.is_empty())) .map(PathBuf::from) @@ -52,3 +77,40 @@ pub fn parse_context(json: Option<&str>, fallback_cwd: PathBuf) -> LaunchContext workspace_id: raw.workspace_id.filter(|s| !s.is_empty()), } } + +/// Whether `dir` is the plugin's own install directory — i.e. our executable lives inside it. +pub fn is_own_install_dir(dir: &std::path::Path, own_exe: Option<&std::path::Path>) -> bool { + own_exe.is_some_and(|exe| exe.starts_with(dir)) +} + +/// Pick a viewed root from the workspace's OTHER panes when the launch context only offers the +/// plugin's own install directory. +/// +/// herdr derives both `focused_pane_cwd` and `workspace_cwd` from the focused pane, and a viewer +/// pane's cwd is the plugin root (its command is relative, so herdr launches it from there). Open +/// the viewer while a viewer is focused and BOTH fields therefore name the plugin's install +/// directory — the fallback chain inside [`parse_context_from`] has nothing better to offer, and +/// the tree shows the plugin's own source instead of the user's project. +/// +/// This asks herdr for the panes in the same workspace and returns the first cwd that is not +/// inside our install directory. Pure over the JSON so it is testable without a live herdr; the +/// caller supplies the document. +pub fn root_from_sibling_panes( + panes_json: &str, + workspace_id: Option<&str>, + own_exe: Option<&std::path::Path>, +) -> Option { + let doc: serde_json::Value = serde_json::from_str(panes_json).ok()?; + let panes = doc.get("result")?.get("panes")?.as_array()?; + panes + .iter() + .filter(|p| match workspace_id { + // Same workspace only: another workspace's pane is a different piece of work. + Some(id) => p.get("workspace_id").and_then(|v| v.as_str()) == Some(id), + None => true, + }) + .filter_map(|p| p.get("cwd").and_then(|v| v.as_str())) + .filter(|cwd| !cwd.is_empty()) + .find(|cwd| own_exe.is_none_or(|exe| !exe.starts_with(PathBuf::from(cwd)))) + .map(PathBuf::from) +} diff --git a/tests/host_context.rs b/tests/host_context.rs index 43d15ab1..8a3b3800 100644 --- a/tests/host_context.rs +++ b/tests/host_context.rs @@ -115,3 +115,109 @@ fn malformed_json_still_yields_none_workspace_id() { let ctx = parse_context(Some("{ this is not json"), PathBuf::from("/fallback")); assert_eq!(ctx.workspace_id, None); } + +#[test] +fn a_focused_pane_inside_the_plugins_own_install_dir_falls_through_to_the_workspace() { + // Opening the viewer while a VIEWER pane is focused: herdr launches the pane from the plugin + // root (the manifest command is relative), so the focused pane's cwd is the plugin's own + // install directory. Rooting there showed the user + // `~/.config/herdr/plugins/github/herdr-file-viewer-…` instead of their project. + let plugin = "/Users/x/.config/herdr/plugins/github/herdr-file-viewer-abc123"; + let json = + format!(r#"{{"focused_pane_cwd":"{plugin}","workspace_cwd":"/Users/x/dev/project"}}"#); + let ctx = herdr_file_viewer::host::parse_context_from( + Some(&json), + PathBuf::from("/fallback"), + Some(PathBuf::from(format!( + "{plugin}/target/release/herdr-file-viewer" + ))), + ); + assert_eq!( + ctx.cwd, + PathBuf::from("/Users/x/dev/project"), + "the plugin's own dir must never be the viewed root when a workspace is known" + ); +} + +#[test] +fn an_ordinary_focused_pane_still_wins_over_the_workspace() { + // The skip must be narrow: only our OWN install dir is ignored. A normal project directory — + // even one that happens to sit near the plugin — is still the most specific answer. + let json = + r#"{"focused_pane_cwd":"/Users/x/dev/project/src","workspace_cwd":"/Users/x/dev/project"}"#; + let ctx = herdr_file_viewer::host::parse_context_from( + Some(json), + PathBuf::from("/fallback"), + Some(PathBuf::from( + "/Users/x/.config/herdr/plugins/github/hfv/target/release/herdr-file-viewer", + )), + ); + assert_eq!(ctx.cwd, PathBuf::from("/Users/x/dev/project/src")); +} + +#[test] +fn the_plugin_dir_is_still_used_when_there_is_nothing_better() { + // Degrade, don't break: with no workspace and no cwd in the context, the plugin dir is all we + // have and is better than an empty root. + let plugin = "/plugins/hfv"; + let json = format!(r#"{{"focused_pane_cwd":"{plugin}"}}"#); + let ctx = herdr_file_viewer::host::parse_context_from( + Some(&json), + PathBuf::from("/fallback"), + Some(PathBuf::from(format!( + "{plugin}/target/release/herdr-file-viewer" + ))), + ); + assert_eq!(ctx.cwd, PathBuf::from("/fallback")); +} + +#[test] +fn a_sibling_pane_supplies_the_root_when_the_context_only_offers_the_plugin_dir() { + // herdr derives BOTH context cwds from the focused pane, so opening the viewer while a viewer + // is focused reports the plugin's install dir twice over and the fallback chain has nothing + // better. The workspace's other panes do. + let plugin = "/Users/x/.config/herdr/plugins/github/hfv"; + let exe = PathBuf::from(format!("{plugin}/target/release/herdr-file-viewer")); + let json = format!( + r#"{{"result":{{"panes":[ + {{"pane_id":"w1:p1","workspace_id":"w1","cwd":"{plugin}"}}, + {{"pane_id":"w1:p2","workspace_id":"w1","cwd":"/Users/x/dev/project"}} + ]}}}}"# + ); + let root = herdr_file_viewer::host::root_from_sibling_panes(&json, Some("w1"), Some(&exe)); + assert_eq!(root, Some(PathBuf::from("/Users/x/dev/project"))); +} + +#[test] +fn sibling_panes_in_another_workspace_are_not_borrowed() { + // A pane in a different workspace is different work; rooting there would be worse than the + // plugin dir, because it would look plausible. + let plugin = "/plugins/hfv"; + let exe = PathBuf::from(format!("{plugin}/target/release/herdr-file-viewer")); + let json = format!( + r#"{{"result":{{"panes":[ + {{"pane_id":"w1:p1","workspace_id":"w1","cwd":"{plugin}"}}, + {{"pane_id":"w2:p1","workspace_id":"w2","cwd":"/somewhere/else"}} + ]}}}}"# + ); + assert_eq!( + herdr_file_viewer::host::root_from_sibling_panes(&json, Some("w1"), Some(&exe)), + None + ); +} + +#[test] +fn malformed_pane_json_yields_no_root_rather_than_panicking() { + for bad in [ + "not json", + "{}", + r#"{"result":{}}"#, + r#"{"result":{"panes":[]}}"#, + ] { + assert_eq!( + herdr_file_viewer::host::root_from_sibling_panes(bad, Some("w1"), None), + None, + "input {bad:?}" + ); + } +} From 66509ef3fdf8857a06f1ef524145201c88bf6a5a Mon Sep 17 00:00:00 2001 From: Charles Ji Date: Sun, 9 Aug 2026 19:08:31 -0400 Subject: [PATCH 8/8] feat(media): media outranks "changed" and offers no diff views 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. --- CHANGELOG.md | 2 +- docs/usage.md | 11 +++++++--- src/view_policy.rs | 53 +++++++++++++++++++++++++++------------------- 3 files changed, 40 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fc6ec5a3..568274fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ All notable changes to this project are documented here. The format is based on ## [Unreleased] ### Added -- **Media view: images and video rendered inline in the content pane** through herdr's documented `pane.graphics.*` socket API — the bytes travel base64 inside JSON, so no escape sequence ever reaches the terminal and a hostile image still cannot drive it. A `.png` is shown natively; other images convert via `ffmpeg`; video plays at the host-limited ~8 fps with `p` (play/pause), `{`/`}` (seek ±5s), and `0` (restart), starting paused on frame 0. A video also carries a caption (resolution, duration, codec, size) and a click/drag-to-seek progress bar. New config keys `image`, `video`, and `media_max_kib`; new remappable intents `media_play_pause`, `media_seek_back`, `media_seek_forward`, `media_restart`. → [usage](docs/usage.md#media-images-and-video) · [renderers](docs/renderers.md#media-images-and-video) · [keys](docs/keys.md) · [configuration](docs/configuration.md) +- **Media view: images and video rendered inline in the content pane** through herdr's documented `pane.graphics.*` socket API — the bytes travel base64 inside JSON, so no escape sequence ever reaches the terminal and a hostile image still cannot drive it. A `.png` is shown natively; other images convert via `ffmpeg`; video plays at the host-limited ~8 fps with `p` (play/pause), `{`/`}` (seek ±5s), and `0` (restart), starting paused on frame 0. Media outranks "changed": an edited image or video shows the media rather than a diff of compressed binary, and offers no diff views in the `v` cycle. A video also carries a caption (resolution, duration, codec, size) and a click/drag-to-seek progress bar. New config keys `image`, `video`, and `media_max_kib`; new remappable intents `media_play_pause`, `media_seek_back`, `media_seek_forward`, `media_restart`. → [usage](docs/usage.md#media-images-and-video) · [renderers](docs/renderers.md#media-images-and-video) · [keys](docs/keys.md) · [configuration](docs/configuration.md) ### Fixed - Opening the viewer while a viewer pane is focused no longer roots 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 instead. → [usage](docs/usage.md) diff --git a/docs/usage.md b/docs/usage.md index 14e97d21..7573beb3 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -163,9 +163,14 @@ This is launch-only. It does not retarget a Files pane that is already running; ## Viewing a file -The content pane shows **the right view for each file, automatically**: a changed file shows its -**diff**, a markdown file **renders**, anything else is **syntax-highlighted** content with line -numbers. No mode-switching, no commands. +The content pane shows **the right view for each file, automatically**: an image or video shows +**the media**, a changed file shows its **diff**, a markdown file **renders**, anything else is +**syntax-highlighted** content with line numbers. No mode-switching, no commands. + +Media outranks "changed" — an edited screenshot still shows the picture, because a diff of +compressed binary is noise and the image is the thing you wanted to see. Media files offer no diff +views at all; `v` steps straight to the plain text beneath (for a text format like SVG, that is the +real source). - **Cycle the view** with `v` to override the automatic choice (e.g. see a changed markdown file's raw source instead of its diff). diff --git a/src/view_policy.rs b/src/view_policy.rs index adc33dc5..4b5e145d 100644 --- a/src/view_policy.rs +++ b/src/view_policy.rs @@ -35,11 +35,16 @@ pub struct FileDescriptor { } /// The auto-selected default view mode for a file. +/// +/// Media outranks "changed", unlike every other kind of file. AC-9's changed-wins rule exists so +/// an edited file shows what you edited, but a diff of an image or a video is a diff of compressed +/// binary: delta renders noise, and the one thing you actually want — to see the picture — is the +/// thing you cannot get. Media files therefore always show the media. pub fn default_mode(fd: &FileDescriptor) -> ViewMode { - if fd.is_changed { - ViewMode::Diff - } else if fd.media.is_some() { + if fd.media.is_some() { ViewMode::Media + } else if fd.is_changed { + ViewMode::Diff } else if fd.is_markdown { ViewMode::RenderedMarkdown } else { @@ -49,8 +54,11 @@ pub fn default_mode(fd: &FileDescriptor) -> ViewMode { /// The modes a cycle key steps through for a file, default first (AC-11). A changed file /// also offers a full-context diff (whole file + line numbers + inline diff) right after -/// the compact diff; markdown adds its rendered view; media adds a media view; every file -/// ends with syntax content. +/// the compact diff; markdown adds its rendered view; every file ends with syntax content. +/// +/// **Media offers no diff views at all**, even when changed: there is nothing legible to show. +/// The cycle is therefore `[Media, SyntaxContent]` — and for a text-based format like SVG that +/// second step is the real source, so nothing is actually lost but the diff itself. pub fn applicable_modes(fd: &FileDescriptor) -> Vec { let mut modes = vec![default_mode(fd)]; let add = |modes: &mut Vec, m: ViewMode| { @@ -58,7 +66,7 @@ pub fn applicable_modes(fd: &FileDescriptor) -> Vec { modes.push(m); } }; - if fd.is_changed { + if fd.is_changed && fd.media.is_none() { add(&mut modes, ViewMode::Diff); add(&mut modes, ViewMode::FullDiff); } @@ -171,27 +179,28 @@ mod tests { } #[test] - fn media_defaults_to_media_unless_changed() { - // A media file defaults to Media, but a changed media file is Diff (git first). + fn media_always_shows_the_media_even_when_changed() { + // Media outranks AC-9's changed-wins rule. A diff of a PNG or an MP4 is a diff of + // compressed binary — delta renders noise, and the picture, which is the only thing worth + // looking at, is unreachable. So an edited image still shows the image. assert_eq!(default_mode(&media_fd("image.png", false)), ViewMode::Media); - assert_eq!(default_mode(&media_fd("image.png", true)), ViewMode::Diff); + assert_eq!(default_mode(&media_fd("image.png", true)), ViewMode::Media); + assert_eq!(default_mode(&media_fd("clip.mp4", true)), ViewMode::Media); + // The rule is scoped to media: an ordinary changed file still defaults to its diff. + assert_eq!(default_mode(&fd("main.rs", false, true)), ViewMode::Diff); } #[test] - fn media_cycle_offers_media_then_plain_content() { - // `Tab` still reaches the plain placeholder text beneath the image (AC-11). - let modes = applicable_modes(&media_fd("image.png", false)); - assert_eq!(modes, vec![ViewMode::Media, ViewMode::SyntaxContent]); - // A changed media file puts the diffs first, media next, plain content last. - let changed = applicable_modes(&media_fd("image.png", true)); + fn media_never_offers_a_diff_view_in_its_cycle() { + // `Tab` reaches the plain text beneath (AC-11) — for a text-based format like SVG that is + // the real source — but never a diff, changed or not. + let expected = vec![ViewMode::Media, ViewMode::SyntaxContent]; + assert_eq!(applicable_modes(&media_fd("image.png", false)), expected); assert_eq!( - changed, - vec![ - ViewMode::Diff, - ViewMode::FullDiff, - ViewMode::Media, - ViewMode::SyntaxContent - ] + applicable_modes(&media_fd("image.png", true)), + expected, + "a changed image must not offer a binary diff" ); + assert_eq!(applicable_modes(&media_fd("clip.mp4", true)), expected); } }