From dc8ce5ab02eb29abe8f829cbf00823681ceba79a Mon Sep 17 00:00:00 2001 From: MBUGUA Date: Fri, 18 Sep 2026 13:06:25 +0300 Subject: [PATCH] fix: render on a live context so the encoders are not killed at Start Evaluate cancelled the recording context in teardown and then handed that same context to Render. Since the encoders are built with exec.CommandContext, every ffmpeg invocation was killed at Start and wrote nothing, while Render logged the empty output and returned nil. VHS printed "Creating out.gif...", exited 0, and produced no file. Give the recorder its own context and render on the parent, and return the encode error instead of swallowing it. Fixes #787 --- evaluator.go | 14 ++++++++++---- render_test.go | 31 +++++++++++++++++++++++++++++++ vhs.go | 1 + 3 files changed, 42 insertions(+), 4 deletions(-) create mode 100644 render_test.go diff --git a/evaluator.go b/evaluator.go index 1189adab..5e835971 100644 --- a/evaluator.go +++ b/evaluator.go @@ -113,8 +113,14 @@ func Evaluate(ctx context.Context, tape string, out io.Writer, opts ...Evaluator } // Begin recording frames as we are now in a recording state. - ctx, cancel := context.WithCancel(ctx) - ch := v.Record(ctx) + // + // This context belongs to the recorder alone. teardown cancels it to stop + // the frame capture, and encoding runs after teardown, so the encoders must + // not inherit it: makeMedia builds its ffmpeg commands with + // exec.CommandContext, and a command started on a cancelled context is + // killed at Start before it writes anything. + recordCtx, cancel := context.WithCancel(ctx) + ch := v.Record(recordCtx) // Clean up temporary files at the end. defer func() { @@ -141,9 +147,9 @@ func Evaluate(ctx context.Context, tape string, out io.Writer, opts ...Evaluator }() for _, cmd := range cmds[offset:] { - if ctx.Err() != nil { + if recordCtx.Err() != nil { teardown() - return []error{ctx.Err()} + return []error{recordCtx.Err()} } // When changing the FontFamily, FontSize, LineHeight, Padding diff --git a/render_test.go b/render_test.go new file mode 100644 index 00000000..b7fe2f50 --- /dev/null +++ b/render_test.go @@ -0,0 +1,31 @@ +package main + +import ( + "context" + "path/filepath" + "testing" +) + +// TestRenderReportsEncodeFailure locks in two halves of the same regression. +// +// Render builds its encoders with exec.CommandContext, so a command started on +// a context that is already done is killed at Start and writes nothing. Before +// this was fixed, Evaluate handed Render the recording context, which teardown +// had just cancelled, and Render logged the empty output and returned nil. VHS +// printed "Creating out.gif...", exited 0, and produced no file +// (https://github.com/charmbracelet/vhs/issues/787). +// +// A cancelled context stands in here for any failing encode: whatever the +// cause, Render must report it rather than return nil. +func TestRenderReportsEncodeFailure(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + v := New() + v.Options.Video.Output.GIF = filepath.Join(t.TempDir(), "out.gif") + v.totalFrames = 1 + + if err := v.Render(ctx); err == nil { + t.Fatal("Render returned nil for an encode that never ran; encode failures must reach the caller") + } +} diff --git a/vhs.go b/vhs.go index b5b7783f..b4da50f1 100644 --- a/vhs.go +++ b/vhs.go @@ -568,6 +568,7 @@ func (vhs *VHS) Render(ctx context.Context) error { out, err := cmd.CombinedOutput() if err != nil { log.Println(string(out)) + return fmt.Errorf("%s: %w", filepath.Base(cmd.Path), err) } }