fix(gif): correct the streaming threshold and three encode-path hazards
This repo has no CI and no adversarial reviewer, so these come from reading the harness back against the production sandbox contract rather than from a bot. The size estimate was wrong by ~1000x in the wrong direction. `width * height * 9e-5` was never MiB — for a 640x480 frame it yields 27.6, so a trivial 2-second GIF "projected" 800+ MiB and every render would have tripped the streaming branch and come back as an MP4. The bench missed it because it bind-mounted /workspace from the host disk, where statvfs reports hundreds of GB and the threshold never fires; production mounts a 64 MiB tmpfs, where it always would have. Replaced with a named PNG_BYTES_PER_PIXEL = 0.29, measured (86 KiB per busy 640x480 frame). Verified against the real tmpfs: a 6s piece now estimates 7.6 MiB against a 38 MiB budget and stays a GIF, while a 90s piece streams. Three smaller ones on the streaming path: - ffmpeg's stderr was a pipe nobody drained until the encode finished, so a chatty encoder could fill the 64 KiB buffer, block, stop reading stdin, and deadlock a long render halfway. It writes to a file now. - _close_pipe assumed a live pipe; if ffmpeg had already died, closing stdin raised BrokenPipeError and buried the real exit code and log. - A failed encode could leave a half-written /workspace/final.* behind, which still comes back as a files_out file_id — a broken artifact that looks deliverable is worse than none, so it's removed on the way out.
This commit is contained in:
+34
-7
@@ -60,6 +60,12 @@ import numpy as np # noqa: E402,F401
|
||||
GIF_MAX_SECONDS = 20
|
||||
# Keep a GIF comfortably under Discord's ~10 MiB inline limit.
|
||||
GIF_MAX_BYTES = 9 * 1024 * 1024
|
||||
# Measured on busy 640x480 matplotlib frames in this sandbox: ~86 KiB per PNG at
|
||||
# compress_level=1, i.e. 88064 / 307200 px. This decides whether a render streams
|
||||
# instead of staging frames on disk, so it must not read low — erring low
|
||||
# reintroduces the ENOSPC this harness exists to prevent, and erring high only
|
||||
# turns a borderline short piece into an MP4.
|
||||
PNG_BYTES_PER_PIXEL = 0.29
|
||||
|
||||
_FONT_CANDIDATES = [
|
||||
"/usr/share/fonts/truetype/dejavu/DejaVuSans-Bold.ttf",
|
||||
@@ -212,8 +218,7 @@ class Animation:
|
||||
|
||||
total_s = sum(s.seconds + s.hold for s in self._scenes)
|
||||
est_frames = int(total_s * self.fps)
|
||||
# ~90 KiB per 640x480 PNG at compress_level=1; budget 60% of free space.
|
||||
est_mb = est_frames * (self.width * self.height * 9e-5)
|
||||
est_mb = est_frames * self.width * self.height * PNG_BYTES_PER_PIXEL / 1048576
|
||||
|
||||
# A piece too long for a GIF is an MP4 anyway, and an MP4 can be encoded
|
||||
# in one streaming pass — so pipe the frames into ffmpeg and keep NOTHING
|
||||
@@ -276,7 +281,17 @@ class Animation:
|
||||
"that differs by a character) and re-run the whole program in a "
|
||||
"fresh code_exec call, remembering gifsmith.py in files_in.")
|
||||
|
||||
try:
|
||||
art = self._encode(total_s, long_edge, force, audio)
|
||||
except Exception: # noqa: BLE001
|
||||
# A half-written final.* at the top of /workspace still comes back as
|
||||
# a files_out file_id, and a broken artifact that LOOKS deliverable is
|
||||
# worse than none — the agent would ship it. Take it with us.
|
||||
for leftover in ("/workspace/final.mp4", "/workspace/final.gif"):
|
||||
if os.path.exists(leftover):
|
||||
os.remove(leftover)
|
||||
shutil.rmtree(self._scratch, ignore_errors=True)
|
||||
raise
|
||||
self._contact_sheet()
|
||||
broken = [f"{i}:{s.title!r}" for i, s in enumerate(self._scenes) if s.error]
|
||||
print(f"[gifsmith] {self._count} frames, scratch "
|
||||
@@ -297,7 +312,11 @@ class Animation:
|
||||
# ------------------------------------------------------------------ encoding
|
||||
|
||||
def _open_pipe(self, out, long_edge, audio):
|
||||
"""ffmpeg reading a PNG stream on stdin — the streaming MP4 path."""
|
||||
"""ffmpeg reading a PNG stream on stdin — the streaming MP4 path.
|
||||
|
||||
stderr goes to a FILE, not a pipe: nobody drains a stderr pipe until the
|
||||
encode is done, so a chatty ffmpeg would fill the 64 KiB pipe buffer,
|
||||
block, stop reading stdin, and deadlock the render halfway through."""
|
||||
cmd = ["ffmpeg", "-y", "-loglevel", "error",
|
||||
"-f", "image2pipe", "-framerate", str(self.fps), "-i", "-"]
|
||||
if audio:
|
||||
@@ -305,15 +324,23 @@ class Animation:
|
||||
cmd += ["-c:v", "libx264", "-pix_fmt", "yuv420p", "-crf", "23",
|
||||
"-vf", f"scale='min({long_edge},iw)':-2:flags=lanczos",
|
||||
"-movflags", "+faststart", out]
|
||||
self._pipe_err = open(os.path.join(self._scratch, "ffmpeg.log"), "w+b")
|
||||
return subprocess.Popen(cmd, stdin=subprocess.PIPE,
|
||||
stdout=subprocess.DEVNULL, stderr=subprocess.PIPE)
|
||||
stdout=subprocess.DEVNULL, stderr=self._pipe_err)
|
||||
|
||||
def _close_pipe(self):
|
||||
try:
|
||||
self._pipe.stdin.close()
|
||||
err = self._pipe.stderr.read().decode(errors="replace")
|
||||
if self._pipe.wait() != 0:
|
||||
raise RuntimeError(f"ffmpeg (streaming) failed:\n{err[-2000:]}")
|
||||
except OSError:
|
||||
# ffmpeg already died; its exit code + log below say why.
|
||||
pass
|
||||
rc = self._pipe.wait()
|
||||
self._pipe_err.seek(0)
|
||||
err = self._pipe_err.read().decode(errors="replace")
|
||||
self._pipe_err.close()
|
||||
self._pipe = None
|
||||
if rc != 0:
|
||||
raise RuntimeError(f"ffmpeg (streaming) exited {rc}:\n{err[-2000:]}")
|
||||
|
||||
def _encode(self, seconds, long_edge, force, audio=None):
|
||||
gif, mp4 = "/workspace/final.gif", "/workspace/final.mp4"
|
||||
|
||||
Reference in New Issue
Block a user