diff --git a/provider/llamaswap/lipsync.go b/provider/llamaswap/lipsync.go index 544be0b..279aa3b 100644 --- a/provider/llamaswap/lipsync.go +++ b/provider/llamaswap/lipsync.go @@ -22,7 +22,7 @@ import ( // LipsyncModel implements videogen.LipsyncProvider. The id selects which // upstream llama-swap loads. -func (p *Provider) LipsyncModel(id string, opts ...videogen.LipsyncModelOption) (videogen.LipSyncer, error) { +func (p *Provider) LipsyncModel(id string, opts ...videogen.LipsyncModelOption) (videogen.Lipsyncer, error) { if err := p.requireBaseURL(); err != nil { return nil, err } @@ -35,7 +35,7 @@ type lipsyncModel struct { id string } -// Lipsync implements videogen.LipSyncer. +// Lipsync implements videogen.Lipsyncer. func (m *lipsyncModel) Lipsync(ctx context.Context, req videogen.LipsyncRequest, opts ...videogen.LipsyncOption) (*videogen.Result, error) { req = req.Apply(opts...) if len(req.Image.Data) == 0 { @@ -95,19 +95,3 @@ func (m *lipsyncModel) Lipsync(ctx context.Context, req videogen.LipsyncRequest, } return singleVideoResult(m.p.name, m.id, "lipsync", raw, respType) } - -// singleVideoResult wraps one raw video body into a videogen.Result, -// requiring positive evidence of video-ness (declared video/* Content-Type -// or sniffed mp4/webm magic) so a proxy error page can never become "the -// clip" — the video sibling of singleImageResult. -func singleVideoResult(provider, model, verb string, raw []byte, contentType string) (*videogen.Result, error) { - if len(raw) == 0 { - return nil, &llm.APIError{Provider: provider, Model: model, Message: verb + " response contained no video"} - } - mimeType := videoMIME(contentType, raw) - if mimeType == "" { - return nil, &llm.APIError{Provider: provider, Model: model, - Message: fmt.Sprintf("%s response is not a video (Content-Type %q)", verb, contentType)} - } - return &videogen.Result{Video: videogen.Video{Data: raw, MIME: mimeType}}, nil -} diff --git a/provider/llamaswap/mediautil.go b/provider/llamaswap/mediautil.go index 334ed49..e2bb5b7 100644 --- a/provider/llamaswap/mediautil.go +++ b/provider/llamaswap/mediautil.go @@ -207,12 +207,5 @@ func (m *interpolatorModel) Interpolate(ctx context.Context, req videogen.Interp if err != nil { return nil, err } - if len(raw) == 0 { - return nil, &llm.APIError{Provider: m.p.name, Model: m.id, Message: "interpolate response contained no video"} - } - mimeType := videoMIME(respType, raw) - if mimeType == "" { - return nil, &llm.APIError{Provider: m.p.name, Model: m.id, Message: "interpolate response is not a video"} - } - return &videogen.Result{Video: videogen.Video{Data: raw, MIME: mimeType}}, nil + return singleVideoResult(m.p.name, m.id, "interpolate", raw, respType) } diff --git a/provider/llamaswap/video.go b/provider/llamaswap/video.go index caa584e..1586a6e 100644 --- a/provider/llamaswap/video.go +++ b/provider/llamaswap/video.go @@ -100,22 +100,7 @@ func (m *videoModel) Generate(ctx context.Context, req videogen.Request, opts .. if err != nil { return nil, err } - if len(videoBytes) == 0 { - return nil, &llm.APIError{Provider: m.p.name, Model: m.id, Message: "video response contained no video"} - } - mimeType := videoMIME(contentType, videoBytes) - if mimeType == "" { - // A 2xx body that is neither declared nor sniffable as video is a - // misconfigured upstream (a JSON job envelope, an HTML error page - // behind a proxy) — fail loud rather than hand back garbage as a - // playable clip. - return nil, &llm.APIError{ - Provider: m.p.name, - Model: m.id, - Message: fmt.Sprintf("video response is not a video (content-type %q)", contentType), - } - } - return &videogen.Result{Video: videogen.Video{Data: videoBytes, MIME: mimeType}}, nil + return singleVideoResult(m.p.name, m.id, "video", videoBytes, contentType) } // videoMIME resolves the result MIME type: the response Content-Type when it @@ -132,6 +117,24 @@ func videoMIME(contentType string, data []byte) string { return "" } +// singleVideoResult wraps one raw video body into a videogen.Result, +// requiring positive evidence of video-ness (declared video/* Content-Type +// or sniffed mp4/webm magic) so a 2xx body that is anything else — a JSON +// job envelope, an HTML error page behind a proxy — fails loud instead of +// coming back as "the clip". The video sibling of singleImageResult, shared +// by every surface whose response body IS the encoded clip. +func singleVideoResult(provider, model, verb string, raw []byte, contentType string) (*videogen.Result, error) { + if len(raw) == 0 { + return nil, &llm.APIError{Provider: provider, Model: model, Message: verb + " response contained no video"} + } + mimeType := videoMIME(contentType, raw) + if mimeType == "" { + return nil, &llm.APIError{Provider: provider, Model: model, + Message: fmt.Sprintf("%s response is not a video (Content-Type %q)", verb, contentType)} + } + return &videogen.Result{Video: videogen.Video{Data: raw, MIME: mimeType}}, nil +} + // initImageFilename picks the multipart filename hint for the conditioning // frame from its MIME subtype. The name is provider-chosen (never // caller-supplied), so no sanitization is needed. diff --git a/provider/llamaswap/videochain.go b/provider/llamaswap/videochain.go index 1e70e60..882c8ae 100644 --- a/provider/llamaswap/videochain.go +++ b/provider/llamaswap/videochain.go @@ -19,6 +19,7 @@ import ( "encoding/base64" "encoding/json" "fmt" + "math" "net/http" "strconv" "strings" @@ -71,8 +72,10 @@ func (m *chainerModel) SubmitChain(ctx context.Context, req videogen.ChainReques if strings.TrimSpace(seg.Prompt) == "" { return "", fmt.Errorf("%w: video chain segment %d requires a prompt", llm.ErrUnsupported, i) } - if seg.Seconds < 0 { - return "", fmt.Errorf("%w: video chain segment %d seconds must be >= 0, got %g", llm.ErrUnsupported, i, seg.Seconds) + // NaN/±Inf would otherwise surface as an obscure json.Marshal error + // (NaN fails every comparison; +Inf passes the >= 0 check). + if seg.Seconds < 0 || math.IsNaN(seg.Seconds) || math.IsInf(seg.Seconds, 0) { + return "", fmt.Errorf("%w: video chain segment %d seconds must be a finite value >= 0, got %g", llm.ErrUnsupported, i, seg.Seconds) } wire.Segments = append(wire.Segments, chainSegmentWire{Prompt: seg.Prompt, Seconds: seg.Seconds}) } @@ -133,10 +136,17 @@ func (m *chainerModel) ChainStatus(ctx context.Context, jobID string) (*videogen Total: out.Total, Raw: raw, } + // Entries that carry no usable id — JSON null (which unmarshals into a + // string as a no-op, leaving ""), an empty string, or an object with + // neither key — are SKIPPED, never appended as "": SegmentIDs promises + // fetchable artifacts, and the full payload stays in Raw for callers + // that want the unfiltered list. for _, entry := range out.Segments { var s string if json.Unmarshal(entry, &s) == nil { - job.SegmentIDs = append(job.SegmentIDs, s) + if s != "" { + job.SegmentIDs = append(job.SegmentIDs, s) + } continue } var obj struct { @@ -197,7 +207,10 @@ func (m *chainerModel) jobPath(jobID, suffix string) (string, error) { if strings.TrimSpace(jobID) == "" { return "", fmt.Errorf("llama-swap: chain job call requires a job id") } - if strings.ContainsAny(jobID, "/?#") || strings.Contains(jobID, "..") { + // '%' is rejected alongside the literal path characters: job ids never + // legitimately carry percent-escapes, and %2F/%2E%2E would decode back + // into path structure server-side. + if strings.ContainsAny(jobID, "/?#%") || strings.Contains(jobID, "..") { return "", fmt.Errorf("llama-swap: invalid chain job id %q (contains path structure)", jobID) } return upstreamPath(m.id, "/v1/jobs/"+jobID+suffix) diff --git a/provider/llamaswap/videochain_test.go b/provider/llamaswap/videochain_test.go index d157bb6..85f9c90 100644 --- a/provider/llamaswap/videochain_test.go +++ b/provider/llamaswap/videochain_test.go @@ -5,6 +5,7 @@ import ( "encoding/base64" "encoding/json" "errors" + "math" "net/http" "net/http/httptest" "reflect" @@ -111,6 +112,17 @@ func TestSubmitChainRejectsBadArgs(t *testing.T) { }); !errors.Is(err, llm.ErrUnsupported) { t.Errorf("negative seconds: err = %v, want ErrUnsupported", err) } + for name, bad := range map[string]float64{ + "NaN": math.NaN(), + "+Inf": math.Inf(1), + "-Inf": math.Inf(-1), + } { + if _, err := ch.SubmitChain(context.Background(), videogen.ChainRequest{ + Segments: []videogen.ChainSegment{{Prompt: "x", Seconds: bad}}, + }); !errors.Is(err, llm.ErrUnsupported) { + t.Errorf("%s seconds: err = %v, want ErrUnsupported", name, err) + } + } } func TestSubmitChainRejectsMissingJobID(t *testing.T) { @@ -175,6 +187,27 @@ func TestChainStatusToleratesObjectSegments(t *testing.T) { } } +func TestChainStatusSkipsIDLessSegments(t *testing.T) { + // null, "", an id-less object, and a mistyped entry must all be + // skipped — never appended as "" (SegmentIDs promises fetchable + // artifacts; the unfiltered list stays in Raw). + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`{"status":"running","segment":3,"total":5,` + + `"segments":[null,"seg-0","",{},{"id":"seg-1"},{"segment_id":""},42]}`)) + })) + defer srv.Close() + + p := New(WithBaseURL(srv.URL), WithHTTPClient(srv.Client())) + ch, _ := p.ChainerModel("videoutils") + job, err := ch.ChainStatus(context.Background(), "job-123") + if err != nil { + t.Fatalf("ChainStatus: %v", err) + } + if !reflect.DeepEqual(job.SegmentIDs, []string{"seg-0", "seg-1"}) { + t.Errorf("SegmentIDs = %v, want [seg-0 seg-1]", job.SegmentIDs) + } +} + func TestChainStatusRejectsStatuslessPayload(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { _, _ = w.Write([]byte(`{"detail":"no such job"}`)) @@ -193,7 +226,7 @@ func TestChainStatusRejectsStatuslessPayload(t *testing.T) { func TestChainJobPathRejectsHostileIDs(t *testing.T) { p := New(WithBaseURL("http://unused")) ch, _ := p.ChainerModel("videoutils") - for _, bad := range []string{"", "a/b", "a?b", "a#b", "..", "a..b"} { + for _, bad := range []string{"", "a/b", "a?b", "a#b", "..", "a..b", "a%2Fb", "%2e%2e", "a%b"} { if _, err := ch.ChainStatus(context.Background(), bad); err == nil { t.Errorf("ChainStatus(%q) succeeded; want error", bad) } diff --git a/videogen/chain.go b/videogen/chain.go index 5903383..d349b41 100644 --- a/videogen/chain.go +++ b/videogen/chain.go @@ -42,9 +42,13 @@ type ChainJob struct { Segment int Total int - // SegmentIDs are the COMPLETED per-segment artifacts, fetchable via - // ChainSegmentResult — a mid-chain failure still leaves these - // retrievable, so multi-minute GPU output is never discarded. + // SegmentIDs name the COMPLETED per-segment artifacts, in order. A + // mid-chain failure still leaves those segments retrievable via + // ChainSegmentResult — note it takes the segment's index in the chain, + // not an id string; this list tells you WHICH segments completed. So + // multi-minute GPU output is never discarded. Entries the backend + // reports without a usable id are skipped (the unfiltered list survives + // in Raw). SegmentIDs []string // Raw is the provider-native job payload. May be nil. diff --git a/videogen/lipsync.go b/videogen/lipsync.go index 2548b9d..197d31b 100644 --- a/videogen/lipsync.go +++ b/videogen/lipsync.go @@ -57,16 +57,16 @@ func (r LipsyncRequest) Apply(opts ...LipsyncOption) LipsyncRequest { return r } -// LipSyncer animates still portraits into talking-head clips. Its own small +// Lipsyncer animates still portraits into talking-head clips. Its own small // interface rather than a method on Model: lip-syncers are not text-to-video // generators — they bind to a different backend id entirely. -type LipSyncer interface { +type Lipsyncer interface { // Lipsync returns the talking-head clip. Generation is slow (minutes); // bound the call with a context deadline. Lipsync(ctx context.Context, req LipsyncRequest, opts ...LipsyncOption) (*Result, error) } -// LipsyncModelOption configures a LipSyncer at construction time. Reserved +// LipsyncModelOption configures a Lipsyncer at construction time. Reserved // for future per-model settings. type LipsyncModelOption func(*LipsyncModelConfig) @@ -82,12 +82,12 @@ func ApplyLipsyncModelOptions(opts []LipsyncModelOption) LipsyncModelConfig { return cfg } -// LipsyncProvider mints LipSyncers bound to one backend. +// LipsyncProvider mints Lipsyncers bound to one backend. type LipsyncProvider interface { // Name is the registry identifier for the provider. Name() string - // LipsyncModel returns a LipSyncer bound to the given id (passed through + // LipsyncModel returns a Lipsyncer bound to the given id (passed through // to the backend verbatim; no catalog validation). - LipsyncModel(id string, opts ...LipsyncModelOption) (LipSyncer, error) + LipsyncModel(id string, opts ...LipsyncModelOption) (Lipsyncer, error) }