diff options
17 files changed, 1191 insertions, 552 deletions
diff --git a/modules/video-audio-recording.el b/modules/video-audio-recording.el index a0e9970b..4e57951d 100644 --- a/modules/video-audio-recording.el +++ b/modules/video-audio-recording.el @@ -2,9 +2,27 @@ ;; author: Craig Jennings <c@cjennings.net> ;; ;;; Commentary: -;; Use ffmpeg to record desktop video or just audio. -;; Records audio from both microphone and system audio (for calls/meetings). -;; Audio recordings use M4A/AAC format for best compatibility. +;; +;; Desktop video and audio recording from within Emacs using ffmpeg. +;; Records from both microphone and system audio simultaneously, which +;; makes it suitable for capturing meetings, presentations, and desktop activity. +;; +;; Architecture: +;; - Audio recordings use ffmpeg directly with PulseAudio inputs → M4A/AAC +;; - Video recordings differ by display server: +;; - X11: ffmpeg with x11grab + PulseAudio → MKV +;; - Wayland: wf-recorder piped to ffmpeg for audio mixing → MKV +;; (wf-recorder captures the compositor, ffmpeg mixes in audio) +;; +;; Process lifecycle: +;; - Start: `start-process-shell-command` creates a shell running the +;; ffmpeg (or wf-recorder|ffmpeg) pipeline. Process ref is stored in +;; `cj/video-recording-ffmpeg-process' or `cj/audio-recording-ffmpeg-process'. +;; - Stop: SIGINT is sent to the shell's process group so all pipeline +;; children (wf-recorder, ffmpeg) receive it. We then poll until the +;; process actually exits, giving ffmpeg time to finalize the container. +;; - Cleanup: A process sentinel auto-clears the process variable and +;; updates the modeline if the process dies unexpectedly. ;; ;; Note: video-recordings-dir and audio-recordings-dir are defined ;; (and directory created) in user-constants.el @@ -14,7 +32,7 @@ ;; 1. Press C-; r a (start/stop audio recording) ;; 2. First time: you'll be prompted for device setup ;; 3. Choose "Bluetooth Headset" (or your device) -;; 4. Recording starts - you'll see 🔴Audio in your modeline +;; 4. Recording starts - you'll see in your modeline ;; 5. Press C-; r a again to stop (🔴 disappears) ;; ;; Device Setup (First Time Only) @@ -24,11 +42,11 @@ ;; ;; Manual device selection: ;; -;; C-; r c (cj/recording-quick-setup-for-calls) - RECOMMENDED -;; Quick setup: picks one device for both mic and monitor. -;; Perfect for calls, meetings, or when using headset. +;; C-; r s (cj/recording-quick-setup) - RECOMMENDED +;; Quick setup: pick a mic, system audio is auto-detected. +;; Works for any recording scenario. ;; -;; C-; r s (cj/recording-select-devices) - ADVANCED +;; C-; r S (cj/recording-select-devices) - ADVANCED ;; Manual selection: choose mic and monitor separately. ;; Use when you need different devices for input/output. ;; @@ -42,7 +60,7 @@ ;; ;; Testing Devices Before Important Recordings ;; ============================================ -;; Always test devices before important meetings/calls: +;; Always test devices before important recordings: ;; ;; C-; r t b (cj/recording-test-both) - RECOMMENDED ;; Guided test: mic only, monitor only, then both together. @@ -66,7 +84,11 @@ (require 'system-lib) -;; Forward declarations +;;; ============================================================ +;;; Configuration Variables +;;; ============================================================ + +;; Forward declarations for variables defined in user-constants.el (eval-when-compile (defvar video-recordings-dir)) (eval-when-compile (defvar audio-recordings-dir)) (defvar cj/custom-keymap) @@ -78,7 +100,7 @@ (defvar cj/recording-system-volume 2.0 "Volume multiplier for system audio in recordings. 1.0 = normal volume, 2.0 = double volume (+6dB), 0.5 = half volume (-6dB). -Default is 2.0 because the pan filter reduces by 50%, so final level is 1.0x.") +Default is 2.0 because the amerge filter reduces level, so 2.0x compensates.") (defvar cj/recording-mic-device nil "PulseAudio device name for microphone input. @@ -88,32 +110,49 @@ If nil, will auto-detect on first use.") "PulseAudio device name for system audio monitor. If nil, will auto-detect on first use.") +;;; ============================================================ +;;; Internal State +;;; ============================================================ + +;; These hold the Emacs process objects for running recordings. +;; The process is the shell that runs the ffmpeg (or wf-recorder|ffmpeg) +;; pipeline. When non-nil, a recording is in progress. + (defvar cj/video-recording-ffmpeg-process nil - "Variable to store the process of the ffmpeg video recording.") + "Emacs process object for the active video recording shell, or nil.") (defvar cj/audio-recording-ffmpeg-process nil - "Variable to store the process of the ffmpeg audio recording.") + "Emacs process object for the active audio recording shell, or nil.") + +;;; ============================================================ +;;; Modeline Indicator +;;; ============================================================ -;; Modeline recording indicator (defun cj/recording-modeline-indicator () "Return modeline string showing active recordings. -Shows 🔴 when recording (audio and/or video). +Shows (microphone) for audio, (camcorder) for video. Checks if process is actually alive, not just if variable is set." (let ((audio-active (and cj/audio-recording-ffmpeg-process (process-live-p cj/audio-recording-ffmpeg-process))) (video-active (and cj/video-recording-ffmpeg-process (process-live-p cj/video-recording-ffmpeg-process)))) (cond - ((and audio-active video-active) " 🔴A+V ") - (audio-active " 🔴Audio ") - (video-active " 🔴Video ") + ((and audio-active video-active) " ") + (audio-active " ") + (video-active " ") (t "")))) +;;; ============================================================ +;;; Process Lifecycle (Sentinel and Graceful Shutdown) +;;; ============================================================ + (defun cj/recording-process-sentinel (process event) - "Sentinel for recording processes to clean up and update modeline. -PROCESS is the ffmpeg process, EVENT describes what happened." + "Sentinel for recording processes — handles unexpected exits. +PROCESS is the ffmpeg shell process, EVENT describes what happened. +This is called by Emacs when the process changes state (exits, is killed, etc.). +It clears the process variable and updates the modeline so the recording indicator +disappears even if the recording crashes or is killed externally." (when (memq (process-status process) '(exit signal)) - ;; Process ended - clear the variable (cond ((eq process cj/audio-recording-ffmpeg-process) (setq cj/audio-recording-ffmpeg-process nil) @@ -121,26 +160,63 @@ PROCESS is the ffmpeg process, EVENT describes what happened." ((eq process cj/video-recording-ffmpeg-process) (setq cj/video-recording-ffmpeg-process nil) (message "Video recording stopped: %s" (string-trim event)))) - ;; Force modeline update (force-mode-line-update t))) +(defun cj/recording--wait-for-exit (process timeout-secs) + "Wait for PROCESS to exit, polling until done or TIMEOUT-SECS elapsed. +Returns t if the process exited within the timeout, nil if it timed out. + +This replaces fixed `sit-for' delays with an actual check that ffmpeg has +finished writing its output file. Container finalization (writing index +tables, flushing buffers) can take several seconds for large recordings, +so a fixed 0.5s wait was causing zero-byte output files." + (let ((deadline (+ (float-time) timeout-secs))) + (while (and (process-live-p process) + (< (float-time) deadline)) + (accept-process-output process 0.1)) + (not (process-live-p process)))) + +;;; ============================================================ +;;; Dependency Checks +;;; ============================================================ + (defun cj/recording-check-ffmpeg () - "Check if ffmpeg is available. -Return t if found, nil otherwise." + "Check if ffmpeg is available. Error if not found." (unless (executable-find "ffmpeg") (user-error "Ffmpeg not found. Install with: sudo pacman -S ffmpeg") nil) t) -;; Auto-detection functions removed - they were unreliable and preferred built-in -;; audio over Bluetooth/USB devices. Use explicit device selection instead: -;; - C-; r c (cj/recording-quick-setup-for-calls) - recommended for most use cases -;; - C-; r s (cj/recording-select-devices) - manual selection of mic + monitor +(defun cj/recording--wayland-p () + "Return non-nil if running under Wayland." + (string= (getenv "XDG_SESSION_TYPE") "wayland")) + +(defun cj/recording--check-wf-recorder () + "Check if wf-recorder is available (needed for Wayland video capture)." + (if (executable-find "wf-recorder") + t + (user-error "wf-recorder not found. Install with: sudo pacman -S wf-recorder") + nil)) + +;;; ============================================================ +;;; PulseAudio Device Discovery +;;; ============================================================ +;; +;; Audio devices are discovered via `pactl list sources short'. +;; Two types of sources matter: +;; - Input sources (microphones): capture your voice +;; - Monitor sources (*.monitor): capture system audio output +;; These tap into what's playing through speakers/headphones, +;; which is how we capture system audio (music, calls, etc.). +;; +;; Device selection is required before first recording. The quick +;; setup (C-; r s) groups hardware devices and lets you pick one +;; device to use for both mic and monitor — ideal for headsets. (defun cj/recording--parse-pactl-output (output) - "Internal parser for pactl sources output. Takes OUTPUT string. + "Parse pactl sources OUTPUT into structured list. Returns list of (device-name driver state) tuples. -Extracted for testing without shell command execution." +Extracted as a separate function for testability." (let ((sources nil)) (dolist (line (split-string output "\n" t)) (when (string-match "^[0-9]+\t\\([^\t]+\\)\t\\([^\t]+\\)\t\\([^\t]+\\)\t\\([^\t]+\\)" line) @@ -157,13 +233,79 @@ Returns list of (device-name driver state) tuples." (shell-command-to-string "pactl list sources short 2>/dev/null"))) (defun cj/recording-friendly-state (state) - "Convert technical state name to user-friendly label. + "Convert technical STATE name to user-friendly label. STATE is the raw state from pactl (SUSPENDED, RUNNING, IDLE, etc.)." (pcase state ("SUSPENDED" "Ready") ("RUNNING" "Active") ("IDLE" "Ready") - (_ state))) ; fallback to original if unknown + (_ state))) + +(defun cj/recording--get-default-sink-monitor () + "Return the PulseAudio monitor source for the default audio output. +The monitor source captures whatever is playing through the default sink +(music, calls, system sounds, etc.). This is the correct device +for capturing \"what I hear\" regardless of which output hardware is active." + (let ((default-sink (string-trim + (shell-command-to-string + "pactl get-default-sink 2>/dev/null")))) + (if (string-empty-p default-sink) + (user-error "No default audio output found. Is PulseAudio/PipeWire running?") + (concat default-sink ".monitor")))) + +(defun cj/recording--parse-pactl-sources-verbose (output) + "Parse verbose `pactl list sources' OUTPUT into structured list. +Returns list of (name description mute state) tuples. +OUTPUT should be the full output of `pactl list sources'." + (let ((sources nil) + (current-name nil) + (current-desc nil) + (current-mute nil) + (current-state nil)) + (dolist (line (split-string output "\n")) + (cond + ((string-match "^Source #" line) + ;; Save previous source if complete + (when current-name + (push (list current-name current-desc current-mute current-state) + sources)) + (setq current-name nil current-desc nil + current-mute nil current-state nil)) + ((string-match "^\\s-+Name:\\s-+\\(.+\\)" line) + (setq current-name (match-string 1 line))) + ((string-match "^\\s-+Description:\\s-+\\(.+\\)" line) + (setq current-desc (match-string 1 line))) + ((string-match "^\\s-+Mute:\\s-+\\(.+\\)" line) + (setq current-mute (match-string 1 line))) + ((string-match "^\\s-+State:\\s-+\\(.+\\)" line) + (setq current-state (match-string 1 line))))) + ;; Don't forget the last source + (when current-name + (push (list current-name current-desc current-mute current-state) + sources)) + (nreverse sources))) + +(defun cj/recording--get-available-mics () + "Return available microphone sources as (name . description) alist. +Filters out monitor sources and muted devices. Uses the friendly +description from PulseAudio (e.g. \"Jabra SPEAK 510 Mono\") rather +than the raw device name." + (let* ((output (shell-command-to-string "pactl list sources 2>/dev/null")) + (sources (cj/recording--parse-pactl-sources-verbose output)) + (mics nil)) + (dolist (source sources) + (let ((name (nth 0 source)) + (desc (nth 1 source)) + (mute (nth 2 source))) + ;; Include non-monitor, non-muted sources + (when (and (not (string-match-p "\\.monitor$" name)) + (not (equal mute "yes"))) + (push (cons name (or desc name)) mics)))) + (nreverse mics))) + +;;; ============================================================ +;;; Device Selection UI +;;; ============================================================ (defun cj/recording-list-devices () "Show all available audio sources in a readable format. @@ -183,7 +325,7 @@ Opens a buffer showing devices with their states." (dolist (source sources) (let ((device (nth 0 source)) (driver (nth 1 source)) - (state (nth 2 source)) + (_state (nth 2 source)) (friendly-state (cj/recording-friendly-state (nth 2 source)))) (insert (format "%-10s [%s]\n" friendly-state driver)) (insert (format " %s\n\n" device)))) @@ -193,9 +335,9 @@ Opens a buffer showing devices with their states." (switch-to-buffer-other-window "*Recording Devices*"))) (defun cj/recording-show-active-audio () - "Show which audio sinks are currently PLAYING audio in real-time. -Useful for diagnosing why phone call audio isn't being captured - helps identify -which device the phone app is actually using for output." + "Show which audio sinks are currently PLAYING audio. +Useful for diagnosing why phone call audio isn't being captured — helps +identify which device the phone app is actually using for output." (interactive) (let ((output (shell-command-to-string "pactl list sink-inputs"))) (with-current-buffer (get-buffer-create "*Active Audio Playback*") @@ -225,7 +367,8 @@ which device the phone app is actually using for output." (defun cj/recording-select-device (prompt device-type) "Interactively select an audio device. -PROMPT is shown to user. DEVICE-TYPE is 'mic or 'monitor for filtering. +PROMPT is shown to user. DEVICE-TYPE is \\='mic or \\='monitor for filtering. +Monitor devices end in .monitor (they tap system audio output). Returns selected device name or nil." (let* ((sources (cj/recording-parse-sources)) (filtered (if (eq device-type 'monitor) @@ -233,8 +376,8 @@ Returns selected device name or nil." (seq-filter (lambda (s) (not (string-match-p "\\.monitor$" (car s)))) sources))) (choices (mapcar (lambda (s) (let ((device (nth 0 s)) - (driver (nth 1 s)) - (state (nth 2 s)) + (_driver (nth 1 s)) + (_state (nth 2 s)) (friendly-state (cj/recording-friendly-state (nth 2 s)))) (cons (format "%-10s %s" friendly-state device) device))) filtered))) @@ -243,8 +386,8 @@ Returns selected device name or nil." (user-error "No %s devices found" (if (eq device-type 'monitor) "monitor" "input"))))) (defun cj/recording-select-devices () - "Interactively select microphone and system audio devices. -Sets cj/recording-mic-device and cj/recording-system-device." + "Interactively select microphone and system audio devices separately. +Sets `cj/recording-mic-device' and `cj/recording-system-device'." (interactive) (setq cj/recording-mic-device (cj/recording-select-device "Select microphone device: " 'mic)) @@ -255,25 +398,27 @@ Sets cj/recording-mic-device and cj/recording-system-device." cj/recording-system-device)) (defun cj/recording-group-devices-by-hardware () - "Group audio sources by hardware device. -Returns alist of (device-name . (mic-source . monitor-source))." + "Group audio sources by physical hardware device. +Returns alist of (friendly-name . (mic-source . monitor-source)). +Only includes devices that have BOTH a mic and a monitor source, +since recording needs both to capture your voice and system audio." (let ((sources (cj/recording-parse-sources)) (devices (make-hash-table :test 'equal)) (result nil)) - ;; Group sources by base device name + ;; Group sources by base device name (hardware identifier) (dolist (source sources) (let* ((device (nth 0 source)) - (driver (nth 1 source)) - ;; Extract hardware ID (the unique part that identifies the physical device) + ;; Extract hardware ID — the unique part identifying the physical device. + ;; Different device types use different naming conventions in PulseAudio. (base-name (cond ;; USB devices: extract usb-XXXXX-XX part ((string-match "\\.\\(usb-[^.]+\\-[0-9]+\\)\\." device) (match-string 1 device)) - ;; Built-in (pci) devices: extract pci-XXXXX part + ;; Built-in (PCI) devices: extract pci-XXXXX part ((string-match "\\.\\(pci-[^.]+\\)\\." device) (match-string 1 device)) ;; Bluetooth devices: extract and normalize MAC address - ;; (input uses colons, output uses underscores - normalize to colons) + ;; (input uses colons, output uses underscores) ((string-match "bluez_\\(?:input\\|output\\)\\.\\([^.]+\\)" device) (replace-regexp-in-string "_" ":" (match-string 1 device))) (t device))) @@ -282,14 +427,13 @@ Returns alist of (device-name . (mic-source . monitor-source))." (unless device-entry (setf device-entry (cons nil nil)) (puthash base-name device-entry devices)) - ;; Store mic or monitor in the pair (if is-monitor (setcdr device-entry device) (setcar device-entry device)))) - ;; Convert hash table to alist with friendly names + ;; Convert hash table to alist with user-friendly names (maphash (lambda (base-name pair) - (when (and (car pair) (cdr pair)) ; Only include if we have both mic and monitor + (when (and (car pair) (cdr pair)) (let ((friendly-name (cond ((string-match-p "usb.*[Jj]abra" base-name) "Jabra SPEAK 510 USB") @@ -301,35 +445,52 @@ Returns alist of (device-name . (mic-source . monitor-source))." devices) (nreverse result))) -(defun cj/recording-quick-setup-for-calls () - "Quick setup for recording call/meetings. -Detects available audio devices and lets you pick one device to use for -both microphone (your voice) and monitor (remote person + sound effects). -Perfect for recording video calls, phone calls, or presentations." +(defun cj/recording-quick-setup () + "Quick device setup for recording. +Shows available microphones and lets you pick one. System audio is +automatically captured from the default audio output's monitor source, +so it records whatever you hear (music, calls, system sounds) +regardless of which output device is active. + +This approach is portable across systems — plug in a new mic, run this +command, and it appears in the list. No hardware-specific configuration +needed." (interactive) - (let* ((grouped-devices (cj/recording-group-devices-by-hardware)) - (choices (mapcar #'car grouped-devices))) + (let* ((mics (cj/recording--get-available-mics)) + (monitor (cj/recording--get-default-sink-monitor)) + (choices (mapcar (lambda (mic) + (cons (cdr mic) (car mic))) + mics))) (if (null choices) - (user-error "No complete audio devices found (need both mic and monitor)") - (let* ((choice (completing-read "Which device are you using for the call? " choices nil t)) - (device-pair (cdr (assoc choice grouped-devices))) - (mic (car device-pair)) - (monitor (cdr device-pair))) - (setq cj/recording-mic-device mic) - (setq cj/recording-system-device monitor) - (message "Call recording ready! Using: %s\n Mic: %s\n Monitor: %s" - choice - (file-name-nondirectory mic) - (file-name-nondirectory monitor)))))) + (user-error "No microphones found. Is a mic plugged in and unmuted?") + (let* ((choices-with-cancel (append choices '(("Cancel" . nil)))) + (choice (completing-read "Select microphone: " + (lambda (string pred action) + (if (eq action 'metadata) + '(metadata (display-sort-function . identity)) + (complete-with-action action choices-with-cancel string pred))) + nil t)) + (mic-device (cdr (assoc choice choices-with-cancel)))) + (if (null mic-device) + (user-error "Device setup cancelled") + (setq cj/recording-mic-device mic-device) + (setq cj/recording-system-device monitor) + (message "Recording ready!\n Mic: %s\n System audio: %s (default output monitor)" + choice + (file-name-nondirectory monitor))))))) + +;;; ============================================================ +;;; Device Testing +;;; ============================================================ +;; +;; These functions record short clips and play them back so you can +;; verify hardware works BEFORE an important recording. (defun cj/recording-test-mic () - "Test microphone by recording 5 seconds and playing it back. -Records from configured mic device, saves to temp file, plays back. -Useful for verifying mic hardware works before important recordings." + "Test microphone by recording 5 seconds and playing it back." (interactive) (unless cj/recording-mic-device - (user-error "No microphone configured. Run C-; r c first")) - + (user-error "No microphone configured. Run C-; r s first")) (let* ((temp-file (make-temp-file "mic-test-" nil ".wav")) (duration 5)) (message "Recording from mic for %d seconds... SPEAK NOW!" duration) @@ -345,13 +506,10 @@ Useful for verifying mic hardware works before important recordings." (defun cj/recording-test-monitor () "Test system audio monitor by recording 5 seconds and playing it back. -Records from configured monitor device (system audio output). -Play some audio/video during test. Useful for verifying you can capture -conference call audio, YouTube, etc." +Play some audio/video during the test so there's something to capture." (interactive) (unless cj/recording-system-device - (user-error "No system monitor configured. Run C-; r c first")) - + (user-error "No system monitor configured. Run C-; r s first")) (let* ((temp-file (make-temp-file "monitor-test-" nil ".wav")) (duration 5)) (message "Recording system audio for %d seconds... PLAY SOMETHING NOW!" duration) @@ -366,24 +524,23 @@ conference call audio, YouTube, etc." (message "Monitor test complete. Temp file: %s" temp-file))) (defun cj/recording-test-both () - "Test both mic and monitor together with guided prompts. -This simulates a real recording scenario: -1. Tests mic only (speak into it) -2. Tests monitor only (play audio/video) -3. Tests both together (speak while audio plays) - -Run this before important recordings to verify everything works!" + "Guided test of both mic and monitor together. +Runs three sequential tests: + 1. Mic only — speak into it + 2. Monitor only — play audio/video + 3. Both together — speak while audio plays +Run this before important recordings to verify everything works." (interactive) (unless (and cj/recording-mic-device cj/recording-system-device) - (user-error "Devices not configured. Run C-; r c first")) + (user-error "Devices not configured. Run C-; r s first")) (when (y-or-n-p "Test 1: Record from MICROPHONE only (5 sec). Ready? ") (cj/recording-test-mic) - (sit-for 6)) ; Wait for playback + (sit-for 6)) (when (y-or-n-p "Test 2: Record from SYSTEM AUDIO only (5 sec). Start playing audio/video, then press y: ") (cj/recording-test-monitor) - (sit-for 6)) ; Wait for playback + (sit-for 6)) (when (y-or-n-p "Test 3: Record BOTH mic + system audio (5 sec). Speak while audio plays, then press y: ") (let* ((temp-file (make-temp-file "both-test-" nil ".wav")) @@ -405,31 +562,32 @@ Run this before important recordings to verify everything works!" (message "Device testing complete. If you heard audio in all tests, recording will work!")) +;;; ============================================================ +;;; Device Validation +;;; ============================================================ + (defun cj/recording-get-devices () "Get audio devices, prompting user if not already configured. -Returns (mic-device . system-device) or nil on error." - ;; If devices not set, prompt user to select them +Returns (mic-device . system-device) cons cell. +If devices aren't set, goes straight into quick setup (mic selection)." (unless (and cj/recording-mic-device cj/recording-system-device) - (if (y-or-n-p "Audio devices not configured. Use quick setup for calls? ") - (cj/recording-quick-setup-for-calls) - (cj/recording-select-devices))) - - ;; Final validation + (cj/recording-quick-setup)) (unless (and cj/recording-mic-device cj/recording-system-device) - (user-error "Audio devices not configured. Run C-; r c (quick setup) or C-; r s (manual select)")) - + (user-error "Audio devices not configured. Run C-; r s (quick setup) or C-; r S (manual select)")) (cons cj/recording-mic-device cj/recording-system-device)) +;;; ============================================================ +;;; Toggle Commands (User-Facing) +;;; ============================================================ + (defun cj/video-recording-toggle (arg) "Toggle video recording: start if not recording, stop if recording. -On first use (or when devices not configured), runs quick setup (C-; r c). +On first use (or when devices not configured), runs quick setup (C-; r s). With prefix ARG, prompt for recording location. Otherwise use the default location in `video-recordings-dir'." (interactive "P") (if cj/video-recording-ffmpeg-process - ;; Recording in progress - stop it (cj/video-recording-stop) - ;; Not recording - start it (let* ((location (if arg (read-directory-name "Enter recording location: ") video-recordings-dir)) @@ -440,14 +598,12 @@ Otherwise use the default location in `video-recordings-dir'." (defun cj/audio-recording-toggle (arg) "Toggle audio recording: start if not recording, stop if recording. -On first use (or when devices not configured), runs quick setup (C-; r c). +On first use (or when devices not configured), runs quick setup (C-; r s). With prefix ARG, prompt for recording location. Otherwise use the default location in `audio-recordings-dir'." (interactive "P") (if cj/audio-recording-ffmpeg-process - ;; Recording in progress - stop it (cj/audio-recording-stop) - ;; Not recording - start it (let* ((location (if arg (read-directory-name "Enter recording location: ") audio-recordings-dir)) @@ -456,24 +612,24 @@ Otherwise use the default location in `audio-recordings-dir'." (make-directory directory t)) (cj/ffmpeg-record-audio location)))) -(defun cj/recording--wayland-p () - "Return non-nil if running under Wayland." - (string= (getenv "XDG_SESSION_TYPE") "wayland")) - -(defun cj/recording--check-wf-recorder () - "Check if wf-recorder is available (needed for Wayland). -Return t if found, nil otherwise." - (if (executable-find "wf-recorder") - t - (user-error "wf-recorder not found. Install with: sudo pacman -S wf-recorder") - nil)) +;;; ============================================================ +;;; Start Recording +;;; ============================================================ (defun cj/ffmpeg-record-video (directory) - "Start a video recording. Save output to DIRECTORY. -Uses wf-recorder on Wayland, x11grab on X11." + "Start a video recording, saving output to DIRECTORY. +Uses wf-recorder on Wayland, x11grab on X11. + +On Wayland, the pipeline is: + wf-recorder (captures screen → H.264) | ffmpeg (mixes in audio → MKV) + +On X11, ffmpeg handles everything: + ffmpeg (x11grab for screen + PulseAudio for audio → MKV)" (cj/recording-check-ffmpeg) (unless cj/video-recording-ffmpeg-process - ;; On Wayland, kill any orphan wf-recorder processes from previous crashes + ;; On Wayland, kill any orphan wf-recorder processes left over from + ;; previous crashes. Without this, old wf-recorders hold the compositor + ;; capture and new ones fail silently. (when (cj/recording--wayland-p) (call-process "pkill" nil nil nil "-INT" "wf-recorder") (sit-for 0.1)) @@ -486,10 +642,12 @@ Uses wf-recorder on Wayland, x11grab on X11." (on-wayland (cj/recording--wayland-p)) (record-command (if on-wayland - ;; Wayland: wf-recorder pipes H264 video to ffmpeg for audio mixing - ;; wf-recorder outputs matroska container with H264, ffmpeg adds audio (progn (cj/recording--check-wf-recorder) + ;; Wayland pipeline: wf-recorder captures screen as H.264 in + ;; matroska container, piped to ffmpeg which adds mic + system + ;; audio via PulseAudio, then writes the final MKV. + ;; -c:v copy means ffmpeg passes video through without re-encoding. (format (concat "wf-recorder -y -c libx264 -m matroska -f /dev/stdout 2>/dev/null | " "ffmpeg -i pipe:0 " "-f pulse -i %s " @@ -503,7 +661,7 @@ Uses wf-recorder on Wayland, x11grab on X11." cj/recording-mic-boost cj/recording-system-volume (shell-quote-argument filename))) - ;; X11: use x11grab directly + ;; X11: ffmpeg captures screen directly via x11grab (format (concat "ffmpeg -framerate 30 -f x11grab -i :0.0+ " "-f pulse -i %s " "-ac 1 " @@ -517,7 +675,6 @@ Uses wf-recorder on Wayland, x11grab on X11." cj/recording-mic-boost cj/recording-system-volume filename)))) - ;; start the recording (setq cj/video-recording-ffmpeg-process (start-process-shell-command "ffmpeg-video-recording" "*ffmpeg-video-recording*" @@ -531,111 +688,175 @@ Uses wf-recorder on Wayland, x11grab on X11." cj/recording-mic-boost cj/recording-system-volume)))) (defun cj/ffmpeg-record-audio (directory) - "Start an ffmpeg audio recording. Save output to DIRECTORY. -Records from microphone and system audio monitor (configured device), mixing them together. -Use C-; r c to configure which device to use - it must match the device your phone call uses." + "Start an audio recording, saving output to DIRECTORY. +Records from microphone and system audio monitor (configured device), +mixing them together into a single M4A/AAC file. + +The filter graph mixes two PulseAudio inputs: + [mic] → volume boost → amerge → AAC encoder → .m4a + [sys] → volume boost ↗" (cj/recording-check-ffmpeg) (unless cj/audio-recording-ffmpeg-process - (let* ((devices (cj/recording-get-devices)) - (mic-device (car devices)) - ;; Use the explicitly configured monitor device - ;; This must match the device your phone call/audio is using - (system-device (cdr devices)) - (location (expand-file-name directory)) - (name (format-time-string "%Y-%m-%d-%H-%M-%S")) - (filename (expand-file-name (concat name ".m4a") location)) - (ffmpeg-command - (format (concat "ffmpeg " - "-f pulse -i %s " ; Input 0: Microphone (specific device) - "-f pulse -i %s " ; Input 1: System audio monitor - "-filter_complex \"" - "[0:a]volume=%.1f[mic];" ; Apply mic boost - "[1:a]volume=%.1f[sys];" ; Apply system volume - "[mic][sys]amix=inputs=2:duration=longest[out]\" " ; Mix both inputs - "-map \"[out]\" " - "-c:a aac " - "-b:a 64k " - "%s") - mic-device - system-device - cj/recording-mic-boost - cj/recording-system-volume - filename))) - ;; Log the command for debugging - (message "Recording from mic: %s + ALL system outputs" mic-device) - (cj/log-silently "Audio recording ffmpeg command: %s" ffmpeg-command) - ;; start the recording - (setq cj/audio-recording-ffmpeg-process - (start-process-shell-command "ffmpeg-audio-recording" - "*ffmpeg-audio-recording*" - ffmpeg-command)) - (set-process-query-on-exit-flag cj/audio-recording-ffmpeg-process nil) - (set-process-sentinel cj/audio-recording-ffmpeg-process #'cj/recording-process-sentinel) - (force-mode-line-update t) - (message "Started recording to %s (mic: %.1fx, all system audio: %.1fx)" - filename cj/recording-mic-boost cj/recording-system-volume)))) + (let* ((devices (cj/recording-get-devices)) + (mic-device (car devices)) + (system-device (cdr devices)) + (location (expand-file-name directory)) + (name (format-time-string "%Y-%m-%d-%H-%M-%S")) + (filename (expand-file-name (concat name ".m4a") location)) + (ffmpeg-command + (format (concat "ffmpeg " + "-f pulse -i %s " ; Input 0: microphone + "-f pulse -i %s " ; Input 1: system audio monitor + "-filter_complex \"" + "[0:a]volume=%.1f[mic];" + "[1:a]volume=%.1f[sys];" + "[mic][sys]amix=inputs=2:duration=longest[out]\" " + "-map \"[out]\" " + "-c:a aac " + "-b:a 64k " + "%s") + mic-device + system-device + cj/recording-mic-boost + cj/recording-system-volume + filename))) + (message "Recording from mic: %s + ALL system outputs" mic-device) + (cj/log-silently "Audio recording ffmpeg command: %s" ffmpeg-command) + (setq cj/audio-recording-ffmpeg-process + (start-process-shell-command "ffmpeg-audio-recording" + "*ffmpeg-audio-recording*" + ffmpeg-command)) + (set-process-query-on-exit-flag cj/audio-recording-ffmpeg-process nil) + (set-process-sentinel cj/audio-recording-ffmpeg-process #'cj/recording-process-sentinel) + (force-mode-line-update t) + (message "Started recording to %s (mic: %.1fx, all system audio: %.1fx)" + filename cj/recording-mic-boost cj/recording-system-volume)))) + +;;; ============================================================ +;;; Stop Recording +;;; ============================================================ +;; +;; Stopping a recording requires careful process management, especially +;; on Wayland where we have a two-process pipeline (wf-recorder | ffmpeg). +;; +;; Wayland shutdown order (CRITICAL — order matters!): +;; 1. Kill wf-recorder first (the producer). This closes the pipe +;; to ffmpeg, giving ffmpeg a clean EOF on its video input. +;; 2. Signal the process group with SIGINT so ffmpeg begins its +;; graceful shutdown (flushing audio, writing container metadata). +;; 3. Wait for the shell/ffmpeg to actually exit. MKV container +;; finalization (index tables, seek entries) can take several +;; seconds. A fixed `sit-for' is insufficient. +;; 4. Kill any remaining wf-recorder as a safety net. +;; +;; Why producer-first matters: In a `wf-recorder | ffmpeg` pipeline, +;; sending SIGINT to all processes simultaneously causes ffmpeg to +;; abort mid-stream (no clean EOF on pipe:0). The result is no output +;; file at all. Killing the producer first lets ffmpeg see EOF, start +;; its orderly shutdown, and then SIGINT reinforces "stop now." +;; +;; X11 shutdown: simpler — ffmpeg is the only process, so we just +;; send SIGINT to the process group and wait. (defun cj/video-recording-stop () - "Stop the ffmpeg video recording process." + "Stop the video recording, waiting for ffmpeg to finalize the file. +On Wayland, kills wf-recorder first so ffmpeg gets a clean EOF on its +video input pipe, then signals the process group. Waits up to 5 seconds +for ffmpeg to write container metadata before giving up." (interactive) - (if cj/video-recording-ffmpeg-process - (progn - ;; On Wayland, we run wf-recorder | ffmpeg pipeline. - ;; SIGINT only reaches ffmpeg, leaving wf-recorder as orphan. - ;; Kill wf-recorder explicitly first, then ffmpeg will exit naturally. - (when (cj/recording--wayland-p) - (call-process "pkill" nil nil nil "-INT" "wf-recorder")) - ;; Use interrupt-process to send SIGINT (graceful termination) - (interrupt-process cj/video-recording-ffmpeg-process) - ;; Give ffmpeg a moment to finalize the file - (sit-for 0.5) - (setq cj/video-recording-ffmpeg-process nil) - (force-mode-line-update t) - (message "Stopped video recording.")) - (message "No video recording in progress."))) + (if (not cj/video-recording-ffmpeg-process) + (message "No video recording in progress.") + (let ((proc cj/video-recording-ffmpeg-process)) + ;; 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. + ;; Without this, simultaneous SIGINT to both causes ffmpeg to abort + ;; without creating a file. + (when (cj/recording--wayland-p) + (call-process "pkill" nil nil nil "-INT" "wf-recorder") + (sit-for 0.3)) ; Brief pause for pipe to close + ;; Now send SIGINT to the process group. On Wayland, this reaches + ;; ffmpeg (which is already shutting down from the pipe EOF) and + ;; reinforces the stop. On X11, this is the primary shutdown signal. + (let ((pid (process-id proc))) + (when pid + (signal-process (- pid) 2))) ; 2 = SIGINT + ;; Wait for ffmpeg to finalize the container. MKV files need index + ;; tables written at the end — without this wait, the file is truncated. + (let ((exited (cj/recording--wait-for-exit proc 5))) + (unless exited + (message "Warning: recording process did not exit within 5 seconds"))) + ;; Safety net: kill any straggler wf-recorder on Wayland. + (when (cj/recording--wayland-p) + (call-process "pkill" nil nil nil "-INT" "wf-recorder")) + ;; The sentinel handles clearing cj/video-recording-ffmpeg-process + ;; and updating the modeline. If the process already exited during + ;; our wait, the sentinel has already fired. If not, force cleanup. + (when (eq cj/video-recording-ffmpeg-process proc) + (setq cj/video-recording-ffmpeg-process nil) + (force-mode-line-update t)) + (message "Stopped video recording.")))) (defun cj/audio-recording-stop () - "Stop the ffmpeg audio recording process." + "Stop the audio recording, waiting for ffmpeg to finalize the file. +Sends SIGINT to the process group and waits up to 3 seconds for ffmpeg +to flush audio frames and write the M4A container trailer." (interactive) - (if cj/audio-recording-ffmpeg-process - (progn - ;; Use interrupt-process to send SIGINT (graceful termination) - (interrupt-process cj/audio-recording-ffmpeg-process) - ;; Give ffmpeg a moment to finalize the file - (sit-for 0.2) - (setq cj/audio-recording-ffmpeg-process nil) - (force-mode-line-update t) - (message "Stopped audio recording.")) - (message "No audio recording in progress."))) + (if (not cj/audio-recording-ffmpeg-process) + (message "No audio recording in progress.") + (let ((proc cj/audio-recording-ffmpeg-process)) + ;; Send SIGINT to the process group (see video-recording-stop for details) + (let ((pid (process-id proc))) + (when pid + (signal-process (- pid) 2))) + ;; M4A finalization is faster than MKV, but still needs time to write + ;; the AAC trailer and flush the output buffer. + (let ((exited (cj/recording--wait-for-exit proc 3))) + (unless exited + (message "Warning: recording process did not exit within 3 seconds"))) + ;; Fallback cleanup if sentinel hasn't fired yet + (when (eq cj/audio-recording-ffmpeg-process proc) + (setq cj/audio-recording-ffmpeg-process nil) + (force-mode-line-update t)) + (message "Stopped audio recording.")))) + +;;; ============================================================ +;;; Volume Adjustment +;;; ============================================================ (defun cj/recording-adjust-volumes () - "Interactively adjust recording volume levels." + "Interactively adjust recording volume levels. +Changes take effect on the next recording (not the current one)." (interactive) (let ((mic (read-number "Microphone boost (1.0 = normal, 2.0 = double): " - cj/recording-mic-boost)) - (sys (read-number "System audio level (1.0 = normal, 0.5 = half): " - cj/recording-system-volume))) - (setq cj/recording-mic-boost mic) - (setq cj/recording-system-volume sys) - (message "Recording levels updated - Mic: %.1fx, System: %.1fx" mic sys))) - -;; Recording operations prefix and keymap + cj/recording-mic-boost)) + (sys (read-number "System audio level (1.0 = normal, 0.5 = half): " + cj/recording-system-volume))) + (setq cj/recording-mic-boost mic) + (setq cj/recording-system-volume sys) + (message "Recording levels updated - Mic: %.1fx, System: %.1fx" mic sys))) + +;;; ============================================================ +;;; Keybindings +;;; ============================================================ + +;; All recording operations are under the C-; r prefix. (defvar cj/record-map (let ((map (make-sparse-keymap))) (define-key map (kbd "v") #'cj/video-recording-toggle) (define-key map (kbd "a") #'cj/audio-recording-toggle) (define-key map (kbd "l") #'cj/recording-adjust-volumes) (define-key map (kbd "d") #'cj/recording-list-devices) - (define-key map (kbd "w") #'cj/recording-show-active-audio) ; "w" for "what's playing" - (define-key map (kbd "s") #'cj/recording-select-devices) - (define-key map (kbd "c") #'cj/recording-quick-setup-for-calls) + (define-key map (kbd "w") #'cj/recording-show-active-audio) + (define-key map (kbd "s") #'cj/recording-quick-setup) + (define-key map (kbd "S") #'cj/recording-select-devices) (define-key map (kbd "t m") #'cj/recording-test-mic) (define-key map (kbd "t s") #'cj/recording-test-monitor) (define-key map (kbd "t b") #'cj/recording-test-both) map) - "Keymap for video/audio recording operations.") + "Keymap for video/audio recording operations under C-; r.") -;; Only set keybinding if cj/custom-keymap is bound (not in batch mode) +;; Only bind keys when running interactively (not in batch/test mode) (when (boundp 'cj/custom-keymap) (keymap-set cj/custom-keymap "r" cj/record-map)) @@ -647,8 +868,8 @@ Use C-; r c to configure which device to use - it must match the device your pho "C-; r l" "adjust levels" "C-; r d" "list devices" "C-; r w" "what's playing (diagnostics)" - "C-; r s" "select devices" - "C-; r c" "quick setup for calls" + "C-; r s" "quick setup" + "C-; r S" "select devices (advanced)" "C-; r t" "test devices" "C-; r t m" "test microphone" "C-; r t s" "test system audio" diff --git a/tests/test-integration-recording-modeline-sync.el b/tests/test-integration-recording-modeline-sync.el index fab442bd..6bb0e99e 100644 --- a/tests/test-integration-recording-modeline-sync.el +++ b/tests/test-integration-recording-modeline-sync.el @@ -68,7 +68,7 @@ When user toggles audio recording on: 1. Process is created -2. Modeline indicator updates to show 🔴Audio +2. Modeline indicator updates to show (mic icon) 3. State is in sync immediately (not delayed) Components integrated: @@ -94,7 +94,7 @@ Validates: (cj/audio-recording-toggle nil) ;; Immediately after toggle: modeline should show recording - (should (equal " 🔴Audio " (cj/recording-modeline-indicator))) + (should (equal " " (cj/recording-modeline-indicator))) ;; Process should be alive (should (process-live-p cj/audio-recording-ffmpeg-process))) @@ -127,7 +127,7 @@ Validates: ;; Start recording (cj/audio-recording-toggle nil) - (should (equal " 🔴Audio " (cj/recording-modeline-indicator))) + (should (equal " " (cj/recording-modeline-indicator))) ;; Stop recording (cj/audio-recording-toggle nil) @@ -150,7 +150,7 @@ Components integrated: Validates: - Video recording follows same sync pattern as audio -- Modeline shows 🔴Video correctly" +- Modeline shows correctly" (test-integration-modeline-setup) (unwind-protect (cl-letf (((symbol-function 'file-directory-p) @@ -164,7 +164,7 @@ Validates: ;; Start video (cj/video-recording-toggle nil) - (should (equal " 🔴Video " (cj/recording-modeline-indicator))) + (should (equal " " (cj/recording-modeline-indicator))) ;; Stop video (cj/video-recording-toggle nil) @@ -201,15 +201,15 @@ Validates: ;; State 2: Audio only (cj/audio-recording-toggle nil) - (should (equal " 🔴Audio " (cj/recording-modeline-indicator))) + (should (equal " " (cj/recording-modeline-indicator))) ;; State 3: Both (cj/video-recording-toggle nil) - (should (equal " 🔴A+V " (cj/recording-modeline-indicator))) + (should (equal " " (cj/recording-modeline-indicator))) ;; State 4: Video only (stop audio) (cj/audio-recording-toggle nil) - (should (equal " 🔴Video " (cj/recording-modeline-indicator))) + (should (equal " " (cj/recording-modeline-indicator))) ;; State 5: None (stop video) (cj/video-recording-toggle nil) @@ -249,7 +249,7 @@ Validates: (cj/audio-recording-toggle nil) ;; Immediately after start: should show recording - (should (equal " 🔴Audio " (cj/recording-modeline-indicator))) + (should (equal " " (cj/recording-modeline-indicator))) ;; Wait for process to exit and sentinel to run (sit-for 0.3) @@ -323,7 +323,7 @@ Validates: (dotimes (_i 5) ;; Start (cj/audio-recording-toggle nil) - (should (equal " 🔴Audio " (cj/recording-modeline-indicator))) + (should (equal " " (cj/recording-modeline-indicator))) (should cj/audio-recording-ffmpeg-process) ;; Stop @@ -357,23 +357,23 @@ Validates: ;; Start audio (cj/audio-recording-toggle nil) - (should (equal " 🔴Audio " (cj/recording-modeline-indicator))) + (should (equal " " (cj/recording-modeline-indicator))) ;; Add video - modeline should combine (cj/video-recording-toggle nil) - (should (equal " 🔴A+V " (cj/recording-modeline-indicator))) + (should (equal " " (cj/recording-modeline-indicator))) ;; Stop audio - video indicator should persist (cj/audio-recording-toggle nil) - (should (equal " 🔴Video " (cj/recording-modeline-indicator))) + (should (equal " " (cj/recording-modeline-indicator))) ;; Start audio again - should recombine (cj/audio-recording-toggle nil) - (should (equal " 🔴A+V " (cj/recording-modeline-indicator))) + (should (equal " " (cj/recording-modeline-indicator))) ;; Stop video - audio indicator should persist (cj/video-recording-toggle nil) - (should (equal " 🔴Audio " (cj/recording-modeline-indicator))) + (should (equal " " (cj/recording-modeline-indicator))) ;; Stop audio - should be empty (cj/audio-recording-toggle nil) diff --git a/tests/test-integration-recording-monitor-capture-interactive.el b/tests/test-integration-recording-monitor-capture-interactive.el index ece8b79e..eb7cbf26 100644 --- a/tests/test-integration-recording-monitor-capture-interactive.el +++ b/tests/test-integration-recording-monitor-capture-interactive.el @@ -8,10 +8,10 @@ ;; **INTERACTIVE TEST - Run from within Emacs** ;; ;; This test must be run from an interactive Emacs session where recording -;; devices are already configured (C-; r c). +;; devices are already configured (C-; r s). ;; ;; USAGE: -;; 1. Ensure devices are configured: C-; r c +;; 1. Ensure devices are configured: C-; r s ;; 2. Load this file: M-x load-file RET tests/test-integration-recording-monitor-capture-interactive.el RET ;; 3. Run test: M-x test-recording-monitor-now RET ;; @@ -79,7 +79,7 @@ This function can be called with M-x to test recording without ERT framework." (unless (file-exists-p test-recording--test-audio) (user-error "Test audio file not found: %s" test-recording--test-audio)) (unless (and cj/recording-mic-device cj/recording-system-device) - (user-error "Recording devices not configured. Run C-; r c first")) + (user-error "Recording devices not configured. Run C-; r s first")) (let ((test-dir (make-temp-file "recording-test-" t)) (recording-file nil) diff --git a/tests/test-integration-recording-monitor-capture.el b/tests/test-integration-recording-monitor-capture.el index 7d7c5dfb..9a0c6380 100644 --- a/tests/test-integration-recording-monitor-capture.el +++ b/tests/test-integration-recording-monitor-capture.el @@ -16,7 +16,7 @@ ;; ;; Requirements: ;; - Audio system must be working (PulseAudio/PipeWire) -;; - Recording devices must be configured (C-; r c) +;; - Recording devices must be configured (C-; r s) ;; - paplay must be available ;; - Transcription backend must be configured ;; diff --git a/tests/test-integration-recording-toggle-workflow.el b/tests/test-integration-recording-toggle-workflow.el index c61698c5..0e000b68 100644 --- a/tests/test-integration-recording-toggle-workflow.el +++ b/tests/test-integration-recording-toggle-workflow.el @@ -10,7 +10,7 @@ ;; - cj/audio-recording-toggle (entry point) ;; - cj/video-recording-toggle (entry point) ;; - cj/recording-get-devices (device prompting and setup) -;; - cj/recording-quick-setup-for-calls (device selection workflow) +;; - cj/recording-quick-setup (device selection workflow) ;; - cj/ffmpeg-record-audio (process creation and ffmpeg command) ;; - cj/ffmpeg-record-video (process creation and ffmpeg command) ;; - cj/recording-modeline-indicator (UI state display) @@ -80,7 +80,7 @@ When user presses C-; r a for the first time: Components integrated: - cj/audio-recording-toggle (toggles start/stop) - cj/recording-get-devices (prompts for setup on first use) -- cj/recording-quick-setup-for-calls (device selection) +- cj/recording-quick-setup (device selection) - cj/ffmpeg-record-audio (creates recording process) - cj/recording-modeline-indicator (UI state) - cj/audio-recording-stop (cleanup) @@ -96,10 +96,8 @@ Validates: (let ((setup-called nil) (ffmpeg-cmd nil) (process-created nil)) - ;; Mock the device setup to simulate user choosing quick setup - (cl-letf (((symbol-function 'y-or-n-p) - (lambda (_prompt) t)) ; User says yes to quick setup - ((symbol-function 'cj/recording-quick-setup-for-calls) + ;; Mock the device setup to simulate quick setup + (cl-letf (((symbol-function 'cj/recording-quick-setup) (lambda () (setq setup-called t) (setq cj/recording-mic-device "test-mic") @@ -130,7 +128,7 @@ Validates: (should (string-match-p "test-monitor" ffmpeg-cmd)) ;; Verify modeline shows recording - (should (equal " 🔴Audio " (cj/recording-modeline-indicator))) + (should (equal " " (cj/recording-modeline-indicator))) ;; STEP 2: Second toggle - should stop recording (cj/audio-recording-toggle nil) @@ -168,7 +166,7 @@ Validates: (let ((setup-called nil) (ffmpeg-cmd nil)) - (cl-letf (((symbol-function 'cj/recording-quick-setup-for-calls) + (cl-letf (((symbol-function 'cj/recording-quick-setup) (lambda () (setq setup-called t))) ((symbol-function 'file-directory-p) (lambda (_dir) t)) @@ -207,9 +205,7 @@ Validates: (test-integration-toggle-setup) (unwind-protect (let ((setup-called nil)) - (cl-letf (((symbol-function 'y-or-n-p) - (lambda (_prompt) t)) - ((symbol-function 'cj/recording-quick-setup-for-calls) + (cl-letf (((symbol-function 'cj/recording-quick-setup) (lambda () (setq setup-called t) (setq cj/recording-mic-device "test-mic") @@ -226,7 +222,7 @@ Validates: ;; Verify setup and recording (should setup-called) (should cj/video-recording-ffmpeg-process) - (should (equal " 🔴Video " (cj/recording-modeline-indicator))) + (should (equal " " (cj/recording-modeline-indicator))) ;; Stop recording (cj/video-recording-toggle nil) @@ -271,7 +267,7 @@ Validates: ;; Verify both are recording (should cj/audio-recording-ffmpeg-process) (should cj/video-recording-ffmpeg-process) - (should (equal " 🔴A+V " (cj/recording-modeline-indicator))) + (should (equal " " (cj/recording-modeline-indicator))) ;; Stop audio only (cj/audio-recording-toggle nil) @@ -279,7 +275,7 @@ Validates: ;; Verify only video still recording (should (null cj/audio-recording-ffmpeg-process)) (should cj/video-recording-ffmpeg-process) - (should (equal " 🔴Video " (cj/recording-modeline-indicator))) + (should (equal " " (cj/recording-modeline-indicator))) ;; Stop video (cj/video-recording-toggle nil) @@ -329,7 +325,7 @@ Validates: ;; Verify recording started (should cj/audio-recording-ffmpeg-process) - (should (equal " 🔴Audio " (cj/recording-modeline-indicator))) + (should (equal " " (cj/recording-modeline-indicator))) ;; Wait for process to exit (sentinel should run) (sit-for 0.3) diff --git a/tests/test-video-audio-recording--get-available-mics.el b/tests/test-video-audio-recording--get-available-mics.el new file mode 100644 index 00000000..2fea0d7a --- /dev/null +++ b/tests/test-video-audio-recording--get-available-mics.el @@ -0,0 +1,102 @@ +;;; test-video-audio-recording--get-available-mics.el --- Tests for mic discovery -*- lexical-binding: t; -*- + +;;; Commentary: +;; Unit tests for cj/recording--get-available-mics. +;; Verifies that available microphones are discovered correctly: +;; - Monitor sources are excluded (they capture output, not input) +;; - Muted sources are excluded +;; - Friendly descriptions from PulseAudio are used + +;;; Code: + +(require 'ert) + +;; Stub dependencies before loading the module +(defvar cj/custom-keymap (make-sparse-keymap) + "Stub keymap for testing.") + +(require 'video-audio-recording) + +;;; Helper + +(defun test-mics--make-pactl-output (sources) + "Build fake `pactl list sources' output from SOURCES. +Each source is (name description mute state)." + (mapconcat (lambda (src) + (format "Source #1\n\tState: %s\n\tName: %s\n\tDescription: %s\n\tMute: %s\n" + (nth 3 src) (nth 0 src) (nth 1 src) (nth 2 src))) + sources "")) + +;;; Normal Cases + +(ert-deftest test-video-audio-recording--get-available-mics-normal-filters-monitors () + "Test that monitor sources are excluded from mic list." + (cl-letf (((symbol-function 'shell-command-to-string) + (lambda (_cmd) + (test-mics--make-pactl-output + '(("alsa_input.usb-Jabra.mono" "Jabra Mono" "no" "SUSPENDED") + ("alsa_output.usb-Jabra.monitor" "Monitor of Jabra" "no" "SUSPENDED")))))) + (let ((mics (cj/recording--get-available-mics))) + (should (= 1 (length mics))) + (should (equal "alsa_input.usb-Jabra.mono" (car (car mics))))))) + +(ert-deftest test-video-audio-recording--get-available-mics-normal-filters-muted () + "Test that muted sources are excluded from mic list." + (cl-letf (((symbol-function 'shell-command-to-string) + (lambda (_cmd) + (test-mics--make-pactl-output + '(("active-mic" "Active Mic" "no" "SUSPENDED") + ("muted-mic" "Muted Mic" "yes" "SUSPENDED")))))) + (let ((mics (cj/recording--get-available-mics))) + (should (= 1 (length mics))) + (should (equal "active-mic" (car (car mics))))))) + +(ert-deftest test-video-audio-recording--get-available-mics-normal-uses-descriptions () + "Test that friendly descriptions are returned as cdr." + (cl-letf (((symbol-function 'shell-command-to-string) + (lambda (_cmd) + (test-mics--make-pactl-output + '(("raw-device-name" "Friendly Device Name" "no" "IDLE")))))) + (let ((mics (cj/recording--get-available-mics))) + (should (equal "Friendly Device Name" (cdr (car mics))))))) + +(ert-deftest test-video-audio-recording--get-available-mics-normal-multiple-mics () + "Test that multiple non-muted, non-monitor mics are returned." + (cl-letf (((symbol-function 'shell-command-to-string) + (lambda (_cmd) + (test-mics--make-pactl-output + '(("mic-a" "Jabra Mono" "no" "SUSPENDED") + ("mic-b" "Built-in" "no" "IDLE") + ("alsa_output.jabra.monitor" "Monitor of Jabra" "no" "SUSPENDED") + ("muted-mic" "Muted Mic" "yes" "SUSPENDED")))))) + (let ((mics (cj/recording--get-available-mics))) + (should (= 2 (length mics)))))) + +;;; Boundary Cases + +(ert-deftest test-video-audio-recording--get-available-mics-boundary-empty-output () + "Test that empty pactl output returns empty list." + (cl-letf (((symbol-function 'shell-command-to-string) + (lambda (_cmd) ""))) + (should (null (cj/recording--get-available-mics))))) + +(ert-deftest test-video-audio-recording--get-available-mics-boundary-all-monitors () + "Test that if all sources are monitors, returns empty list." + (cl-letf (((symbol-function 'shell-command-to-string) + (lambda (_cmd) + (test-mics--make-pactl-output + '(("sink-a.monitor" "Monitor A" "no" "SUSPENDED") + ("sink-b.monitor" "Monitor B" "no" "SUSPENDED")))))) + (should (null (cj/recording--get-available-mics))))) + +(ert-deftest test-video-audio-recording--get-available-mics-boundary-all-muted () + "Test that if all non-monitor sources are muted, returns empty list." + (cl-letf (((symbol-function 'shell-command-to-string) + (lambda (_cmd) + (test-mics--make-pactl-output + '(("muted-a" "Mic A" "yes" "SUSPENDED") + ("muted-b" "Mic B" "yes" "SUSPENDED")))))) + (should (null (cj/recording--get-available-mics))))) + +(provide 'test-video-audio-recording--get-available-mics) +;;; test-video-audio-recording--get-available-mics.el ends here diff --git a/tests/test-video-audio-recording--get-default-sink-monitor.el b/tests/test-video-audio-recording--get-default-sink-monitor.el new file mode 100644 index 00000000..c5cf1abb --- /dev/null +++ b/tests/test-video-audio-recording--get-default-sink-monitor.el @@ -0,0 +1,56 @@ +;;; test-video-audio-recording--get-default-sink-monitor.el --- Tests for default sink monitor -*- lexical-binding: t; -*- + +;;; Commentary: +;; Unit tests for cj/recording--get-default-sink-monitor. +;; This function returns the monitor source name for the default audio +;; output, which captures "what you hear" (call audio, music, etc.). + +;;; Code: + +(require 'ert) + +;; Stub dependencies before loading the module +(defvar cj/custom-keymap (make-sparse-keymap) + "Stub keymap for testing.") + +(require 'video-audio-recording) + +;;; Normal Cases + +(ert-deftest test-video-audio-recording--get-default-sink-monitor-normal-appends-monitor () + "Test that .monitor is appended to the default sink name." + (cl-letf (((symbol-function 'shell-command-to-string) + (lambda (_cmd) "alsa_output.usb-JDS_Labs-00.analog-stereo\n"))) + (should (equal "alsa_output.usb-JDS_Labs-00.analog-stereo.monitor" + (cj/recording--get-default-sink-monitor))))) + +(ert-deftest test-video-audio-recording--get-default-sink-monitor-normal-trims-whitespace () + "Test that trailing whitespace/newlines are stripped from sink name." + (cl-letf (((symbol-function 'shell-command-to-string) + (lambda (_cmd) " alsa_output.pci-0000.analog-stereo \n"))) + (should (equal "alsa_output.pci-0000.analog-stereo.monitor" + (cj/recording--get-default-sink-monitor))))) + +(ert-deftest test-video-audio-recording--get-default-sink-monitor-normal-bluetooth-sink () + "Test with a bluetooth default sink." + (cl-letf (((symbol-function 'shell-command-to-string) + (lambda (_cmd) "bluez_output.AA_BB_CC_DD_EE_FF.a2dp-sink\n"))) + (should (equal "bluez_output.AA_BB_CC_DD_EE_FF.a2dp-sink.monitor" + (cj/recording--get-default-sink-monitor))))) + +;;; Error Cases + +(ert-deftest test-video-audio-recording--get-default-sink-monitor-error-empty-output () + "Test that empty pactl output signals user-error." + (cl-letf (((symbol-function 'shell-command-to-string) + (lambda (_cmd) ""))) + (should-error (cj/recording--get-default-sink-monitor) :type 'user-error))) + +(ert-deftest test-video-audio-recording--get-default-sink-monitor-error-whitespace-only () + "Test that whitespace-only output signals user-error." + (cl-letf (((symbol-function 'shell-command-to-string) + (lambda (_cmd) " \n "))) + (should-error (cj/recording--get-default-sink-monitor) :type 'user-error))) + +(provide 'test-video-audio-recording--get-default-sink-monitor) +;;; test-video-audio-recording--get-default-sink-monitor.el ends here diff --git a/tests/test-video-audio-recording--parse-pactl-sources-verbose.el b/tests/test-video-audio-recording--parse-pactl-sources-verbose.el new file mode 100644 index 00000000..e856f29a --- /dev/null +++ b/tests/test-video-audio-recording--parse-pactl-sources-verbose.el @@ -0,0 +1,79 @@ +;;; test-video-audio-recording--parse-pactl-sources-verbose.el --- Tests for verbose pactl parser -*- lexical-binding: t; -*- + +;;; Commentary: +;; Unit tests for cj/recording--parse-pactl-sources-verbose. +;; Parses the verbose output of `pactl list sources' into structured tuples +;; of (name description mute state). + +;;; Code: + +(require 'ert) + +;; Stub dependencies before loading the module +(defvar cj/custom-keymap (make-sparse-keymap) + "Stub keymap for testing.") + +(require 'video-audio-recording) + +;;; Normal Cases + +(ert-deftest test-video-audio-recording--parse-pactl-sources-verbose-normal-single-source () + "Test parsing a single source entry." + (let* ((output "Source #65\n\tState: SUSPENDED\n\tName: alsa_input.usb-Jabra-00.mono\n\tDescription: Jabra SPEAK 510 Mono\n\tMute: no\n") + (result (cj/recording--parse-pactl-sources-verbose output))) + (should (= 1 (length result))) + (should (equal "alsa_input.usb-Jabra-00.mono" (nth 0 (car result)))) + (should (equal "Jabra SPEAK 510 Mono" (nth 1 (car result)))) + (should (equal "no" (nth 2 (car result)))) + (should (equal "SUSPENDED" (nth 3 (car result)))))) + +(ert-deftest test-video-audio-recording--parse-pactl-sources-verbose-normal-multiple-sources () + "Test parsing multiple source entries." + (let* ((output (concat "Source #65\n\tState: SUSPENDED\n\tName: device-a\n\tDescription: Device A\n\tMute: no\n" + "Source #66\n\tState: RUNNING\n\tName: device-b\n\tDescription: Device B\n\tMute: yes\n")) + (result (cj/recording--parse-pactl-sources-verbose output))) + (should (= 2 (length result))) + (should (equal "device-a" (nth 0 (car result)))) + (should (equal "Device B" (nth 1 (cadr result)))) + (should (equal "yes" (nth 2 (cadr result)))))) + +(ert-deftest test-video-audio-recording--parse-pactl-sources-verbose-normal-monitors-included () + "Test that monitor sources are parsed (filtering is done by caller)." + (let* ((output "Source #67\n\tState: SUSPENDED\n\tName: alsa_output.jds.monitor\n\tDescription: Monitor of JDS Labs\n\tMute: no\n") + (result (cj/recording--parse-pactl-sources-verbose output))) + (should (= 1 (length result))) + (should (string-match-p "\\.monitor$" (nth 0 (car result)))))) + +;;; Boundary Cases + +(ert-deftest test-video-audio-recording--parse-pactl-sources-verbose-boundary-empty-input () + "Test that empty input returns empty list." + (should (null (cj/recording--parse-pactl-sources-verbose "")))) + +(ert-deftest test-video-audio-recording--parse-pactl-sources-verbose-boundary-extra-fields () + "Test that extra fields between sources are ignored." + (let* ((output (concat "Source #65\n\tState: IDLE\n\tName: dev-a\n\tDescription: Dev A\n\tMute: no\n" + "\tDriver: PipeWire\n\tSample Specification: s16le 2ch 48000Hz\n" + "\tChannel Map: front-left,front-right\n" + "Source #66\n\tState: SUSPENDED\n\tName: dev-b\n\tDescription: Dev B\n\tMute: no\n")) + (result (cj/recording--parse-pactl-sources-verbose output))) + (should (= 2 (length result))) + (should (equal "dev-a" (nth 0 (car result)))) + (should (equal "dev-b" (nth 0 (cadr result)))))) + +(ert-deftest test-video-audio-recording--parse-pactl-sources-verbose-boundary-description-with-parens () + "Test descriptions containing parentheses are captured fully." + (let* ((output "Source #91\n\tState: SUSPENDED\n\tName: hdmi.monitor\n\tDescription: Monitor of Radeon (HDMI 2)\n\tMute: no\n") + (result (cj/recording--parse-pactl-sources-verbose output))) + (should (equal "Monitor of Radeon (HDMI 2)" (nth 1 (car result)))))) + +;;; Error Cases + +(ert-deftest test-video-audio-recording--parse-pactl-sources-verbose-error-malformed-no-name () + "Test that source entries without Name field are skipped." + (let* ((output "Source #65\n\tState: SUSPENDED\n\tDescription: Orphan\n\tMute: no\n") + (result (cj/recording--parse-pactl-sources-verbose output))) + (should (null result)))) + +(provide 'test-video-audio-recording--parse-pactl-sources-verbose) +;;; test-video-audio-recording--parse-pactl-sources-verbose.el ends here diff --git a/tests/test-video-audio-recording--wait-for-exit.el b/tests/test-video-audio-recording--wait-for-exit.el new file mode 100644 index 00000000..adacd828 --- /dev/null +++ b/tests/test-video-audio-recording--wait-for-exit.el @@ -0,0 +1,80 @@ +;;; test-video-audio-recording--wait-for-exit.el --- Tests for recording wait-for-exit -*- lexical-binding: t; -*- + +;;; Commentary: +;; Unit tests for `cj/recording--wait-for-exit', the function that polls +;; for process exit instead of using a blind `sit-for' delay. +;; +;; This function is critical to the recording fix: without proper waiting, +;; ffmpeg cannot finalize container metadata, resulting in zero-byte files. +;; +;; Tests use real short-lived processes (`true', `sleep') to exercise the +;; actual polling logic rather than mocking `process-live-p'. + +;;; Code: + +(require 'ert) + +;; Stub dependencies before loading the module +(defvar cj/custom-keymap (make-sparse-keymap) + "Stub keymap for testing.") + +;; Now load the actual production module +(require 'video-audio-recording) + +;;; Normal Cases + +(ert-deftest test-video-audio-recording--wait-for-exit-normal-process-exits-returns-t () + "Test that wait-for-exit returns t when process exits within timeout. +Uses `true' which exits immediately." + (let ((proc (make-process :name "test-exit" :command '("true")))) + (sit-for 0.2) ; Let it exit + (should (eq t (cj/recording--wait-for-exit proc 2))))) + +(ert-deftest test-video-audio-recording--wait-for-exit-normal-polls-until-exit () + "Test that wait-for-exit polls and returns t after process exits. +Uses a short sleep to verify polling works across multiple iterations." + (let ((proc (make-process :name "test-short" :command '("sleep" "0.2")))) + (should (eq t (cj/recording--wait-for-exit proc 3))))) + +;;; Boundary Cases + +(ert-deftest test-video-audio-recording--wait-for-exit-boundary-already-dead-returns-t () + "Test that wait-for-exit returns t immediately for already-dead process." + (let ((proc (make-process :name "test-dead" :command '("true")))) + (sit-for 0.2) ; Ensure it's dead + (should-not (process-live-p proc)) + (should (eq t (cj/recording--wait-for-exit proc 1))))) + +(ert-deftest test-video-audio-recording--wait-for-exit-boundary-zero-timeout-dead-process () + "Test that zero timeout with dead process returns t." + (let ((proc (make-process :name "test-zero-dead" :command '("true")))) + (sit-for 0.2) + (should (eq t (cj/recording--wait-for-exit proc 0))))) + +(ert-deftest test-video-audio-recording--wait-for-exit-boundary-zero-timeout-live-process () + "Test that zero timeout with live process returns nil (timed out)." + (let ((proc (make-process :name "test-zero-live" :command '("sleep" "1000")))) + (unwind-protect + (should (eq nil (cj/recording--wait-for-exit proc 0))) + (delete-process proc)))) + +(ert-deftest test-video-audio-recording--wait-for-exit-boundary-timeout-exceeded-returns-nil () + "Test that wait-for-exit returns nil when timeout is exceeded. +Uses a long-running process with a very short timeout." + (let ((proc (make-process :name "test-timeout" :command '("sleep" "1000")))) + (unwind-protect + (should (eq nil (cj/recording--wait-for-exit proc 0.2))) + (delete-process proc)))) + +;;; Error Cases + +(ert-deftest test-video-audio-recording--wait-for-exit-error-deleted-process-returns-t () + "Test that wait-for-exit handles a deleted (not just exited) process. +A process killed via `delete-process' should be detected as not live." + (let ((proc (make-process :name "test-deleted" :command '("sleep" "1000")))) + (delete-process proc) + (sit-for 0.1) + (should (eq t (cj/recording--wait-for-exit proc 1))))) + +(provide 'test-video-audio-recording--wait-for-exit) +;;; test-video-audio-recording--wait-for-exit.el ends here diff --git a/tests/test-video-audio-recording-ffmpeg-functions.el b/tests/test-video-audio-recording-ffmpeg-functions.el index e82614e2..a3bac0cf 100644 --- a/tests/test-video-audio-recording-ffmpeg-functions.el +++ b/tests/test-video-audio-recording-ffmpeg-functions.el @@ -166,17 +166,20 @@ ;;; Stop Functions - Normal Cases (ert-deftest test-video-audio-recording-video-stop-normal-interrupts-process () - "Test that stopping video recording interrupts the process." + "Test that stopping video recording sends SIGINT to process group." (test-ffmpeg-setup) (unwind-protect (let ((fake-process (make-process :name "test-video" :command '("sleep" "1000"))) - (interrupt-called nil)) + (signal-called nil)) (setq cj/video-recording-ffmpeg-process fake-process) - (cl-letf (((symbol-function 'interrupt-process) - (lambda (_proc) (setq interrupt-called t)))) + (cl-letf (((symbol-function 'cj/recording--wayland-p) (lambda () nil)) + ((symbol-function 'signal-process) + (lambda (_pid _sig) (setq signal-called t) 0)) + ((symbol-function 'cj/recording--wait-for-exit) + (lambda (_proc _timeout) t))) (cj/video-recording-stop) - (should interrupt-called)) - (delete-process fake-process)) + (should signal-called)) + (ignore-errors (delete-process fake-process))) (test-ffmpeg-teardown))) (ert-deftest test-video-audio-recording-video-stop-normal-clears-variable () @@ -205,17 +208,19 @@ (test-ffmpeg-teardown))) (ert-deftest test-video-audio-recording-audio-stop-normal-interrupts-process () - "Test that stopping audio recording interrupts the process." + "Test that stopping audio recording sends SIGINT to process group." (test-ffmpeg-setup) (unwind-protect (let ((fake-process (make-process :name "test-audio" :command '("sleep" "1000"))) - (interrupt-called nil)) + (signal-called nil)) (setq cj/audio-recording-ffmpeg-process fake-process) - (cl-letf (((symbol-function 'interrupt-process) - (lambda (_proc) (setq interrupt-called t)))) + (cl-letf (((symbol-function 'signal-process) + (lambda (_pid _sig) (setq signal-called t) 0)) + ((symbol-function 'cj/recording--wait-for-exit) + (lambda (_proc _timeout) t))) (cj/audio-recording-stop) - (should interrupt-called)) - (delete-process fake-process)) + (should signal-called)) + (ignore-errors (delete-process fake-process))) (test-ffmpeg-teardown))) (ert-deftest test-video-audio-recording-audio-stop-normal-clears-variable () @@ -257,70 +262,68 @@ ;;; Error Cases -(ert-deftest test-video-audio-recording-video-stop-error-interrupt-process-fails () - "Test that video stop handles interrupt-process failure gracefully." +(ert-deftest test-video-audio-recording-video-stop-error-signal-process-fails () + "Test that video stop handles signal-process failure gracefully." (test-ffmpeg-setup) (unwind-protect (let ((fake-process (make-process :name "test-video" :command '("sleep" "1000"))) (error-raised nil)) (setq cj/video-recording-ffmpeg-process fake-process) - (cl-letf (((symbol-function 'interrupt-process) - (lambda (_proc) (error "Interrupt failed")))) - ;; Should handle the error without crashing - (condition-case err + (cl-letf (((symbol-function 'cj/recording--wayland-p) (lambda () nil)) + ((symbol-function 'signal-process) + (lambda (_pid _sig) (error "Signal failed")))) + (condition-case _err (cj/video-recording-stop) (error (setq error-raised t))) - ;; Error should propagate (function doesn't catch it) (should error-raised)) - (delete-process fake-process)) + (ignore-errors (delete-process fake-process))) (test-ffmpeg-teardown))) -(ert-deftest test-video-audio-recording-audio-stop-error-interrupt-process-fails () - "Test that audio stop handles interrupt-process failure gracefully." +(ert-deftest test-video-audio-recording-audio-stop-error-signal-process-fails () + "Test that audio stop handles signal-process failure gracefully." (test-ffmpeg-setup) (unwind-protect (let ((fake-process (make-process :name "test-audio" :command '("sleep" "1000"))) (error-raised nil)) (setq cj/audio-recording-ffmpeg-process fake-process) - (cl-letf (((symbol-function 'interrupt-process) - (lambda (_proc) (error "Interrupt failed")))) - ;; Should handle the error without crashing - (condition-case err + (cl-letf (((symbol-function 'signal-process) + (lambda (_pid _sig) (error "Signal failed")))) + (condition-case _err (cj/audio-recording-stop) (error (setq error-raised t))) - ;; Error should propagate (function doesn't catch it) (should error-raised)) - (delete-process fake-process)) + (ignore-errors (delete-process fake-process))) (test-ffmpeg-teardown))) -(ert-deftest test-video-audio-recording-video-stop-error-dead-process-raises-error () - "Test that video stop raises error if process is already dead. -This documents current behavior - interrupt-process on dead process errors. -The sentinel should clear the variable before this happens in practice." +(ert-deftest test-video-audio-recording-video-stop-error-dead-process-handles-gracefully () + "Test that video stop handles already-dead process gracefully. +The new implementation guards against nil process-id, so stopping a dead +process should clean up without error." (test-ffmpeg-setup) (unwind-protect (let ((fake-process (make-process :name "test-video" :command '("sleep" "1000")))) (setq cj/video-recording-ffmpeg-process fake-process) - ;; Kill process before calling stop (delete-process fake-process) (sit-for 0.1) - ;; Calling stop on dead process raises error - (should-error (cj/video-recording-stop))) + ;; Should not error - process-id returns nil, guarded by `when pid' + (cl-letf (((symbol-function 'cj/recording--wayland-p) (lambda () nil))) + (cj/video-recording-stop)) + (should (null cj/video-recording-ffmpeg-process))) (test-ffmpeg-teardown))) -(ert-deftest test-video-audio-recording-audio-stop-error-dead-process-raises-error () - "Test that audio stop raises error if process is already dead. -This documents current behavior - interrupt-process on dead process errors. -The sentinel should clear the variable before this happens in practice." +(ert-deftest test-video-audio-recording-audio-stop-error-dead-process-handles-gracefully () + "Test that audio stop handles already-dead process gracefully. +The new implementation guards against nil process-id, so stopping a dead +process should clean up without error." (test-ffmpeg-setup) (unwind-protect (let ((fake-process (make-process :name "test-audio" :command '("sleep" "1000")))) (setq cj/audio-recording-ffmpeg-process fake-process) - ;; Kill process before calling stop (delete-process fake-process) (sit-for 0.1) - ;; Calling stop on dead process raises error - (should-error (cj/audio-recording-stop))) + ;; Should not error - process-id returns nil, guarded by `when pid' + (cj/audio-recording-stop) + (should (null cj/audio-recording-ffmpeg-process))) (test-ffmpeg-teardown))) (ert-deftest test-video-audio-recording-ffmpeg-record-video-boundary-skips-if-already-recording () diff --git a/tests/test-video-audio-recording-get-devices.el b/tests/test-video-audio-recording-get-devices.el index ba7d95b9..0af02bb3 100644 --- a/tests/test-video-audio-recording-get-devices.el +++ b/tests/test-video-audio-recording-get-devices.el @@ -4,9 +4,8 @@ ;; Unit tests for cj/recording-get-devices function. ;; Tests device prompting and validation workflow. ;; -;; NOTE: This function was refactored to use interactive prompts instead of -;; auto-detection. It now prompts the user with y-or-n-p and calls either -;; cj/recording-quick-setup-for-calls or cj/recording-select-devices. +;; NOTE: This function goes straight into quick setup (mic selection) +;; when devices aren't configured — no confirmation dialog. ;;; Code: @@ -46,29 +45,12 @@ (should (equal "preset-monitor" (cdr result))))) (test-get-devices-teardown))) -(ert-deftest test-video-audio-recording-get-devices-normal-prompts-when-not-configured () - "Test that function prompts user when devices not configured." - (test-get-devices-setup) - (unwind-protect - (let ((prompt-called nil)) - (cl-letf (((symbol-function 'y-or-n-p) - (lambda (_prompt) (setq prompt-called t) t)) - ((symbol-function 'cj/recording-quick-setup-for-calls) - (lambda () - (setq cj/recording-mic-device "quick-mic") - (setq cj/recording-system-device "quick-monitor")))) - (cj/recording-get-devices) - (should prompt-called))) - (test-get-devices-teardown))) - -(ert-deftest test-video-audio-recording-get-devices-normal-calls-quick-setup-on-yes () - "Test that function calls quick setup when user answers yes." +(ert-deftest test-video-audio-recording-get-devices-normal-calls-quick-setup () + "Test that function calls quick setup when devices not configured." (test-get-devices-setup) (unwind-protect (let ((quick-setup-called nil)) - (cl-letf (((symbol-function 'y-or-n-p) - (lambda (_prompt) t)) - ((symbol-function 'cj/recording-quick-setup-for-calls) + (cl-letf (((symbol-function 'cj/recording-quick-setup) (lambda () (setq quick-setup-called t) (setq cj/recording-mic-device "quick-mic") @@ -77,29 +59,11 @@ (should quick-setup-called))) (test-get-devices-teardown))) -(ert-deftest test-video-audio-recording-get-devices-normal-calls-select-devices-on-no () - "Test that function calls manual selection when user answers no." - (test-get-devices-setup) - (unwind-protect - (let ((select-called nil)) - (cl-letf (((symbol-function 'y-or-n-p) - (lambda (_prompt) nil)) - ((symbol-function 'cj/recording-select-devices) - (lambda () - (setq select-called t) - (setq cj/recording-mic-device "manual-mic") - (setq cj/recording-system-device "manual-monitor")))) - (cj/recording-get-devices) - (should select-called))) - (test-get-devices-teardown))) - (ert-deftest test-video-audio-recording-get-devices-normal-returns-cons-cell () "Test that function returns (mic . monitor) cons cell." (test-get-devices-setup) (unwind-protect - (cl-letf (((symbol-function 'y-or-n-p) - (lambda (_prompt) t)) - ((symbol-function 'cj/recording-quick-setup-for-calls) + (cl-letf (((symbol-function 'cj/recording-quick-setup) (lambda () (setq cj/recording-mic-device "test-mic") (setq cj/recording-system-device "test-monitor")))) @@ -111,40 +75,38 @@ ;;; Boundary Cases -(ert-deftest test-video-audio-recording-get-devices-boundary-only-mic-set-prompts () - "Test that function prompts even when only mic is set." +(ert-deftest test-video-audio-recording-get-devices-boundary-only-mic-set-calls-setup () + "Test that function calls setup even when only mic is set." (test-get-devices-setup) (unwind-protect (progn (setq cj/recording-mic-device "preset-mic") (setq cj/recording-system-device nil) - (let ((prompt-called nil)) - (cl-letf (((symbol-function 'y-or-n-p) - (lambda (_prompt) (setq prompt-called t) t)) - ((symbol-function 'cj/recording-quick-setup-for-calls) + (let ((quick-setup-called nil)) + (cl-letf (((symbol-function 'cj/recording-quick-setup) (lambda () + (setq quick-setup-called t) (setq cj/recording-mic-device "new-mic") (setq cj/recording-system-device "new-monitor")))) (cj/recording-get-devices) - (should prompt-called)))) + (should quick-setup-called)))) (test-get-devices-teardown))) -(ert-deftest test-video-audio-recording-get-devices-boundary-only-monitor-set-prompts () - "Test that function prompts even when only monitor is set." +(ert-deftest test-video-audio-recording-get-devices-boundary-only-monitor-set-calls-setup () + "Test that function calls setup even when only monitor is set." (test-get-devices-setup) (unwind-protect (progn (setq cj/recording-mic-device nil) (setq cj/recording-system-device "preset-monitor") - (let ((prompt-called nil)) - (cl-letf (((symbol-function 'y-or-n-p) - (lambda (_prompt) (setq prompt-called t) t)) - ((symbol-function 'cj/recording-quick-setup-for-calls) + (let ((quick-setup-called nil)) + (cl-letf (((symbol-function 'cj/recording-quick-setup) (lambda () + (setq quick-setup-called t) (setq cj/recording-mic-device "new-mic") (setq cj/recording-system-device "new-monitor")))) (cj/recording-get-devices) - (should prompt-called)))) + (should quick-setup-called)))) (test-get-devices-teardown))) ;;; Error Cases @@ -153,9 +115,7 @@ "Test that function signals error when setup fails to set devices." (test-get-devices-setup) (unwind-protect - (cl-letf (((symbol-function 'y-or-n-p) - (lambda (_prompt) t)) - ((symbol-function 'cj/recording-quick-setup-for-calls) + (cl-letf (((symbol-function 'cj/recording-quick-setup) (lambda () nil))) ;; Setup fails - doesn't set devices (should-error (cj/recording-get-devices) :type 'user-error)) (test-get-devices-teardown))) @@ -164,27 +124,14 @@ "Test that error message guides user to setup commands." (test-get-devices-setup) (unwind-protect - (cl-letf (((symbol-function 'y-or-n-p) - (lambda (_prompt) t)) - ((symbol-function 'cj/recording-quick-setup-for-calls) + (cl-letf (((symbol-function 'cj/recording-quick-setup) (lambda () nil))) (condition-case err (cj/recording-get-devices) (user-error - (should (string-match-p "C-; r c" (error-message-string err))) + (should (string-match-p "C-; r s" (error-message-string err))) (should (string-match-p "C-; r s" (error-message-string err)))))) (test-get-devices-teardown))) -(ert-deftest test-video-audio-recording-get-devices-error-select-devices-fails () - "Test that function signals error when manual selection fails." - (test-get-devices-setup) - (unwind-protect - (cl-letf (((symbol-function 'y-or-n-p) - (lambda (_prompt) nil)) - ((symbol-function 'cj/recording-select-devices) - (lambda () nil))) ;; Manual selection fails - (should-error (cj/recording-get-devices) :type 'user-error)) - (test-get-devices-teardown))) - (provide 'test-video-audio-recording-get-devices) ;;; test-video-audio-recording-get-devices.el ends here diff --git a/tests/test-video-audio-recording-modeline-indicator.el b/tests/test-video-audio-recording-modeline-indicator.el index f7f3bbff..f717af7f 100644 --- a/tests/test-video-audio-recording-modeline-indicator.el +++ b/tests/test-video-audio-recording-modeline-indicator.el @@ -45,7 +45,7 @@ (let ((fake-process (make-process :name "test-audio" :command '("sleep" "1000")))) (setq cj/audio-recording-ffmpeg-process fake-process) (let ((result (cj/recording-modeline-indicator))) - (should (equal " 🔴Audio " result))) + (should (equal " " result))) (delete-process fake-process)) (test-modeline-indicator-teardown))) @@ -56,7 +56,7 @@ (let ((fake-process (make-process :name "test-video" :command '("sleep" "1000")))) (setq cj/video-recording-ffmpeg-process fake-process) (let ((result (cj/recording-modeline-indicator))) - (should (equal " 🔴Video " result))) + (should (equal " " result))) (delete-process fake-process)) (test-modeline-indicator-teardown))) @@ -69,7 +69,7 @@ (setq cj/audio-recording-ffmpeg-process audio-proc) (setq cj/video-recording-ffmpeg-process video-proc) (let ((result (cj/recording-modeline-indicator))) - (should (equal " 🔴A+V " result))) + (should (equal " " result))) (delete-process audio-proc) (delete-process video-proc)) (test-modeline-indicator-teardown))) @@ -115,7 +115,7 @@ (delete-process dead-proc) (sit-for 0.1) (let ((result (cj/recording-modeline-indicator))) - (should (equal " 🔴Video " result))) + (should (equal " " result))) (delete-process alive-proc)) (test-modeline-indicator-teardown))) diff --git a/tests/test-video-audio-recording-process-cleanup.el b/tests/test-video-audio-recording-process-cleanup.el index 10b17f2c..d1cd442c 100644 --- a/tests/test-video-audio-recording-process-cleanup.el +++ b/tests/test-video-audio-recording-process-cleanup.el @@ -1,11 +1,14 @@ ;;; test-video-audio-recording-process-cleanup.el --- Tests for recording process cleanup -*- lexical-binding: t; -*- ;;; Commentary: -;; Tests that verify proper cleanup of recording processes, especially -;; wf-recorder on Wayland which can become orphaned if not explicitly killed. +;; Tests that verify proper cleanup of recording processes. ;; -;; Unit tests verify pkill is called at the right times. -;; Integration tests verify no orphan processes remain after stop. +;; Key behaviors tested: +;; - SIGINT is sent to the process group (negative PID), not just the shell +;; - Video stop waits for the process to actually exit before declaring done +;; - On Wayland, wf-recorder is killed AFTER ffmpeg exits (safety net) +;; - On X11, pkill is not called (no wf-recorder involved) +;; - Starting a recording cleans up orphan wf-recorder from previous crashes ;;; Code: @@ -38,29 +41,89 @@ (setq cj/recording-mic-device nil) (setq cj/recording-system-device nil)) -;;; Unit Tests - Verify pkill is called +;;; Unit Tests - Signal delivery -(ert-deftest test-video-recording-stop-wayland-calls-pkill-wf-recorder () - "Test that stopping video on Wayland calls pkill to kill wf-recorder." +(ert-deftest test-video-recording-stop-sends-sigint-to-process-group () + "Test that stopping sends SIGINT to the process group via negative PID." (test-cleanup-setup) (unwind-protect (let ((fake-process (make-process :name "test-video" :command '("sleep" "1000"))) - (pkill-called nil) - (pkill-args nil)) + (signaled-pid nil) + (signaled-sig nil)) + (setq cj/video-recording-ffmpeg-process fake-process) + (cl-letf (((symbol-function 'cj/recording--wayland-p) (lambda () nil)) + ((symbol-function 'signal-process) + (lambda (pid sig) + (setq signaled-pid pid) + (setq signaled-sig sig) + 0)) + ((symbol-function 'cj/recording--wait-for-exit) + (lambda (_proc _timeout) t))) + (cj/video-recording-stop) + ;; Should send signal 2 (SIGINT) to negative PID (process group) + (should signaled-pid) + (should (< signaled-pid 0)) + (should (= signaled-sig 2))) + (ignore-errors (delete-process fake-process))) + (test-cleanup-teardown))) + +;;; Unit Tests - Wayland wf-recorder cleanup + +(ert-deftest test-video-recording-stop-wayland-kills-wf-recorder-before-signal () + "Test that on Wayland, pkill wf-recorder is called BEFORE signal-process. +Orderly pipeline shutdown requires killing the producer (wf-recorder) first +so ffmpeg sees EOF on its video input pipe and starts finalizing the file." + (test-cleanup-setup) + (unwind-protect + (let ((fake-process (make-process :name "test-video" :command '("sleep" "1000"))) + (call-order nil)) (setq cj/video-recording-ffmpeg-process fake-process) (cl-letf (((symbol-function 'cj/recording--wayland-p) (lambda () t)) ((symbol-function 'call-process) (lambda (program &rest args) (when (equal program "pkill") - (setq pkill-called t) - (setq pkill-args args)) + (push (cons 'pkill args) call-order)) + 0)) + ((symbol-function 'signal-process) + (lambda (_pid _sig) + (push 'signal call-order) 0)) - ((symbol-function 'interrupt-process) (lambda (_proc) nil))) + ((symbol-function 'cj/recording--wait-for-exit) + (lambda (_proc _timeout) t))) (cj/video-recording-stop) - (should pkill-called) - ;; Should call: pkill -INT wf-recorder - (should (member "-INT" pkill-args)) - (should (member "wf-recorder" pkill-args))) + ;; call-order is reversed (push adds to front), so last element is first call + (let ((order (reverse call-order))) + ;; First call should be pkill (kill producer first) + (should (consp (car order))) + (should (eq 'pkill (caar order))) + ;; signal-process should come after pkill + (should (eq 'signal (cadr order))))) + (ignore-errors (delete-process fake-process))) + (test-cleanup-teardown))) + +(ert-deftest test-video-recording-stop-wayland-calls-pkill-with-correct-args () + "Test that Wayland stop calls pkill -INT wf-recorder." + (test-cleanup-setup) + (unwind-protect + (let ((fake-process (make-process :name "test-video" :command '("sleep" "1000"))) + (pkill-args-list nil)) + (setq cj/video-recording-ffmpeg-process fake-process) + (cl-letf (((symbol-function 'cj/recording--wayland-p) (lambda () t)) + ((symbol-function 'call-process) + (lambda (program &rest args) + (when (equal program "pkill") + (push args pkill-args-list)) + 0)) + ((symbol-function 'signal-process) (lambda (_pid _sig) 0)) + ((symbol-function 'cj/recording--wait-for-exit) + (lambda (_proc _timeout) t))) + (cj/video-recording-stop) + ;; Should call pkill at least once with -INT wf-recorder + (should pkill-args-list) + (should (cl-some (lambda (args) + (and (member "-INT" args) + (member "wf-recorder" args))) + pkill-args-list))) (ignore-errors (delete-process fake-process))) (test-cleanup-teardown))) @@ -77,13 +140,16 @@ (when (equal program "pkill") (setq pkill-called t)) 0)) - ((symbol-function 'interrupt-process) (lambda (_proc) nil))) + ((symbol-function 'signal-process) (lambda (_pid _sig) 0)) + ((symbol-function 'cj/recording--wait-for-exit) + (lambda (_proc _timeout) t))) (cj/video-recording-stop) - ;; Should NOT call pkill on X11 (should-not pkill-called)) (ignore-errors (delete-process fake-process))) (test-cleanup-teardown))) +;;; Unit Tests - Start cleans orphans + (ert-deftest test-video-recording-start-wayland-kills-orphans () "Test that starting video on Wayland kills orphan wf-recorder processes." (test-cleanup-setup) @@ -102,7 +168,6 @@ (lambda (_name _buffer _command) (make-process :name "fake-video" :command '("sleep" "1000"))))) (cj/ffmpeg-record-video video-recordings-dir) - ;; Should call pkill to clean up orphans before starting (should pkill-called) (should (member "-INT" pkill-args)) (should (member "wf-recorder" pkill-args)))) @@ -123,10 +188,95 @@ (lambda (_name _buffer _command) (make-process :name "fake-video" :command '("sleep" "1000"))))) (cj/ffmpeg-record-video video-recordings-dir) - ;; Should NOT call pkill on X11 (should-not pkill-called))) (test-cleanup-teardown))) +;;; Unit Tests - Wait for exit + +(ert-deftest test-video-recording-stop-waits-for-process-exit () + "Test that stop waits for the process to exit instead of blind sit-for." + (test-cleanup-setup) + (unwind-protect + (let ((fake-process (make-process :name "test-video" :command '("sleep" "1000"))) + (wait-called nil) + (wait-timeout nil)) + (setq cj/video-recording-ffmpeg-process fake-process) + (cl-letf (((symbol-function 'cj/recording--wayland-p) (lambda () nil)) + ((symbol-function 'signal-process) (lambda (_pid _sig) 0)) + ((symbol-function 'cj/recording--wait-for-exit) + (lambda (_proc timeout) + (setq wait-called t) + (setq wait-timeout timeout) + t))) + (cj/video-recording-stop) + (should wait-called) + ;; Video uses 5 second timeout for MKV finalization + (should (= wait-timeout 5))) + (ignore-errors (delete-process fake-process))) + (test-cleanup-teardown))) + +(ert-deftest test-video-recording-stop-timeout-shows-warning () + "Test that video stop shows warning when process doesn't exit in time." + (test-cleanup-setup) + (unwind-protect + (let ((fake-process (make-process :name "test-video" :command '("sleep" "1000"))) + (warning-shown nil)) + (setq cj/video-recording-ffmpeg-process fake-process) + (cl-letf (((symbol-function 'cj/recording--wayland-p) (lambda () nil)) + ((symbol-function 'signal-process) (lambda (_pid _sig) 0)) + ((symbol-function 'cj/recording--wait-for-exit) + (lambda (_proc _timeout) nil)) ; Simulate timeout + ((symbol-function 'message) + (lambda (fmt &rest args) + (let ((msg (apply #'format fmt args))) + (when (string-match-p "did not exit" msg) + (setq warning-shown t)))))) + (cj/video-recording-stop) + (should warning-shown)) + (ignore-errors (delete-process fake-process))) + (test-cleanup-teardown))) + +(ert-deftest test-audio-recording-stop-timeout-shows-warning () + "Test that audio stop shows warning when process doesn't exit in time." + (test-cleanup-setup) + (unwind-protect + (let ((fake-process (make-process :name "test-audio" :command '("sleep" "1000"))) + (warning-shown nil)) + (setq cj/audio-recording-ffmpeg-process fake-process) + (cl-letf (((symbol-function 'signal-process) (lambda (_pid _sig) 0)) + ((symbol-function 'cj/recording--wait-for-exit) + (lambda (_proc _timeout) nil)) ; Simulate timeout + ((symbol-function 'message) + (lambda (fmt &rest args) + (let ((msg (apply #'format fmt args))) + (when (string-match-p "did not exit" msg) + (setq warning-shown t)))))) + (cj/audio-recording-stop) + (should warning-shown)) + (ignore-errors (delete-process fake-process))) + (test-cleanup-teardown))) + +(ert-deftest test-audio-recording-stop-waits-for-process-exit () + "Test that audio stop waits for the process to exit." + (test-cleanup-setup) + (unwind-protect + (let ((fake-process (make-process :name "test-audio" :command '("sleep" "1000"))) + (wait-called nil) + (wait-timeout nil)) + (setq cj/audio-recording-ffmpeg-process fake-process) + (cl-letf (((symbol-function 'signal-process) (lambda (_pid _sig) 0)) + ((symbol-function 'cj/recording--wait-for-exit) + (lambda (_proc timeout) + (setq wait-called t) + (setq wait-timeout timeout) + t))) + (cj/audio-recording-stop) + (should wait-called) + ;; Audio uses 3 second timeout for M4A finalization + (should (= wait-timeout 3))) + (ignore-errors (delete-process fake-process))) + (test-cleanup-teardown))) + ;;; Integration Tests - Verify actual process cleanup ;;; These tests require wf-recorder to be installed and a Wayland session @@ -150,21 +300,13 @@ This is an integration test that requires wf-recorder and Wayland." (test-cleanup-setup) (unwind-protect (let ((initial-count (test-cleanup--count-wf-recorder-processes))) - ;; Start recording (cj/ffmpeg-record-video video-recordings-dir) - (sit-for 1.0) ; Let it start - - ;; Should have at least one wf-recorder running + (sit-for 1.0) (should (> (test-cleanup--count-wf-recorder-processes) initial-count)) - - ;; Stop recording (cj/video-recording-stop) - (sit-for 1.0) ; Let cleanup complete - - ;; Should be back to initial count (no orphans) + (sit-for 1.0) (should (= (test-cleanup--count-wf-recorder-processes) initial-count))) (test-cleanup-teardown) - ;; Extra cleanup in case test failed mid-way (ignore-errors (call-process "pkill" nil nil nil "-INT" "wf-recorder")))) (ert-deftest test-integration-video-recording-start-cleans-orphans () @@ -179,28 +321,17 @@ This is an integration test that requires wf-recorder and Wayland." ;; Create an orphan wf-recorder (simulating a crash) (start-process "orphan-wf" nil "wf-recorder" "-c" "libx264" "-m" "matroska" "-f" "/dev/null") (sit-for 0.5) - (let ((orphan-pids (test-cleanup--get-wf-recorder-pids))) - (should orphan-pids) ; Verify orphan was created - - ;; Start recording - should clean up orphan + (should orphan-pids) (cj/ffmpeg-record-video video-recordings-dir) (sit-for 1.0) - - ;; The orphan PIDs should no longer exist (let ((current-pids (test-cleanup--get-wf-recorder-pids))) - ;; Old orphan should be gone (new recording process may exist) (dolist (orphan-pid orphan-pids) - ;; Check if old orphan is still in current pids - ;; It's OK if there's a NEW wf-recorder, just not the orphan (let ((still-running (member orphan-pid current-pids))) (should-not still-running)))) - - ;; Clean up the recording we started (cj/video-recording-stop) (sit-for 0.5))) (test-cleanup-teardown) - ;; Extra cleanup (ignore-errors (call-process "pkill" nil nil nil "-INT" "wf-recorder")))) (ert-deftest test-integration-video-recording-multiple-start-stop-cycles () @@ -212,17 +343,13 @@ This is an integration test that requires wf-recorder and Wayland." (test-cleanup-setup) (unwind-protect (let ((initial-count (test-cleanup--count-wf-recorder-processes))) - ;; Do 3 start/stop cycles (dotimes (_ 3) (cj/ffmpeg-record-video video-recordings-dir) (sit-for 0.5) (cj/video-recording-stop) (sit-for 0.5)) - - ;; Should be back to initial count - no accumulated orphans (should (= (test-cleanup--count-wf-recorder-processes) initial-count))) (test-cleanup-teardown) - ;; Extra cleanup (ignore-errors (call-process "pkill" nil nil nil "-INT" "wf-recorder")))) (provide 'test-video-audio-recording-process-cleanup) diff --git a/tests/test-video-audio-recording-quick-setup-for-calls.el b/tests/test-video-audio-recording-quick-setup-for-calls.el deleted file mode 100644 index 70980ad4..00000000 --- a/tests/test-video-audio-recording-quick-setup-for-calls.el +++ /dev/null @@ -1,144 +0,0 @@ -;;; test-video-audio-recording-quick-setup-for-calls.el --- Tests for cj/recording-quick-setup-for-calls -*- lexical-binding: t; -*- - -;;; Commentary: -;; Unit tests for cj/recording-quick-setup-for-calls function. -;; Tests quick device setup workflow for call recording. - -;;; Code: - -(require 'ert) - -;; Stub dependencies before loading the module -(defvar cj/custom-keymap (make-sparse-keymap) - "Stub keymap for testing.") - -;; Now load the actual production module -(require 'video-audio-recording) - -;;; Setup and Teardown - -(defun test-quick-setup-setup () - "Reset device variables before each test." - (setq cj/recording-mic-device nil) - (setq cj/recording-system-device nil)) - -(defun test-quick-setup-teardown () - "Clean up device variables after each test." - (setq cj/recording-mic-device nil) - (setq cj/recording-system-device nil)) - -;;; Normal Cases - -(ert-deftest test-video-audio-recording-quick-setup-for-calls-normal-sets-both-devices () - "Test that function sets both mic and system device variables." - (test-quick-setup-setup) - (unwind-protect - (let ((grouped-devices '(("Bluetooth Headset" . ("bluez_input.00:1B:66" . "bluez_output.00_1B_66.monitor"))))) - (cl-letf (((symbol-function 'cj/recording-group-devices-by-hardware) - (lambda () grouped-devices)) - ((symbol-function 'completing-read) - (lambda (_prompt _choices &rest _args) "Bluetooth Headset"))) - (cj/recording-quick-setup-for-calls) - (should (equal "bluez_input.00:1B:66" cj/recording-mic-device)) - (should (equal "bluez_output.00_1B_66.monitor" cj/recording-system-device)))) - (test-quick-setup-teardown))) - -(ert-deftest test-video-audio-recording-quick-setup-for-calls-normal-presents-friendly-names () - "Test that function presents friendly device names to user." - (test-quick-setup-setup) - (unwind-protect - (let ((grouped-devices '(("Jabra SPEAK 510 USB" . ("usb-input" . "usb-monitor")) - ("Built-in Audio" . ("pci-input" . "pci-monitor")))) - (presented-choices nil)) - (cl-letf (((symbol-function 'cj/recording-group-devices-by-hardware) - (lambda () grouped-devices)) - ((symbol-function 'completing-read) - (lambda (_prompt choices &rest _args) - (setq presented-choices choices) - (car choices)))) - (cj/recording-quick-setup-for-calls) - (should (member "Jabra SPEAK 510 USB" presented-choices)) - (should (member "Built-in Audio" presented-choices)))) - (test-quick-setup-teardown))) - -(ert-deftest test-video-audio-recording-quick-setup-for-calls-normal-displays-confirmation () - "Test that function displays confirmation message with device details." - (test-quick-setup-setup) - (unwind-protect - (let ((grouped-devices '(("Bluetooth Headset" . ("bluez_input.00:1B:66" . "bluez_output.00_1B_66.monitor")))) - (message-text nil)) - (cl-letf (((symbol-function 'cj/recording-group-devices-by-hardware) - (lambda () grouped-devices)) - ((symbol-function 'completing-read) - (lambda (_prompt _choices &rest _args) "Bluetooth Headset")) - ((symbol-function 'message) - (lambda (fmt &rest args) (setq message-text (apply #'format fmt args))))) - (cj/recording-quick-setup-for-calls) - (should (string-match-p "Call recording ready" message-text)) - (should (string-match-p "Bluetooth Headset" message-text)))) - (test-quick-setup-teardown))) - -;;; Boundary Cases - -(ert-deftest test-video-audio-recording-quick-setup-for-calls-boundary-single-device-no-prompt () - "Test that with single device, selection still happens." - (test-quick-setup-setup) - (unwind-protect - (let ((grouped-devices '(("Built-in Audio" . ("pci-input" . "pci-monitor"))))) - (cl-letf (((symbol-function 'cj/recording-group-devices-by-hardware) - (lambda () grouped-devices)) - ((symbol-function 'completing-read) - (lambda (_prompt _choices &rest _args) "Built-in Audio"))) - (cj/recording-quick-setup-for-calls) - (should (equal "pci-input" cj/recording-mic-device)) - (should (equal "pci-monitor" cj/recording-system-device)))) - (test-quick-setup-teardown))) - -(ert-deftest test-video-audio-recording-quick-setup-for-calls-boundary-device-name-with-special-chars () - "Test that device names with special characters are handled correctly." - (test-quick-setup-setup) - (unwind-protect - (let ((grouped-devices '(("Device (USB-C)" . ("special-input" . "special-monitor"))))) - (cl-letf (((symbol-function 'cj/recording-group-devices-by-hardware) - (lambda () grouped-devices)) - ((symbol-function 'completing-read) - (lambda (_prompt _choices &rest _args) "Device (USB-C)"))) - (cj/recording-quick-setup-for-calls) - (should (equal "special-input" cj/recording-mic-device)) - (should (equal "special-monitor" cj/recording-system-device)))) - (test-quick-setup-teardown))) - -;;; Error Cases - -(ert-deftest test-video-audio-recording-quick-setup-for-calls-error-no-devices-signals-error () - "Test that function signals user-error when no complete devices are found." - (test-quick-setup-setup) - (unwind-protect - (cl-letf (((symbol-function 'cj/recording-group-devices-by-hardware) - (lambda () nil))) - (should-error (cj/recording-quick-setup-for-calls) :type 'user-error)) - (test-quick-setup-teardown))) - -(ert-deftest test-video-audio-recording-quick-setup-for-calls-error-message-mentions-both-devices () - "Test that error message mentions need for both mic and monitor." - (test-quick-setup-setup) - (unwind-protect - (cl-letf (((symbol-function 'cj/recording-group-devices-by-hardware) - (lambda () nil))) - (condition-case err - (cj/recording-quick-setup-for-calls) - (user-error - (should (string-match-p "both mic and monitor" (error-message-string err)))))) - (test-quick-setup-teardown))) - -(ert-deftest test-video-audio-recording-quick-setup-for-calls-error-empty-device-list () - "Test that empty device list from grouping is handled gracefully." - (test-quick-setup-setup) - (unwind-protect - (cl-letf (((symbol-function 'cj/recording-group-devices-by-hardware) - (lambda () '()))) - (should-error (cj/recording-quick-setup-for-calls) :type 'user-error)) - (test-quick-setup-teardown))) - -(provide 'test-video-audio-recording-quick-setup-for-calls) -;;; test-video-audio-recording-quick-setup-for-calls.el ends here diff --git a/tests/test-video-audio-recording-quick-setup.el b/tests/test-video-audio-recording-quick-setup.el new file mode 100644 index 00000000..8b46b9b0 --- /dev/null +++ b/tests/test-video-audio-recording-quick-setup.el @@ -0,0 +1,172 @@ +;;; test-video-audio-recording-quick-setup.el --- Tests for cj/recording-quick-setup -*- lexical-binding: t; -*- + +;;; Commentary: +;; Unit tests for cj/recording-quick-setup function. +;; The quick setup shows available mics and auto-selects the default +;; sink's monitor for system audio capture. + +;;; Code: + +(require 'ert) + +;; Stub dependencies before loading the module +(defvar cj/custom-keymap (make-sparse-keymap) + "Stub keymap for testing.") + +;; Now load the actual production module +(require 'video-audio-recording) + +;;; Setup and Teardown + +(defun test-quick-setup-setup () + "Reset device variables before each test." + (setq cj/recording-mic-device nil) + (setq cj/recording-system-device nil)) + +(defun test-quick-setup-teardown () + "Clean up device variables after each test." + (setq cj/recording-mic-device nil) + (setq cj/recording-system-device nil)) + +;;; Normal Cases + +(ert-deftest test-video-audio-recording-quick-setup-normal-sets-mic-device () + "Test that selecting a mic sets cj/recording-mic-device." + (test-quick-setup-setup) + (unwind-protect + (cl-letf (((symbol-function 'cj/recording--get-available-mics) + (lambda () '(("jabra-input" . "Jabra SPEAK 510 Mono") + ("builtin-input" . "Built-in Analog")))) + ((symbol-function 'cj/recording--get-default-sink-monitor) + (lambda () "jds-labs.monitor")) + ((symbol-function 'completing-read) + (lambda (_prompt table &rest _args) + (car (all-completions "" table))))) + (cj/recording-quick-setup) + (should (equal "jabra-input" cj/recording-mic-device))) + (test-quick-setup-teardown))) + +(ert-deftest test-video-audio-recording-quick-setup-normal-sets-system-to-default-monitor () + "Test that system device is set to the default sink's monitor." + (test-quick-setup-setup) + (unwind-protect + (cl-letf (((symbol-function 'cj/recording--get-available-mics) + (lambda () '(("jabra-input" . "Jabra SPEAK 510 Mono")))) + ((symbol-function 'cj/recording--get-default-sink-monitor) + (lambda () "alsa_output.usb-JDS_Labs-00.analog-stereo.monitor")) + ((symbol-function 'completing-read) + (lambda (_prompt table &rest _args) + (car (all-completions "" table))))) + (cj/recording-quick-setup) + (should (equal "alsa_output.usb-JDS_Labs-00.analog-stereo.monitor" + cj/recording-system-device))) + (test-quick-setup-teardown))) + +(ert-deftest test-video-audio-recording-quick-setup-normal-presents-descriptions () + "Test that completing-read receives friendly descriptions and Cancel option." + (test-quick-setup-setup) + (unwind-protect + (let ((presented-candidates nil)) + (cl-letf (((symbol-function 'cj/recording--get-available-mics) + (lambda () '(("raw-device-1" . "Jabra SPEAK 510 Mono") + ("raw-device-2" . "Built-in Analog")))) + ((symbol-function 'cj/recording--get-default-sink-monitor) + (lambda () "default.monitor")) + ((symbol-function 'completing-read) + (lambda (_prompt table &rest _args) + (setq presented-candidates (all-completions "" table)) + (car presented-candidates)))) + (cj/recording-quick-setup) + ;; Candidates should have friendly descriptions + (should (member "Jabra SPEAK 510 Mono" presented-candidates)) + (should (member "Built-in Analog" presented-candidates)) + ;; Cancel option should be present + (should (member "Cancel" presented-candidates)))) + (test-quick-setup-teardown))) + +(ert-deftest test-video-audio-recording-quick-setup-normal-confirmation-message () + "Test that confirmation message mentions the selected mic and monitor." + (test-quick-setup-setup) + (unwind-protect + (let ((message-text nil)) + (cl-letf (((symbol-function 'cj/recording--get-available-mics) + (lambda () '(("jabra-input" . "Jabra SPEAK 510 Mono")))) + ((symbol-function 'cj/recording--get-default-sink-monitor) + (lambda () "jds-labs.analog-stereo.monitor")) + ((symbol-function 'completing-read) + (lambda (_prompt table &rest _args) + (car (all-completions "" table)))) + ((symbol-function 'message) + (lambda (fmt &rest args) + (setq message-text (apply #'format fmt args))))) + (cj/recording-quick-setup) + (should (string-match-p "Recording ready" message-text)) + (should (string-match-p "Jabra SPEAK 510 Mono" message-text)) + (should (string-match-p "default output monitor" message-text)))) + (test-quick-setup-teardown))) + +;;; Boundary Cases + +(ert-deftest test-video-audio-recording-quick-setup-boundary-single-mic () + "Test that with only one mic, it still presents selection." + (test-quick-setup-setup) + (unwind-protect + (let ((read-called nil)) + (cl-letf (((symbol-function 'cj/recording--get-available-mics) + (lambda () '(("sole-mic" . "Only Mic Available")))) + ((symbol-function 'cj/recording--get-default-sink-monitor) + (lambda () "default.monitor")) + ((symbol-function 'completing-read) + (lambda (_prompt table &rest _args) + (setq read-called t) + (car (all-completions "" table))))) + (cj/recording-quick-setup) + (should read-called) + (should (equal "sole-mic" cj/recording-mic-device)))) + (test-quick-setup-teardown))) + +;;; Error Cases + +(ert-deftest test-video-audio-recording-quick-setup-error-cancel-selected () + "Test that selecting Cancel signals user-error and does not set devices." + (test-quick-setup-setup) + (unwind-protect + (cl-letf (((symbol-function 'cj/recording--get-available-mics) + (lambda () '(("jabra-input" . "Jabra SPEAK 510 Mono")))) + ((symbol-function 'cj/recording--get-default-sink-monitor) + (lambda () "default.monitor")) + ((symbol-function 'completing-read) + (lambda (_prompt _choices &rest _args) + "Cancel"))) + (should-error (cj/recording-quick-setup) :type 'user-error) + (should (null cj/recording-mic-device)) + (should (null cj/recording-system-device))) + (test-quick-setup-teardown))) + +(ert-deftest test-video-audio-recording-quick-setup-error-no-mics () + "Test that function signals error when no mics are found." + (test-quick-setup-setup) + (unwind-protect + (cl-letf (((symbol-function 'cj/recording--get-available-mics) + (lambda () nil)) + ((symbol-function 'cj/recording--get-default-sink-monitor) + (lambda () "default.monitor"))) + (should-error (cj/recording-quick-setup) :type 'user-error)) + (test-quick-setup-teardown))) + +(ert-deftest test-video-audio-recording-quick-setup-error-no-mics-message () + "Test that error message mentions mic and unmuted." + (test-quick-setup-setup) + (unwind-protect + (cl-letf (((symbol-function 'cj/recording--get-available-mics) + (lambda () nil)) + ((symbol-function 'cj/recording--get-default-sink-monitor) + (lambda () "default.monitor"))) + (condition-case err + (cj/recording-quick-setup) + (user-error + (should (string-match-p "mic" (error-message-string err)))))) + (test-quick-setup-teardown))) + +(provide 'test-video-audio-recording-quick-setup) +;;; test-video-audio-recording-quick-setup.el ends here diff --git a/tests/test-video-audio-recording-test-mic.el b/tests/test-video-audio-recording-test-mic.el index 5aa794bb..60b9eb0b 100644 --- a/tests/test-video-audio-recording-test-mic.el +++ b/tests/test-video-audio-recording-test-mic.el @@ -124,7 +124,7 @@ (condition-case err (cj/recording-test-mic) (user-error - (should (string-match-p "C-; r c" (error-message-string err)))))) + (should (string-match-p "C-; r s" (error-message-string err)))))) (test-mic-teardown))) (ert-deftest test-video-audio-recording-test-mic-error-ffmpeg-failure-handled () diff --git a/tests/test-video-audio-recording-test-monitor.el b/tests/test-video-audio-recording-test-monitor.el index f1476577..d821600f 100644 --- a/tests/test-video-audio-recording-test-monitor.el +++ b/tests/test-video-audio-recording-test-monitor.el @@ -125,7 +125,7 @@ (condition-case err (cj/recording-test-monitor) (user-error - (should (string-match-p "C-; r c" (error-message-string err)))))) + (should (string-match-p "C-; r s" (error-message-string err)))))) (test-monitor-teardown))) (ert-deftest test-video-audio-recording-test-monitor-error-ffmpeg-failure-handled () |