Skip to content

Commit d6d99c8

Browse files
committed
Fix screen-recording capture on macOS without Homebrew ffmpeg
The user's "EE4802 screen recording not captured" bug wasn't in the download path — it was in frame_extractor.py, which still used bare "ffmpeg" and "ffprobe" subprocess literals. On macOS users whose only ffmpeg comes from the imageio-ffmpeg wheel (which doesn't ship ffprobe), every scene-detection / frame-extraction call silently FileNotFoundError'd. The output PNG never appeared, so the video got classified as "camera" (no frames = default), screen-recording frame extraction was skipped, and notes ended up with no slide content. frame_extractor.py: - Add _resolve_ffmpeg() / _resolve_ffprobe() / _parse_ffmpeg_duration(), mirroring the resolver introduced in extract_caption.py for issue #7. - Cache resolved paths at module level (hot path: one call per scene frame). - Replace all 4 "ffmpeg" and 1 "ffprobe" literals with resolved paths. - get_video_duration falls back to parsing ffmpeg stderr when ffprobe absent. downloader.py (_run_ffmpeg_hls hardening): - Add -y flag (idempotent against stale partial files). - Add -loglevel error + -map 0 (keep all selected-variant streams). - Check output-file size after FfmpegProgress run — progress library doesn't raise on non-zero exit, so a silent failure would otherwise write a 0-byte mp4 that downstream stages mangled. - Surface last 10 lines of stderr when returncode != 0 in the no-progress-library fallback. test/test_v0_12_fixes.py: +15 tests (now 65 total) - Frame-extractor resolver coverage (cache, system/imageio fallback, errors). - get_video_duration cross-validation (both paths agree, zero on garbage). - _run_ffmpeg_hls hardening (raises on missing/tiny output, -y presence, stderr surfaced on non-zero exit). - Source scan asserts no bare ["ffmpeg", ...] / ["ffprobe", ...] launch lists remain in frame_extractor.py. Release: v0.12.10.
1 parent 2ee0573 commit d6d99c8

4 files changed

Lines changed: 413 additions & 19 deletions

File tree

downloader.py

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -946,6 +946,12 @@ def _run_ffmpeg_hls(stream_url: str, out_path: Path, progress_cb) -> None:
946946
947947
Uses the bundled `imageio-ffmpeg` binary when no system ffmpeg is
948948
present, so macOS users without Homebrew get a working downloader.
949+
950+
Dropping `-f hls` lets ffmpeg auto-detect from the URL — this is more
951+
robust when the master playlist references a mixed-protocol variant set.
952+
`-map 0` keeps all streams from the selected variant (video + audio).
953+
Non-zero exit codes are surfaced as RuntimeError so partial/empty files
954+
don't silently get written to disk.
949955
"""
950956
ff_bin = _resolve_ffmpeg()
951957
if not ff_bin:
@@ -954,17 +960,40 @@ def _run_ffmpeg_hls(stream_url: str, out_path: Path, progress_cb) -> None:
954960
"macOS or `choco install ffmpeg` on Windows), or `pip install "
955961
"imageio-ffmpeg` for a bundled fallback."
956962
)
957-
cmd = [ff_bin, "-f", "hls", "-i", stream_url, "-c", "copy", str(out_path)]
963+
# -y : overwrite partial leftovers from a prior failed attempt
964+
# -loglevel error: only surface real errors (progress plugin reads stderr)
965+
# -map 0 : keep every stream of the selected variant (a/v)
966+
# -c copy : no re-encoding; lossless + fast
967+
cmd = [
968+
ff_bin, "-y", "-loglevel", "error",
969+
"-i", stream_url,
970+
"-map", "0",
971+
"-c", "copy",
972+
str(out_path),
973+
]
958974
try:
959975
from ffmpeg_progress_yield import FfmpegProgress
960976
ff = FfmpegProgress(cmd)
961977
for pct in ff.run_command_with_progress():
962978
progress_cb(pct)
979+
# FfmpegProgress doesn't raise on non-zero exit; inspect the
980+
# resulting file to catch silent failures.
981+
if not out_path.exists() or out_path.stat().st_size < 1024:
982+
raise RuntimeError(
983+
f"ffmpeg produced no output for {out_path.name} — "
984+
f"check network connectivity or stream auth."
985+
)
963986
except ImportError:
964-
import subprocess
965987
tqdm.write(" (ffmpeg-progress-yield not available, running ffmpeg without progress)")
966-
cmd_y = [ff_bin, "-y", "-f", "hls", "-i", stream_url, "-c", "copy", str(out_path)]
967-
subprocess.run(cmd_y, capture_output=True, timeout=3600)
988+
result = subprocess.run(
989+
cmd, capture_output=True, text=True, timeout=3600,
990+
)
991+
if result.returncode != 0:
992+
tail = (result.stderr or "").strip().splitlines()[-10:]
993+
raise RuntimeError(
994+
f"ffmpeg failed (exit {result.returncode}):\n"
995+
+ "\n".join(tail)
996+
)
968997
progress_cb(100)
969998

970999

electron/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "auto-note",
3-
"version": "0.12.9",
3+
"version": "0.12.10",
44
"description": "AutoNote — lecture notes generator from Canvas recordings",
55
"homepage": "https://github.com/nodeeeeee/Auto-Note",
66
"author": {

frame_extractor.py

Lines changed: 125 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,97 @@
4343
# Prevent console windows flashing on Windows
4444
_SUBPROCESS_FLAGS = subprocess.CREATE_NO_WINDOW if sys.platform == "win32" else 0
4545

46+
47+
# ── ffmpeg/ffprobe resolution ─────────────────────────────────────────────────
48+
# macOS users without Homebrew get ffmpeg via the `imageio-ffmpeg` wheel, which
49+
# does NOT ship `ffprobe`. Without these resolvers, every subprocess call below
50+
# raised FileNotFoundError and produced zero frames — silently, because the
51+
# code only checks whether the output PNG exists afterwards. That turned into
52+
# "screen recording not captured" in generated notes on EE4802 test runs.
53+
54+
_FFMPEG_BIN: str | None = None
55+
_FFPROBE_BIN: str | None = None # None sentinel means "resolved; unavailable"
56+
_FFPROBE_RESOLVED = False
57+
58+
59+
def _resolve_ffmpeg() -> str:
60+
"""Locate an ffmpeg executable.
61+
62+
Order: system PATH → imageio-ffmpeg bundled binary → auto-install. Raises
63+
RuntimeError if none works. Result is cached so resolution cost is paid
64+
once per process.
65+
"""
66+
global _FFMPEG_BIN
67+
if _FFMPEG_BIN:
68+
return _FFMPEG_BIN
69+
70+
from shutil import which
71+
sys_ff = which("ffmpeg")
72+
if sys_ff:
73+
_FFMPEG_BIN = sys_ff
74+
return sys_ff
75+
76+
def _try_imageio() -> str | None:
77+
try:
78+
import imageio_ffmpeg
79+
return imageio_ffmpeg.get_ffmpeg_exe()
80+
except ImportError:
81+
return None
82+
except Exception as e:
83+
print(f" [warn] imageio-ffmpeg get_ffmpeg_exe failed: {e}")
84+
return None
85+
86+
ff = _try_imageio()
87+
if ff:
88+
_FFMPEG_BIN = ff
89+
return ff
90+
91+
print(" ffmpeg not found locally — installing imageio-ffmpeg fallback…")
92+
try:
93+
subprocess.run(
94+
[sys.executable, "-m", "pip", "install", "--quiet",
95+
"--disable-pip-version-check", "imageio-ffmpeg"],
96+
check=True, timeout=180,
97+
)
98+
except Exception as e:
99+
raise RuntimeError(f"ffmpeg unavailable and auto-install failed: {e}") from e
100+
101+
ff = _try_imageio()
102+
if not ff:
103+
raise RuntimeError("ffmpeg unavailable after imageio-ffmpeg install")
104+
_FFMPEG_BIN = ff
105+
return ff
106+
107+
108+
def _resolve_ffprobe() -> str | None:
109+
"""Locate ffprobe on system PATH. Returns None when unavailable.
110+
111+
imageio-ffmpeg doesn't bundle ffprobe, so when only it is available we
112+
fall back to parsing `ffmpeg -i` stderr for duration.
113+
"""
114+
global _FFPROBE_BIN, _FFPROBE_RESOLVED
115+
if _FFPROBE_RESOLVED:
116+
return _FFPROBE_BIN
117+
from shutil import which
118+
_FFPROBE_BIN = which("ffprobe")
119+
_FFPROBE_RESOLVED = True
120+
return _FFPROBE_BIN
121+
122+
123+
_DURATION_RE = re.compile(r"Duration:\s*(\d+):(\d+):(\d+(?:\.\d+)?)")
124+
125+
126+
def _parse_ffmpeg_duration(stderr: str | None) -> float | None:
127+
"""Parse 'Duration: HH:MM:SS.ss' from ffmpeg -i stderr."""
128+
if not stderr:
129+
return None
130+
m = _DURATION_RE.search(stderr)
131+
if not m:
132+
return None
133+
h, mi, s = m.group(1), m.group(2), m.group(3)
134+
return int(h) * 3600 + int(mi) * 60 + float(s)
135+
136+
46137
# ── Tunable constants ────────────────────────────────────────────────────────
47138

48139
# Scene detection threshold: lower = more sensitive (more frames extracted).
@@ -115,10 +206,11 @@ def classify_video(video_path: Path) -> str:
115206

116207
tmp_dir = Path(tempfile.mkdtemp(prefix="classify_"))
117208
frame_paths = []
209+
ff_bin = _resolve_ffmpeg()
118210
for i, ts in enumerate(timestamps):
119211
png = tmp_dir / f"sample_{i}.png"
120212
subprocess.run(
121-
["ffmpeg", "-ss", f"{ts:.1f}", "-i", str(video_path),
213+
[ff_bin, "-ss", f"{ts:.1f}", "-i", str(video_path),
122214
"-frames:v", "1", "-q:v", "2", str(png), "-y"],
123215
capture_output=True, timeout=30,
124216
creationflags=_SUBPROCESS_FLAGS,
@@ -287,8 +379,9 @@ def detect_scenes(video_path: Path, threshold: float = SCENE_THRESHOLD,
287379
always_periodic = False
288380

289381
# ── Pass 1: ffmpeg scene detection ───────────────────────────────────────
382+
ff_bin = _resolve_ffmpeg()
290383
cmd = [
291-
"ffmpeg", "-i", str(video_path),
384+
ff_bin, "-i", str(video_path),
292385
"-vf", f"select='gt(scene\\,{threshold})',showinfo",
293386
"-vsync", "vfr",
294387
"-f", "null", "-"
@@ -348,7 +441,7 @@ def detect_scenes(video_path: Path, threshold: float = SCENE_THRESHOLD,
348441
for ts in raw_timestamps[:MAX_FRAMES * 2]:
349442
png = tmp_dir / f"cand_{ts:.1f}.png"
350443
subprocess.run(
351-
["ffmpeg", "-ss", f"{ts:.3f}", "-i", str(video_path),
444+
[ff_bin, "-ss", f"{ts:.3f}", "-i", str(video_path),
352445
"-frames:v", "1", "-q:v", "3", str(png), "-y"],
353446
capture_output=True, timeout=30,
354447
creationflags=_SUBPROCESS_FLAGS,
@@ -409,19 +502,37 @@ def detect_scenes(video_path: Path, threshold: float = SCENE_THRESHOLD,
409502

410503

411504
def get_video_duration(video_path: Path) -> float:
412-
"""Get video duration in seconds using ffprobe."""
413-
cmd = [
414-
"ffprobe", "-v", "quiet", "-show_entries", "format=duration",
415-
"-of", "default=noprint_wrappers=1:nokey=1", str(video_path)
416-
]
505+
"""Return video duration in seconds.
506+
507+
Uses ffprobe when available, falls back to parsing ffmpeg's stderr
508+
('Duration: HH:MM:SS.ss'). The fallback is the path macOS users hit
509+
when they only have the imageio-ffmpeg wheel — which bundles ffmpeg
510+
but not ffprobe.
511+
"""
512+
ffprobe = _resolve_ffprobe()
513+
if ffprobe:
514+
result = subprocess.run(
515+
[ffprobe, "-v", "quiet", "-show_entries", "format=duration",
516+
"-of", "default=noprint_wrappers=1:nokey=1", str(video_path)],
517+
capture_output=True, text=True, timeout=30,
518+
creationflags=_SUBPROCESS_FLAGS,
519+
)
520+
try:
521+
return float(result.stdout.strip())
522+
except (ValueError, AttributeError):
523+
return 0.0
524+
525+
try:
526+
ff = _resolve_ffmpeg()
527+
except RuntimeError:
528+
return 0.0
417529
result = subprocess.run(
418-
cmd, capture_output=True, text=True, timeout=30,
530+
[ff, "-hide_banner", "-i", str(video_path)],
531+
capture_output=True, text=True, timeout=30,
419532
creationflags=_SUBPROCESS_FLAGS,
420533
)
421-
try:
422-
return float(result.stdout.strip())
423-
except (ValueError, AttributeError):
424-
return 0.0
534+
dur = _parse_ffmpeg_duration(result.stderr)
535+
return dur if dur is not None else 0.0
425536

426537

427538
# ── Per-frame screen/camera classification ────────────────────────────────────
@@ -658,7 +769,7 @@ def extract_frames(video_path: Path, timestamps: list[float],
658769
continue
659770

660771
cmd = [
661-
"ffmpeg", "-ss", f"{ts:.3f}",
772+
_resolve_ffmpeg(), "-ss", f"{ts:.3f}",
662773
"-i", str(video_path),
663774
"-frames:v", "1",
664775
"-q:v", "2",

0 commit comments

Comments
 (0)