fix(headless): finalize the last frame duration in frames.ffconcat
- commit
- a29ced43cacf174f6fef93e251f9ed5165ae5d1c
- author
- brice · · 2026-10-21 04:40 UTC
- parent
- 241d9be
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +96 -3
M internal/adapters/headless/output.go +7 -1
| @@ -100,8 +100,14 @@ func Run(ctx context.Context, opts Options, scenes <-chan ports.Scene, contents | |||
| 100 | 100 | var index *os.File | |
| 101 | 101 | var indexWriter *bufio.Writer | |
| 102 | 102 | var lastShot time.Time | |
| 103 | + | var lastShotName string | |
| 103 | 104 | defer func() { | |
| 104 | 105 | if index != nil { | |
| 106 | + | // A following file makes concat honor the final held interval. | |
| 107 | + | if !lastShot.IsZero() { | |
| 108 | + | _, _ = fmt.Fprintf(indexWriter, "duration %.6f\nfile '%s'\n", time.Since(lastShot).Seconds(), lastShotName) | |
| 109 | + | _ = indexWriter.Flush() | |
| 110 | + | } | |
| 105 | 111 | _ = index.Close() | |
| 106 | 112 | } | |
| 107 | 113 | }() | |
| @@ -598,7 +604,7 @@ func Run(ctx context.Context, opts Options, scenes <-chan ports.Scene, contents | |||
| 598 | 604 | if err := indexWriter.Flush(); err != nil { | |
| 599 | 605 | return fmt.Errorf("frame index: %w", err) | |
| 600 | 606 | } | |
| 601 | - | lastShot = now | |
| 607 | + | lastShot, lastShotName = now, name | |
| 602 | 608 | if opts.RawHDR && opts.HDR { | |
| 603 | 609 | // No cursor on the raw image. Only a confirmed HDR output | |
| 604 | 610 | // has a readable PQ target; else HDRPixels is nil. | |
| @@ -100,8 +100,14 @@ func Run(ctx context.Context, opts Options, scenes <-chan ports.Scene, contents | |||||
| 100 | var index *os.File | 100 | var index *os.File | ||
| 101 | var indexWriter *bufio.Writer | 101 | var indexWriter *bufio.Writer | ||
| 102 | var lastShot time.Time | 102 | var lastShot time.Time | ||
| 103 | + | var lastShotName string | |||
| 103 | defer func() { | 104 | defer func() { | ||
| 104 | if index != nil { | 105 | if index != nil { | ||
| 106 | + | // A following file makes concat honor the final held interval. | |||
| 107 | + | if !lastShot.IsZero() { | |||
| 108 | + | _, _ = fmt.Fprintf(indexWriter, "duration %.6f\nfile '%s'\n", time.Since(lastShot).Seconds(), lastShotName) | |||
| 109 | + | _ = indexWriter.Flush() | |||
| 110 | + | } | |||
| 105 | _ = index.Close() | 111 | _ = index.Close() | ||
| 106 | } | 112 | } | ||
| 107 | }() | 113 | }() | ||
| @@ -598,7 +604,7 @@ func Run(ctx context.Context, opts Options, scenes <-chan ports.Scene, contents | |||||
| 598 | if err := indexWriter.Flush(); err != nil { | 604 | if err := indexWriter.Flush(); err != nil { | ||
| 599 | return fmt.Errorf("frame index: %w", err) | 605 | return fmt.Errorf("frame index: %w", err) | ||
| 600 | } | 606 | } | ||
| 601 | - | lastShot = now | 607 | + | lastShot, lastShotName = now, name |
| 602 | if opts.RawHDR && opts.HDR { | 608 | if opts.RawHDR && opts.HDR { | ||
| 603 | // No cursor on the raw image. Only a confirmed HDR output | 609 | // No cursor on the raw image. Only a confirmed HDR output | ||
| 604 | // has a readable PQ target; else HDRPixels is nil. | 610 | // has a readable PQ target; else HDRPixels is nil. | ||
M internal/adapters/headless/output_test.go +48 -2
| @@ -125,14 +125,14 @@ func TestRun(t *testing.T) { | |||
| 125 | 125 | t.Fatal(err) | |
| 126 | 126 | } | |
| 127 | 127 | lines := strings.Split(strings.TrimSpace(string(index)), "\n") | |
| 128 | - | if len(lines) != 6 || lines[0] != "ffconcat version 1.0" { | |
| 128 | + | if len(lines) != 8 || lines[0] != "ffconcat version 1.0" { | |
| 129 | 129 | t.Fatalf("unexpected index: %s", index) | |
| 130 | 130 | } | |
| 131 | 131 | for i, want := range []string{"file 'frame-000001.png'", "file 'frame-000002.png'", "file 'frame-000003.png'"} { | |
| 132 | 132 | if lines[1+2*i] != want { | |
| 133 | 133 | t.Fatalf("frame %d: %s", i+1, lines[1+2*i]) | |
| 134 | 134 | } | |
| 135 | - | if i < 2 { | |
| 135 | + | if i < 3 { | |
| 136 | 136 | line := lines[2+2*i] | |
| 137 | 137 | value, err := strconv.ParseFloat(strings.TrimPrefix(line, "duration "), 64) | |
| 138 | 138 | if !strings.HasPrefix(line, "duration ") || err != nil || value < 0 || math.IsNaN(value) || math.IsInf(value, 0) { | |
| @@ -140,6 +140,9 @@ func TestRun(t *testing.T) { | |||
| 140 | 140 | } | |
| 141 | 141 | } | |
| 142 | 142 | } | |
| 143 | + | if lines[7] != lines[5] { | |
| 144 | + | t.Fatalf("missing final repeated frame: %s", index) | |
| 145 | + | } | |
| 143 | 146 | } | |
| 144 | 147 | ||
| 145 | 148 | // A scene whose Seq the output already holds is not drawn again. | |
| @@ -530,3 +533,46 @@ func TestRunAppliesWholeBatch(t *testing.T) { | |||
| 530 | 533 | t.Error("batch returned more than once") | |
| 531 | 534 | } | |
| 532 | 535 | } | |
| 536 | + | ||
| 537 | + | func TestRunFinalizesLastFrameDuration(t *testing.T) { | |
| 538 | + | dir := t.TempDir() | |
| 539 | + | scenes := make(chan ports.Scene, 1) | |
| 540 | + | scenes <- ports.Scene{Seq: 1} | |
| 541 | + | r, _ := recordingRenderer(t, nil) | |
| 542 | + | ctx, cancel := context.WithCancel(context.Background()) | |
| 543 | + | defer cancel() | |
| 544 | + | done := make(chan error, 1) | |
| 545 | + | go func() { | |
| 546 | + | done <- Run(ctx, Options{Width: 2, Height: 2, ScreenshotDir: dir, NewRenderer: func(int, int) (ports.Renderer, error) { return r, nil }}, scenes, nil, nil, nil, nil) | |
| 547 | + | }() | |
| 548 | + | deadline := time.Now().Add(3 * time.Second) | |
| 549 | + | for { | |
| 550 | + | data, _ := os.ReadFile(filepath.Join(dir, "frames.ffconcat")) | |
| 551 | + | if strings.Contains(string(data), "file 'frame-000001.png'") { | |
| 552 | + | break | |
| 553 | + | } | |
| 554 | + | if time.Now().After(deadline) { | |
| 555 | + | cancel() | |
| 556 | + | <-done | |
| 557 | + | t.Fatal("frame index not written") | |
| 558 | + | } | |
| 559 | + | time.Sleep(time.Millisecond) | |
| 560 | + | } | |
| 561 | + | time.Sleep(50 * time.Millisecond) | |
| 562 | + | cancel() | |
| 563 | + | if err := <-done; err != nil { | |
| 564 | + | t.Fatal(err) | |
| 565 | + | } | |
| 566 | + | data, err := os.ReadFile(filepath.Join(dir, "frames.ffconcat")) | |
| 567 | + | if err != nil { | |
| 568 | + | t.Fatal(err) | |
| 569 | + | } | |
| 570 | + | lines := strings.Split(strings.TrimSpace(string(data)), "\n") | |
| 571 | + | if len(lines) != 4 || lines[0] != "ffconcat version 1.0" || lines[1] != "file 'frame-000001.png'" || lines[3] != lines[1] { | |
| 572 | + | t.Fatalf("unexpected finalized index: %s", data) | |
| 573 | + | } | |
| 574 | + | duration, err := strconv.ParseFloat(strings.TrimPrefix(lines[2], "duration "), 64) | |
| 575 | + | if err != nil || duration < 0.05 || math.IsNaN(duration) || math.IsInf(duration, 0) { | |
| 576 | + | t.Fatalf("final duration: %q (%v)", lines[2], err) | |
| 577 | + | } | |
| 578 | + | } | |
| @@ -125,14 +125,14 @@ func TestRun(t *testing.T) { | |||||
| 125 | t.Fatal(err) | 125 | t.Fatal(err) | ||
| 126 | } | 126 | } | ||
| 127 | lines := strings.Split(strings.TrimSpace(string(index)), "\n") | 127 | lines := strings.Split(strings.TrimSpace(string(index)), "\n") | ||
| 128 | - | if len(lines) != 6 || lines[0] != "ffconcat version 1.0" { | 128 | + | if len(lines) != 8 || lines[0] != "ffconcat version 1.0" { |
| 129 | t.Fatalf("unexpected index: %s", index) | 129 | t.Fatalf("unexpected index: %s", index) | ||
| 130 | } | 130 | } | ||
| 131 | for i, want := range []string{"file 'frame-000001.png'", "file 'frame-000002.png'", "file 'frame-000003.png'"} { | 131 | for i, want := range []string{"file 'frame-000001.png'", "file 'frame-000002.png'", "file 'frame-000003.png'"} { | ||
| 132 | if lines[1+2*i] != want { | 132 | if lines[1+2*i] != want { | ||
| 133 | t.Fatalf("frame %d: %s", i+1, lines[1+2*i]) | 133 | t.Fatalf("frame %d: %s", i+1, lines[1+2*i]) | ||
| 134 | } | 134 | } | ||
| 135 | - | if i < 2 { | 135 | + | if i < 3 { |
| 136 | line := lines[2+2*i] | 136 | line := lines[2+2*i] | ||
| 137 | value, err := strconv.ParseFloat(strings.TrimPrefix(line, "duration "), 64) | 137 | value, err := strconv.ParseFloat(strings.TrimPrefix(line, "duration "), 64) | ||
| 138 | if !strings.HasPrefix(line, "duration ") || err != nil || value < 0 || math.IsNaN(value) || math.IsInf(value, 0) { | 138 | if !strings.HasPrefix(line, "duration ") || err != nil || value < 0 || math.IsNaN(value) || math.IsInf(value, 0) { | ||
| @@ -140,6 +140,9 @@ func TestRun(t *testing.T) { | |||||
| 140 | } | 140 | } | ||
| 141 | } | 141 | } | ||
| 142 | } | 142 | } | ||
| 143 | + | if lines[7] != lines[5] { | |||
| 144 | + | t.Fatalf("missing final repeated frame: %s", index) | |||
| 145 | + | } | |||
| 143 | } | 146 | } | ||
| 144 | 147 | ||||
| 145 | // A scene whose Seq the output already holds is not drawn again. | 148 | // A scene whose Seq the output already holds is not drawn again. | ||
| @@ -530,3 +533,46 @@ func TestRunAppliesWholeBatch(t *testing.T) { | |||||
| 530 | t.Error("batch returned more than once") | 533 | t.Error("batch returned more than once") | ||
| 531 | } | 534 | } | ||
| 532 | } | 535 | } | ||
| 536 | + | ||||
| 537 | + | func TestRunFinalizesLastFrameDuration(t *testing.T) { | |||
| 538 | + | dir := t.TempDir() | |||
| 539 | + | scenes := make(chan ports.Scene, 1) | |||
| 540 | + | scenes <- ports.Scene{Seq: 1} | |||
| 541 | + | r, _ := recordingRenderer(t, nil) | |||
| 542 | + | ctx, cancel := context.WithCancel(context.Background()) | |||
| 543 | + | defer cancel() | |||
| 544 | + | done := make(chan error, 1) | |||
| 545 | + | go func() { | |||
| 546 | + | done <- Run(ctx, Options{Width: 2, Height: 2, ScreenshotDir: dir, NewRenderer: func(int, int) (ports.Renderer, error) { return r, nil }}, scenes, nil, nil, nil, nil) | |||
| 547 | + | }() | |||
| 548 | + | deadline := time.Now().Add(3 * time.Second) | |||
| 549 | + | for { | |||
| 550 | + | data, _ := os.ReadFile(filepath.Join(dir, "frames.ffconcat")) | |||
| 551 | + | if strings.Contains(string(data), "file 'frame-000001.png'") { | |||
| 552 | + | break | |||
| 553 | + | } | |||
| 554 | + | if time.Now().After(deadline) { | |||
| 555 | + | cancel() | |||
| 556 | + | <-done | |||
| 557 | + | t.Fatal("frame index not written") | |||
| 558 | + | } | |||
| 559 | + | time.Sleep(time.Millisecond) | |||
| 560 | + | } | |||
| 561 | + | time.Sleep(50 * time.Millisecond) | |||
| 562 | + | cancel() | |||
| 563 | + | if err := <-done; err != nil { | |||
| 564 | + | t.Fatal(err) | |||
| 565 | + | } | |||
| 566 | + | data, err := os.ReadFile(filepath.Join(dir, "frames.ffconcat")) | |||
| 567 | + | if err != nil { | |||
| 568 | + | t.Fatal(err) | |||
| 569 | + | } | |||
| 570 | + | lines := strings.Split(strings.TrimSpace(string(data)), "\n") | |||
| 571 | + | if len(lines) != 4 || lines[0] != "ffconcat version 1.0" || lines[1] != "file 'frame-000001.png'" || lines[3] != lines[1] { | |||
| 572 | + | t.Fatalf("unexpected finalized index: %s", data) | |||
| 573 | + | } | |||
| 574 | + | duration, err := strconv.ParseFloat(strings.TrimPrefix(lines[2], "duration "), 64) | |||
| 575 | + | if err != nil || duration < 0.05 || math.IsNaN(duration) || math.IsInf(duration, 0) { | |||
| 576 | + | t.Fatalf("final duration: %q (%v)", lines[2], err) | |||
| 577 | + | } | |||
| 578 | + | } | |||
M internal/adapters/headless/security_test.go +41 -0
| @@ -464,3 +464,44 @@ func TestSecurityCompletionLeaseRecycledBeforeProtectionRenderFailure(t *testing | |||
| 464 | 464 | // don't let testify format that expired borrowed memory at cleanup. | |
| 465 | 465 | frame.Calls = nil | |
| 466 | 466 | } | |
| 467 | + | ||
| 468 | + | func TestSecuritySkippedPNGHasNoFrameIndex(t *testing.T) { | |
| 469 | + | dir := t.TempDir() | |
| 470 | + | var state atomic.Value | |
| 471 | + | state.Store(ports.SecurityState{}) | |
| 472 | + | protected := ports.SecurityState{Generation: 1, Protected: true} | |
| 473 | + | gate := portsmocks.NewMockSessionSecurity(t) | |
| 474 | + | gate.EXPECT().Snapshot().RunAndReturn(func() ports.SecurityState { | |
| 475 | + | return state.Load().(ports.SecurityState) | |
| 476 | + | }) | |
| 477 | + | r := portsmocks.NewMockRenderer(t) | |
| 478 | + | black := make(chan struct{}, 1) | |
| 479 | + | r.EXPECT().Render(mock.Anything, mock.Anything).RunAndReturn(func(s ports.Scene, _ map[ports.WindowID]ports.SurfaceContent) (*os.File, error) { | |
| 480 | + | if s.Security.Protected { | |
| 481 | + | black <- struct{}{} | |
| 482 | + | } | |
| 483 | + | return nil, nil | |
| 484 | + | }).Twice() | |
| 485 | + | r.EXPECT().Pixels().RunAndReturn(func() *image.RGBA { | |
| 486 | + | state.Store(protected) | |
| 487 | + | return image.NewRGBA(image.Rect(0, 0, 2, 2)) | |
| 488 | + | }).Once() | |
| 489 | + | r.EXPECT().Close().Return().Once() | |
| 490 | + | scenes := make(chan ports.Scene, 1) | |
| 491 | + | scenes <- ports.Scene{Seq: 1} | |
| 492 | + | ctx, cancel := context.WithCancel(context.Background()) | |
| 493 | + | defer cancel() | |
| 494 | + | done := make(chan error, 1) | |
| 495 | + | go func() { | |
| 496 | + | done <- Run(ctx, Options{Security: gate, Width: 2, Height: 2, ScreenshotDir: dir, NewRenderer: func(int, int) (ports.Renderer, error) { return r, nil }}, scenes, nil, nil, nil, nil) | |
| 497 | + | }() | |
| 498 | + | securityReceive(t, black) | |
| 499 | + | cancel() | |
| 500 | + | if err := securityReceive(t, done); err != nil { | |
| 501 | + | t.Fatal(err) | |
| 502 | + | } | |
| 503 | + | files, err := os.ReadDir(dir) | |
| 504 | + | if err != nil || len(files) != 0 { | |
| 505 | + | t.Fatalf("security-skipped frame produced PNG or index: %v (%v)", files, err) | |
| 506 | + | } | |
| 507 | + | } | |
| @@ -464,3 +464,44 @@ func TestSecurityCompletionLeaseRecycledBeforeProtectionRenderFailure(t *testing | |||||
| 464 | // don't let testify format that expired borrowed memory at cleanup. | 464 | // don't let testify format that expired borrowed memory at cleanup. | ||
| 465 | frame.Calls = nil | 465 | frame.Calls = nil | ||
| 466 | } | 466 | } | ||
| 467 | + | ||||
| 468 | + | func TestSecuritySkippedPNGHasNoFrameIndex(t *testing.T) { | |||
| 469 | + | dir := t.TempDir() | |||
| 470 | + | var state atomic.Value | |||
| 471 | + | state.Store(ports.SecurityState{}) | |||
| 472 | + | protected := ports.SecurityState{Generation: 1, Protected: true} | |||
| 473 | + | gate := portsmocks.NewMockSessionSecurity(t) | |||
| 474 | + | gate.EXPECT().Snapshot().RunAndReturn(func() ports.SecurityState { | |||
| 475 | + | return state.Load().(ports.SecurityState) | |||
| 476 | + | }) | |||
| 477 | + | r := portsmocks.NewMockRenderer(t) | |||
| 478 | + | black := make(chan struct{}, 1) | |||
| 479 | + | r.EXPECT().Render(mock.Anything, mock.Anything).RunAndReturn(func(s ports.Scene, _ map[ports.WindowID]ports.SurfaceContent) (*os.File, error) { | |||
| 480 | + | if s.Security.Protected { | |||
| 481 | + | black <- struct{}{} | |||
| 482 | + | } | |||
| 483 | + | return nil, nil | |||
| 484 | + | }).Twice() | |||
| 485 | + | r.EXPECT().Pixels().RunAndReturn(func() *image.RGBA { | |||
| 486 | + | state.Store(protected) | |||
| 487 | + | return image.NewRGBA(image.Rect(0, 0, 2, 2)) | |||
| 488 | + | }).Once() | |||
| 489 | + | r.EXPECT().Close().Return().Once() | |||
| 490 | + | scenes := make(chan ports.Scene, 1) | |||
| 491 | + | scenes <- ports.Scene{Seq: 1} | |||
| 492 | + | ctx, cancel := context.WithCancel(context.Background()) | |||
| 493 | + | defer cancel() | |||
| 494 | + | done := make(chan error, 1) | |||
| 495 | + | go func() { | |||
| 496 | + | done <- Run(ctx, Options{Security: gate, Width: 2, Height: 2, ScreenshotDir: dir, NewRenderer: func(int, int) (ports.Renderer, error) { return r, nil }}, scenes, nil, nil, nil, nil) | |||
| 497 | + | }() | |||
| 498 | + | securityReceive(t, black) | |||
| 499 | + | cancel() | |||
| 500 | + | if err := securityReceive(t, done); err != nil { | |||
| 501 | + | t.Fatal(err) | |||
| 502 | + | } | |||
| 503 | + | files, err := os.ReadDir(dir) | |||
| 504 | + | if err != nil || len(files) != 0 { | |||
| 505 | + | t.Fatalf("security-skipped frame produced PNG or index: %v (%v)", files, err) | |||
| 506 | + | } | |||
| 507 | + | } | |||