fix(recording): stop wf-recorder start-race producing 0.5s fragment files

On Wayland the video start path signalled any running wf-recorder with pkill -INT and then waited a fixed 0.1s before launching a new one. Releasing the compositor capture takes longer than that, so the new wf-recorder launched into a still-held grab, died immediately, and ffmpeg finalized a ~0.5-second fragment .mkv. Seeing nothing record, I'd press the key again and burst out a pile of fragments until the capture finally freed.

I replaced the fixed wait with a poll that waits until no wf-recorder remains (capped at 2s), the same move the stop path already made when it swapped its own fixed wait for cj/recording--wait-for-exit. Two supporting changes came with it. wf-recorder stderr no longer goes to /dev/null, so a failed grab shows up in the process buffer. And the sentinel now tells a ~0.5s failed start from a real stop, saying "failed to start, try again" instead of silently clearing, so one failure doesn't snowball into a blind-retry burst.

The poll and the fail-fast timing are unit-tested. The live capture needs a manual check.

3 files changed, 122 insertions, 3 deletions

diff --git a/modules/video-audio-recording-capture.el b/modules/video-audio-recording-capture.el
index ea0d687c..41b72ebd 100644
--- a/modules/video-audio-recording-capture.el
+++ b/modules/video-audio-recording-capture.el
@@ -70,7 +70,16 @@ is killed externally."
       (message "Audio recording stopped: %s" (string-trim event)))
      ((eq process cj/video-recording-ffmpeg-process)
       (setq cj/video-recording-ffmpeg-process nil)
-      (message "Video recording stopped: %s" (string-trim event))))
+      (let ((start (process-get process 'cj-start-time)))
+        (if (and start
+                 (not (process-get process 'cj-stopping))
+                 (cj/recording--start-failed-p (- (float-time) start)
+                                               cj/recording-start-fail-threshold))
+            ;; Died almost immediately and the user didn't stop it: wf-recorder
+            ;; couldn't grab the screen. Say so instead of silently clearing,
+            ;; so Craig isn't left blind-retrying (which bursts fragment files).
+            (message "Video recording failed to start (wf-recorder couldn't grab the screen). Try again.")
+          (message "Video recording stopped: %s" (string-trim event))))))
     (force-mode-line-update t)))
 
 (defun cj/recording--wait-for-exit (process timeout-secs)
@@ -87,6 +96,39 @@ so a fixed 0.5s wait was causing zero-byte output files."
       (accept-process-output process 0.1))
     (not (process-live-p process))))
 
