From dbc96898abaf12e164cc0b9173f6cb0538db7320 Mon Sep 17 00:00:00 2001 From: Steve Dudenhoeffer Date: Sat, 8 Aug 2026 02:50:43 -0400 Subject: [PATCH] fix(videogen): distinct FILENAMES for the two keyframes, not just distinct field names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Caught while writing the receiving end. Distinct multipart field names are not sufficient: backends stage an uploaded frame under a name derived from the FILENAME, and our own ComfyUI shim posts to /upload/image with overwrite=true. Both parts were sending initImageFilename(mime) — literally "frame.png" for each — so the second upload would have clobbered the first and BOTH keyframe inputs would have resolved to the same stored image. The failure mode is the worst kind: a clip pinned at both ends to the same frame renders cleanly, returns 200, and looks like the feature not working rather than like a bug. Nothing upstream or downstream would report a fault. writeImagePart now takes the filename stem (frame / frame_last), and the test asserts the two arrive under different filenames. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01PLjgrxvHjm1sJgUu9zBPH9 --- provider/llamaswap/video.go | 23 ++++++++++++++++------- provider/llamaswap/video_test.go | 19 +++++++++++++++++-- 2 files changed, 33 insertions(+), 9 deletions(-) diff --git a/provider/llamaswap/video.go b/provider/llamaswap/video.go index d61ae34..d9e3308 100644 --- a/provider/llamaswap/video.go +++ b/provider/llamaswap/video.go @@ -86,7 +86,7 @@ func (m *videoModel) Generate(ctx context.Context, req videogen.Request, opts .. return nil, err } if req.InitImage != nil { - if err := writeImagePart(w, "input_reference", req.InitImage); err != nil { + if err := writeImagePart(w, "input_reference", "frame", req.InitImage); err != nil { return nil, err } } @@ -97,7 +97,7 @@ func (m *videoModel) Generate(ctx context.Context, req videogen.Request, opts .. // can see they have broken. A backend that does not know the name ignores // the part, which is the same degradation as any other unknown field. if req.LastImage != nil { - if err := writeImagePart(w, "input_reference_last", req.LastImage); err != nil { + if err := writeImagePart(w, "input_reference_last", "frame_last", req.LastImage); err != nil { return nil, err } } @@ -151,11 +151,20 @@ func initImageFilename(mimeType string) string { return imageFilename(mimeType, "frame") } -// writeImagePart attaches one conditioning frame under the given field name. -// Shared by the first- and last-frame parts so the two cannot drift in how -// they encode, which is the usual way a second copy of a block goes wrong. -func writeImagePart(w *multipart.Writer, field string, img *videogen.Image) error { - fw, err := w.CreateFormFile(field, initImageFilename(img.MIME)) +// writeImagePart attaches one conditioning frame under the given field name, +// with a filename derived from nameStem. Shared by the first- and last-frame +// parts so the two cannot drift in how they encode, which is the usual way a +// second copy of a block goes wrong. +// +// The two frames MUST carry DISTINCT filenames, not merely distinct field +// names. Backends commonly stage an uploaded frame under a name derived from +// the filename — our own ComfyUI shim posts to /upload/image with +// overwrite=true — so two parts sharing "frame.png" would have the second +// clobber the first, and BOTH keyframe inputs would then resolve to the same +// stored image. The clip would render clean, pinned at both ends to the same +// frame, with nothing anywhere reporting a problem. +func writeImagePart(w *multipart.Writer, field, nameStem string, img *videogen.Image) error { + fw, err := w.CreateFormFile(field, imageFilename(img.MIME, nameStem)) if err != nil { return fmt.Errorf("llama-swap: build video form: %w", err) } diff --git a/provider/llamaswap/video_test.go b/provider/llamaswap/video_test.go index 7ebb264..81a6ae5 100644 --- a/provider/llamaswap/video_test.go +++ b/provider/llamaswap/video_test.go @@ -233,19 +233,22 @@ func TestVideoGenerateNonVideoBodyErrors(t *testing.T) { // breaking. Asserting the names is what pins it. func TestVideoGenerateSendsBothKeyframes(t *testing.T) { var gotFirst, gotLast []byte + var firstName, lastName string var sawLastPart bool srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if err := r.ParseMultipartForm(32 << 20); err != nil { t.Errorf("parse form: %v", err) return } - if f, _, err := r.FormFile("input_reference"); err == nil { + if f, hdr, err := r.FormFile("input_reference"); err == nil { gotFirst, _ = io.ReadAll(f) + firstName = hdr.Filename f.Close() } - if f, _, err := r.FormFile("input_reference_last"); err == nil { + if f, hdr, err := r.FormFile("input_reference_last"); err == nil { sawLastPart = true gotLast, _ = io.ReadAll(f) + lastName = hdr.Filename f.Close() } w.Header().Set("Content-Type", "video/mp4") @@ -283,6 +286,18 @@ func TestVideoGenerateSendsBothKeyframes(t *testing.T) { if string(gotFirst) == string(gotLast) { t.Error("both parts carry identical bytes — the frames are being aliased") } + // DISTINCT FILENAMES, not just distinct field names. Backends stage an + // uploaded frame under a name derived from the filename (our ComfyUI shim + // posts to /upload/image with overwrite=true), so two parts sharing + // "frame.png" would have the second clobber the first and BOTH keyframes + // would resolve to the same stored image — a clip pinned at both ends to + // the same frame, rendering cleanly with nothing reporting a fault. + if firstName == "" || lastName == "" { + t.Fatalf("filenames = %q / %q, want both set", firstName, lastName) + } + if firstName == lastName { + t.Errorf("both parts use filename %q — the second upload would clobber the first", firstName) + } } // LastImage alone (no InitImage) is a legitimate request: pin the destination