+(defvar cj/recording-wf-recorder-wait-timeout 2.0
+  "Seconds to wait for a dying wf-recorder to release the compositor capture.
+Bounds the start-path poll in `cj/ffmpeg-record-video' so it never hangs.")
+
+(defvar cj/recording-start-fail-threshold 1.5
+  "Seconds below which a video recording that exits is treated as a failed start.
+A wf-recorder that can't grab the screen dies almost immediately (~0.5s); a real
+recording runs far longer, so an exit sooner than this is a start failure, not a
+normal stop.")
+
+(defun cj/recording--wf-recorder-running-p ()
+  "Return non-nil if any wf-recorder process is currently running."
+  (eq 0 (call-process "pgrep" nil nil nil "-x" "wf-recorder")))
+
+(defun cj/recording--wait-for-no-wf-recorder (timeout-secs &optional running-p)
+  "Poll until no wf-recorder remains, or TIMEOUT-SECS elapse.
+Returns t if wf-recorder cleared within the timeout, nil on timeout.  RUNNING-P
+is the predicate checked each poll (default `cj/recording--wf-recorder-running-p');
+tests inject a fake.  This replaces a fixed `sit-for' after the start-path
+`pkill -INT wf-recorder': the kill signals the old recorder to finalize and
+exit, but releasing the compositor capture takes longer than a fixed wait, so
+launching too soon loses the grab and produces a ~0.5s fragment file."
+  (let ((check (or running-p #'cj/recording--wf-recorder-running-p))
+        (deadline (+ (float-time) timeout-secs)))
+    (while (and (funcall check) (< (float-time) deadline))
+      (sleep-for 0.05))
+    (not (funcall check))))
+
+(defun cj/recording--start-failed-p (elapsed threshold)
+  "Return non-nil when ELAPSED seconds since start is below THRESHOLD.
+A failed wf-recorder start exits almost immediately; a real recording does not."
+  (< elapsed threshold))
+
 ;;; Dependency Checks
 
 (defun cj/recording-check-ffmpeg ()
@@ -199,7 +241,10 @@ On X11: ffmpeg captures screen directly via x11grab with PulseAudio audio."
   (if on-wayland
       (progn
         (cj/recording--check-wf-recorder)
-        (format (concat "wf-recorder -y -c libx264 -m matroska -f /dev/stdout 2>/dev/null | "
+        ;; wf-recorder stderr is NOT discarded: it flows to the process buffer
+        ;; (*ffmpeg-video-recording*) so a failed capture grab is diagnosable
+        ;; instead of silent.
+        (format (concat "wf-recorder -y -c libx264 -m matroska -f /dev/stdout | "
                         "ffmpeg -i pipe:0 "
                         "-f pulse -i %s "
                         "-f pulse -i %s "
@@ -264,9 +309,13 @@ Uses wf-recorder on Wayland, x11grab on X11."
     ;; kill on purpose: the orphans' launching shells are already dead, so
     ;; there is no live PID to scope to. The stop path, by contrast, scopes
     ;; to our own shell's child (see cj/recording--interrupt-child-wf-recorder).
+    ;; Wait for the signalled wf-recorder to actually exit and release the
+    ;; compositor capture before launching a new one. A fixed `sit-for' here
+    ;; raced the dying recorder and left ~0.5s fragment files (same class the
+    ;; stop path already fixed with `cj/recording--wait-for-exit').
     (when (cj/recording--wayland-p)
       (call-process "pkill" nil nil nil "-INT" "wf-recorder")
-      (sit-for 0.1))
+      (cj/recording--wait-for-no-wf-recorder cj/recording-wf-recorder-wait-timeout))
     (let* ((devices (cj/recording-get-devices))
            (mic-device (car devices))
            (system-device (cdr devices))
@@ -282,6 +331,9 @@ Uses wf-recorder on Wayland, x11grab on X11."
                                          record-command))
       (set-process-query-on-exit-flag cj/video-recording-ffmpeg-process nil)
       (set-process-sentinel cj/video-recording-ffmpeg-process #'cj/recording-process-sentinel)
+      ;; Stamp the start time so the sentinel can tell a ~0.5s failed start
+      ;; (wf-recorder couldn't grab the screen) from a normal recording.
+      (process-put cj/video-recording-ffmpeg-process 'cj-start-time (float-time))
       (force-mode-line-update t)
       (message "Started video recording to %s (%s, mic: %.1fx, system: %.1fx)."
                filename
@@ -339,6 +391,9 @@ for ffmpeg to write container metadata before giving up."
   (if (not cj/video-recording-ffmpeg-process)
       (message "No video recording in progress.")
     (let ((proc cj/video-recording-ffmpeg-process))
+      ;; Mark this as a user stop so the sentinel's fail-fast check doesn't
+      ;; misread a quick intentional stop as a failed start.
+      (process-put proc 'cj-stopping t)
       ;; On Wayland, kill the producer (wf-recorder) FIRST so ffmpeg sees
       ;; a clean EOF on pipe:0. This triggers ffmpeg's orderly shutdown:
       ;; drain remaining frames, write container metadata, close file.
diff --git a/tests/test-video-audio-recording--build-video-command.el b/tests/test-video-audio-recording--build-video-command.el
index 4f290978..1ffce95b 100644
--- a/tests/test-video-audio-recording--build-video-command.el
+++ b/tests/test-video-audio-recording--build-video-command.el
@@ -27,6 +27,14 @@
         (should (string-match-p "-i pipe:0" cmd))
         (should (string-match-p "-c:v copy" cmd))))))
 
+(ert-deftest test-video-audio-recording--build-video-command-normal-wayland-keeps-wf-recorder-stderr ()
+  "Wayland command does not discard wf-recorder stderr, so a failed grab is diagnosable."
+  (let ((cj/recording-mic-boost 2.0)
+        (cj/recording-system-volume 1.0))
+    (cl-letf (((symbol-function 'executable-find) (lambda (_prog &rest _) t)))
+      (let ((cmd (cj/recording--build-video-command "mic" "sys" "/tmp/out.mkv" t)))
+        (should-not (string-match-p "2>/dev/null" cmd))))))
+
 (ert-deftest test-video-audio-recording--build-video-command-normal-x11-uses-x11grab ()
   "X11 command uses ffmpeg with x11grab, no wf-recorder."
   (let ((cj/recording-mic-boost 2.0)
diff --git a/tests/test-video-audio-recording--start-race.el b/tests/test-video-audio-recording--start-race.el
new file mode 100644
index 00000000..36ea8595
--- /dev/null
+++ b/tests/test-video-audio-recording--start-race.el
@@ -0,0 +1,56 @@
+;;; test-video-audio-recording--start-race.el --- start-race fix tests -*- lexical-binding: t; -*-
+
+;;; Commentary:
+;; Tests for the wf-recorder start-race fix: the poll that waits for a dying
+;; wf-recorder to release the compositor capture before launching a new one, and
+;; the fail-fast timing predicate that tells a 0.5s failed start from a real
+;; recording.  The pgrep wrapper and the live start/stop wiring are exercised in
+;; the daemon, not here; the poll is tested with an injected predicate so no real
+;; process is needed.
+
+;;; Code:
+
+(require 'ert)
+
+;; Stub dependencies before loading the module.
+(defvar cj/custom-keymap (make-sparse-keymap)
+  "Stub keymap for testing.")
+
+(require 'video-audio-recording)
+
+(declare-function cj/recording--start-failed-p "video-audio-recording-capture" (elapsed threshold))
+(declare-function cj/recording--wait-for-no-wf-recorder "video-audio-recording-capture" (timeout-secs &optional running-p))
+
+;;; ------------------------- cj/recording--start-failed-p ---------------------
+
+(ert-deftest test-recording-start-failed-p-short-exit-is-failure ()
+  "Normal: an exit well before the threshold is a failed start."
+  (should (cj/recording--start-failed-p 0.5 1.5)))
+
+(ert-deftest test-recording-start-failed-p-long-run-is-not-failure ()
+  "Normal: a long-running recording that ends is not a failed start."
+  (should-not (cj/recording--start-failed-p 30.0 1.5)))
+
+(ert-deftest test-recording-start-failed-p-at-threshold-is-not-failure ()
+  "Boundary: an exit exactly at the threshold is not counted as failed."
+  (should-not (cj/recording--start-failed-p 1.5 1.5)))
+
+;;; -------------------- cj/recording--wait-for-no-wf-recorder ------------------
+
+(ert-deftest test-recording-wait-for-no-wf-recorder-clears ()
+  "Normal: returns t once the injected predicate reports wf-recorder gone."
+  (let ((n 0))
+    (should (cj/recording--wait-for-no-wf-recorder
+             2.0
+             (lambda () (setq n (1+ n)) (< n 3))))))
+
+(ert-deftest test-recording-wait-for-no-wf-recorder-already-clear ()
+  "Boundary: an already-clear predicate returns t immediately."
+  (should (cj/recording--wait-for-no-wf-recorder 2.0 (lambda () nil))))
+
+(ert-deftest test-recording-wait-for-no-wf-recorder-times-out ()
+  "Error: a predicate that never clears returns nil at the timeout."
+  (should-not (cj/recording--wait-for-no-wf-recorder 0.15 (lambda () t))))
+
+(provide 'test-video-audio-recording--start-race)
+;;; test-video-audio-recording--start-race.el ends here