fix(media): reject odd sources, use private modes and bound encoder threads
- commit
- ca02ed9013f84d2fc7589baa48529f8145e1b9f4
- author
- brice · · 2026-09-30 08:12 UTC
- committer
- brice · · 2026-09-30 14:48 UTC
- parent
- 4d62f01
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +79 -25
M internal/adapters/ffmpeg/ffmpeg.go +17 -17
| @@ -107,35 +107,35 @@ func NewWithBinary(binary string) *Writer { | |||
| 107 | 107 | } | |
| 108 | 108 | ||
| 109 | 109 | // outputSize validates settings and returns the encoded dimensions. Zero | |
| 110 | - | // dimensions retain the source size rounded down to even values (yuv420p). | |
| 110 | + | // dimensions retain the source resolution exactly, which yuv420p only allows | |
| 111 | + | // for even sizes; odd sources are rejected rather than cropped. Explicit | |
| 112 | + | // sizes must be even and no larger than a captured frame may be. | |
| 111 | 113 | func outputSize(f ports.Frame, s ports.VideoSettings) (int, int, error) { | |
| 112 | 114 | if s.FPS < 1 || s.FPS > ports.MaxFPS { | |
| 113 | 115 | return 0, 0, fmt.Errorf("ffmpeg: fps %d outside 1..%d", s.FPS, ports.MaxFPS) | |
| 114 | 116 | } | |
| 115 | 117 | if s.Width == 0 && s.Height == 0 { | |
| 116 | - | w, h := f.Width&^1, f.Height&^1 | |
| 117 | - | if w < 2 || h < 2 { | |
| 118 | - | return 0, 0, fmt.Errorf("ffmpeg: source %dx%d too small to encode", f.Width, f.Height) | |
| 118 | + | if f.Width%2 != 0 || f.Height%2 != 0 { | |
| 119 | + | return 0, 0, fmt.Errorf("ffmpeg: source %dx%d has an odd dimension; set an even output size to scale", f.Width, f.Height) | |
| 119 | 120 | } | |
| 120 | - | return w, h, nil | |
| 121 | + | return f.Width, f.Height, nil | |
| 121 | 122 | } | |
| 122 | 123 | if s.Width < 2 || s.Height < 2 || s.Width > ports.MaxDimension || s.Height > ports.MaxDimension || | |
| 123 | 124 | s.Width%2 != 0 || s.Height%2 != 0 { | |
| 124 | 125 | return 0, 0, fmt.Errorf("ffmpeg: output size %dx%d must be even, positive and at most %d", s.Width, s.Height, ports.MaxDimension) | |
| 125 | 126 | } | |
| 127 | + | if s.Width > ports.MaxFrameBytes/ports.BytesPerPixel/s.Height { | |
| 128 | + | return 0, 0, fmt.Errorf("ffmpeg: output size %dx%d exceeds %d bytes per frame", s.Width, s.Height, ports.MaxFrameBytes) | |
| 129 | + | } | |
| 126 | 130 | return s.Width, s.Height, nil | |
| 127 | 131 | } | |
| 128 | 132 | ||
| 129 | 133 | // buildArgs returns the FFmpeg argument vector (no shell). Input is raw bgr0 | |
| 130 | 134 | // on stdin; MP4 is fragmented to fd 3, an inherited pre-opened output file. | |
| 131 | - | // When retain is set, an odd source is cropped by at most one row/column | |
| 132 | - | // instead of being resampled. | |
| 133 | - | func buildArgs(f ports.Frame, outW, outH, fps int, retain bool) []string { | |
| 134 | - | filter := "" | |
| 135 | - | if retain && (outW != f.Width || outH != f.Height) { | |
| 136 | - | filter = "crop=" + strconv.Itoa(outW) + ":" + strconv.Itoa(outH) + ":0:0," | |
| 137 | - | } | |
| 138 | - | filter += "scale=" + strconv.Itoa(outW) + ":" + strconv.Itoa(outH) + | |
| 135 | + | // The scale filter always runs: with unchanged geometry it only performs the | |
| 136 | + | // required RGB to BT.709 limited-range YUV conversion and does not resample. | |
| 137 | + | func buildArgs(f ports.Frame, outW, outH, fps int) []string { | |
| 138 | + | filter := "scale=" + strconv.Itoa(outW) + ":" + strconv.Itoa(outH) + | |
| 139 | 139 | ":flags=bicubic:out_color_matrix=bt709:out_range=tv,format=yuv420p" | |
| 140 | 140 | rate := strconv.Itoa(fps) | |
| 141 | 141 | return []string{ | |
| @@ -143,8 +143,8 @@ func buildArgs(f ports.Frame, outW, outH, fps int, retain bool) []string { | |||
| 143 | 143 | "-f", "rawvideo", "-pix_fmt", "bgr0", | |
| 144 | 144 | "-video_size", strconv.Itoa(f.Width) + "x" + strconv.Itoa(f.Height), | |
| 145 | 145 | "-framerate", rate, "-i", "pipe:0", | |
| 146 | - | "-an", "-vf", filter, | |
| 147 | - | "-c:v", "libx264", "-preset", "fast", "-crf", "23", | |
| 146 | + | "-filter_threads", "1", "-an", "-vf", filter, | |
| 147 | + | "-c:v", "libx264", "-threads", "2", "-preset", "fast", "-crf", "23", | |
| 148 | 148 | "-x264-params", "rc-lookahead=10:sync-lookahead=0", | |
| 149 | 149 | "-g", strconv.Itoa(fps * 2), "-r", rate, | |
| 150 | 150 | "-colorspace", "bt709", "-color_primaries", "bt709", "-color_trc", "bt709", "-color_range", "tv", | |
| @@ -181,7 +181,7 @@ func (w *Writer) Start(ctx context.Context, first ports.Frame, path string, s po | |||
| 181 | 181 | if err != nil { | |
| 182 | 182 | return fmt.Errorf("ffmpeg: %w", err) | |
| 183 | 183 | } | |
| 184 | - | out, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o644) | |
| 184 | + | out, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o600) | |
| 185 | 185 | if err != nil { | |
| 186 | 186 | if errors.Is(err, fs.ErrExist) { | |
| 187 | 187 | return fmt.Errorf("%w: %s", ports.ErrPathExists, path) | |
| @@ -203,7 +203,7 @@ func (w *Writer) Start(ctx context.Context, first ports.Frame, path string, s po | |||
| 203 | 203 | return fail(fmt.Errorf("ffmpeg: stdin pipe: %w", err), out) | |
| 204 | 204 | } | |
| 205 | 205 | stderr := &boundedBuffer{limit: stderrLimit} | |
| 206 | - | cmd := exec.Command(bin, buildArgs(first, outW, outH, s.FPS, s.Width == 0)...) | |
| 206 | + | cmd := exec.Command(bin, buildArgs(first, outW, outH, s.FPS)...) | |
| 207 | 207 | cmd.Stdin = pr | |
| 208 | 208 | cmd.Stderr = stderr | |
| 209 | 209 | cmd.ExtraFiles = []*os.File{out} | |
| @@ -107,35 +107,35 @@ func NewWithBinary(binary string) *Writer { | |||||
| 107 | } | 107 | } | ||
| 108 | 108 | ||||
| 109 | // outputSize validates settings and returns the encoded dimensions. Zero | 109 | // outputSize validates settings and returns the encoded dimensions. Zero | ||
| 110 | - | // dimensions retain the source size rounded down to even values (yuv420p). | 110 | + | // dimensions retain the source resolution exactly, which yuv420p only allows |
| 111 | + | // for even sizes; odd sources are rejected rather than cropped. Explicit | |||
| 112 | + | // sizes must be even and no larger than a captured frame may be. | |||
| 111 | func outputSize(f ports.Frame, s ports.VideoSettings) (int, int, error) { | 113 | func outputSize(f ports.Frame, s ports.VideoSettings) (int, int, error) { | ||
| 112 | if s.FPS < 1 || s.FPS > ports.MaxFPS { | 114 | if s.FPS < 1 || s.FPS > ports.MaxFPS { | ||
| 113 | return 0, 0, fmt.Errorf("ffmpeg: fps %d outside 1..%d", s.FPS, ports.MaxFPS) | 115 | return 0, 0, fmt.Errorf("ffmpeg: fps %d outside 1..%d", s.FPS, ports.MaxFPS) | ||
| 114 | } | 116 | } | ||
| 115 | if s.Width == 0 && s.Height == 0 { | 117 | if s.Width == 0 && s.Height == 0 { | ||
| 116 | - | w, h := f.Width&^1, f.Height&^1 | 118 | + | if f.Width%2 != 0 || f.Height%2 != 0 { |
| 117 | - | if w < 2 || h < 2 { | 119 | + | return 0, 0, fmt.Errorf("ffmpeg: source %dx%d has an odd dimension; set an even output size to scale", f.Width, f.Height) |
| 118 | - | return 0, 0, fmt.Errorf("ffmpeg: source %dx%d too small to encode", f.Width, f.Height) | |||
| 119 | } | 120 | } | ||
| 120 | - | return w, h, nil | 121 | + | return f.Width, f.Height, nil |
| 121 | } | 122 | } | ||
| 122 | if s.Width < 2 || s.Height < 2 || s.Width > ports.MaxDimension || s.Height > ports.MaxDimension || | 123 | if s.Width < 2 || s.Height < 2 || s.Width > ports.MaxDimension || s.Height > ports.MaxDimension || | ||
| 123 | s.Width%2 != 0 || s.Height%2 != 0 { | 124 | s.Width%2 != 0 || s.Height%2 != 0 { | ||
| 124 | return 0, 0, fmt.Errorf("ffmpeg: output size %dx%d must be even, positive and at most %d", s.Width, s.Height, ports.MaxDimension) | 125 | return 0, 0, fmt.Errorf("ffmpeg: output size %dx%d must be even, positive and at most %d", s.Width, s.Height, ports.MaxDimension) | ||
| 125 | } | 126 | } | ||
| 127 | + | if s.Width > ports.MaxFrameBytes/ports.BytesPerPixel/s.Height { | |||
| 128 | + | return 0, 0, fmt.Errorf("ffmpeg: output size %dx%d exceeds %d bytes per frame", s.Width, s.Height, ports.MaxFrameBytes) | |||
| 129 | + | } | |||
| 126 | return s.Width, s.Height, nil | 130 | return s.Width, s.Height, nil | ||
| 127 | } | 131 | } | ||
| 128 | 132 | ||||
| 129 | // buildArgs returns the FFmpeg argument vector (no shell). Input is raw bgr0 | 133 | // buildArgs returns the FFmpeg argument vector (no shell). Input is raw bgr0 | ||
| 130 | // on stdin; MP4 is fragmented to fd 3, an inherited pre-opened output file. | 134 | // on stdin; MP4 is fragmented to fd 3, an inherited pre-opened output file. | ||
| 131 | - | // When retain is set, an odd source is cropped by at most one row/column | 135 | + | // The scale filter always runs: with unchanged geometry it only performs the |
| 132 | - | // instead of being resampled. | 136 | + | // required RGB to BT.709 limited-range YUV conversion and does not resample. |
| 133 | - | func buildArgs(f ports.Frame, outW, outH, fps int, retain bool) []string { | 137 | + | func buildArgs(f ports.Frame, outW, outH, fps int) []string { |
| 134 | - | filter := "" | 138 | + | filter := "scale=" + strconv.Itoa(outW) + ":" + strconv.Itoa(outH) + |
| 135 | - | if retain && (outW != f.Width || outH != f.Height) { | |||
| 136 | - | filter = "crop=" + strconv.Itoa(outW) + ":" + strconv.Itoa(outH) + ":0:0," | |||
| 137 | - | } | |||
| 138 | - | filter += "scale=" + strconv.Itoa(outW) + ":" + strconv.Itoa(outH) + | |||
| 139 | ":flags=bicubic:out_color_matrix=bt709:out_range=tv,format=yuv420p" | 139 | ":flags=bicubic:out_color_matrix=bt709:out_range=tv,format=yuv420p" | ||
| 140 | rate := strconv.Itoa(fps) | 140 | rate := strconv.Itoa(fps) | ||
| 141 | return []string{ | 141 | return []string{ | ||
| @@ -143,8 +143,8 @@ func buildArgs(f ports.Frame, outW, outH, fps int, retain bool) []string { | |||||
| 143 | "-f", "rawvideo", "-pix_fmt", "bgr0", | 143 | "-f", "rawvideo", "-pix_fmt", "bgr0", | ||
| 144 | "-video_size", strconv.Itoa(f.Width) + "x" + strconv.Itoa(f.Height), | 144 | "-video_size", strconv.Itoa(f.Width) + "x" + strconv.Itoa(f.Height), | ||
| 145 | "-framerate", rate, "-i", "pipe:0", | 145 | "-framerate", rate, "-i", "pipe:0", | ||
| 146 | - | "-an", "-vf", filter, | 146 | + | "-filter_threads", "1", "-an", "-vf", filter, |
| 147 | - | "-c:v", "libx264", "-preset", "fast", "-crf", "23", | 147 | + | "-c:v", "libx264", "-threads", "2", "-preset", "fast", "-crf", "23", |
| 148 | "-x264-params", "rc-lookahead=10:sync-lookahead=0", | 148 | "-x264-params", "rc-lookahead=10:sync-lookahead=0", | ||
| 149 | "-g", strconv.Itoa(fps * 2), "-r", rate, | 149 | "-g", strconv.Itoa(fps * 2), "-r", rate, | ||
| 150 | "-colorspace", "bt709", "-color_primaries", "bt709", "-color_trc", "bt709", "-color_range", "tv", | 150 | "-colorspace", "bt709", "-color_primaries", "bt709", "-color_trc", "bt709", "-color_range", "tv", | ||
| @@ -181,7 +181,7 @@ func (w *Writer) Start(ctx context.Context, first ports.Frame, path string, s po | |||||
| 181 | if err != nil { | 181 | if err != nil { | ||
| 182 | return fmt.Errorf("ffmpeg: %w", err) | 182 | return fmt.Errorf("ffmpeg: %w", err) | ||
| 183 | } | 183 | } | ||
| 184 | - | out, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o644) | 184 | + | out, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o600) |
| 185 | if err != nil { | 185 | if err != nil { | ||
| 186 | if errors.Is(err, fs.ErrExist) { | 186 | if errors.Is(err, fs.ErrExist) { | ||
| 187 | return fmt.Errorf("%w: %s", ports.ErrPathExists, path) | 187 | return fmt.Errorf("%w: %s", ports.ErrPathExists, path) | ||
| @@ -203,7 +203,7 @@ func (w *Writer) Start(ctx context.Context, first ports.Frame, path string, s po | |||||
| 203 | return fail(fmt.Errorf("ffmpeg: stdin pipe: %w", err), out) | 203 | return fail(fmt.Errorf("ffmpeg: stdin pipe: %w", err), out) | ||
| 204 | } | 204 | } | ||
| 205 | stderr := &boundedBuffer{limit: stderrLimit} | 205 | stderr := &boundedBuffer{limit: stderrLimit} | ||
| 206 | - | cmd := exec.Command(bin, buildArgs(first, outW, outH, s.FPS, s.Width == 0)...) | 206 | + | cmd := exec.Command(bin, buildArgs(first, outW, outH, s.FPS)...) |
| 207 | cmd.Stdin = pr | 207 | cmd.Stdin = pr | ||
| 208 | cmd.Stderr = stderr | 208 | cmd.Stderr = stderr | ||
| 209 | cmd.ExtraFiles = []*os.File{out} | 209 | cmd.ExtraFiles = []*os.File{out} | ||
M internal/adapters/ffmpeg/ffmpeg_test.go +58 -7
| @@ -79,9 +79,13 @@ func TestStartValidation(t *testing.T) { | |||
| 79 | 79 | {"one dimension", f, ports.VideoSettings{FPS: 30, Width: 32}, false}, | |
| 80 | 80 | {"odd output", f, ports.VideoSettings{FPS: 30, Width: 33, Height: 32}, false}, | |
| 81 | 81 | {"huge output", f, ports.VideoSettings{FPS: 30, Width: 16386, Height: 32}, false}, | |
| 82 | - | {"odd source retained", odd, ports.VideoSettings{FPS: 30}, true}, | |
| 82 | + | {"odd source without size", odd, ports.VideoSettings{FPS: 30}, false}, | |
| 83 | + | {"odd width source without size", newFrame(63, 48, 256, 0), ports.VideoSettings{FPS: 30}, false}, | |
| 84 | + | {"odd source explicit scale", odd, ports.VideoSettings{FPS: 30, Width: 32, Height: 24}, true}, | |
| 85 | + | {"even source retained", f, ports.VideoSettings{FPS: 30}, true}, | |
| 86 | + | {"output over frame limit", f, ports.VideoSettings{FPS: 30, Width: 16384, Height: 16384}, false}, | |
| 83 | 87 | {"even scale", f, ports.VideoSettings{FPS: 120, Width: 32, Height: 24}, true}, | |
| 84 | - | {"1x1 source retained", newFrame(1, 1, 4, 0), ports.VideoSettings{FPS: 30}, false}, | |
| 88 | + | {"1x1 source without size", newFrame(1, 1, 4, 0), ports.VideoSettings{FPS: 30}, false}, | |
| 85 | 89 | } | |
| 86 | 90 | for _, tc := range cases { | |
| 87 | 91 | t.Run(tc.name, func(t *testing.T) { | |
| @@ -102,14 +106,61 @@ func TestStartValidation(t *testing.T) { | |||
| 102 | 106 | ||
| 103 | 107 | func TestBuildArgs(t *testing.T) { | |
| 104 | 108 | f := newFrame(63, 47, 252, 0) | |
| 105 | - | args := strings.Join(buildArgs(f, 62, 46, 30, true), " ") | |
| 106 | - | for _, want := range []string{"-pix_fmt bgr0", "-video_size 63x47", "crop=62:46:0:0,scale=62:46", "-preset fast", "libx264", "pipe:3", "-an"} { | |
| 109 | + | args := strings.Join(buildArgs(f, 62, 46, 30), " ") | |
| 110 | + | for _, want := range []string{"-pix_fmt bgr0", "-video_size 63x47", "-vf scale=62:46:", "-preset fast", "libx264", "-threads 2", "-filter_threads 1", "pipe:3", "-an"} { | |
| 107 | 111 | if !strings.Contains(args, want) { | |
| 108 | 112 | t.Errorf("args missing %q: %s", want, args) | |
| 109 | 113 | } | |
| 110 | 114 | } | |
| 111 | - | if strings.Contains(strings.Join(buildArgs(f, 32, 32, 30, false), " "), "crop") { | |
| 112 | - | t.Error("explicit scaling must not crop") | |
| 115 | + | if strings.Contains(args, "crop") { | |
| 116 | + | t.Error("frames must never be cropped") | |
| 117 | + | } | |
| 118 | + | } | |
| 119 | + | ||
| 120 | + | func TestOutputSize(t *testing.T) { | |
| 121 | + | const limit = ports.MaxFrameBytes / ports.BytesPerPixel | |
| 122 | + | f := newFrame(64, 48, 256, 0) | |
| 123 | + | for _, tc := range []struct { | |
| 124 | + | name string | |
| 125 | + | frame ports.Frame | |
| 126 | + | set ports.VideoSettings | |
| 127 | + | wantW, wantH int | |
| 128 | + | ok bool | |
| 129 | + | }{ | |
| 130 | + | {"retain", f, ports.VideoSettings{FPS: 1}, 64, 48, true}, | |
| 131 | + | {"odd height source", newFrame(64, 47, 256, 0), ports.VideoSettings{FPS: 1}, 0, 0, false}, | |
| 132 | + | {"explicit scale of odd source", newFrame(63, 47, 252, 0), ports.VideoSettings{FPS: 1, Width: 64, Height: 48}, 64, 48, true}, | |
| 133 | + | {"only width", f, ports.VideoSettings{FPS: 1, Width: 64}, 0, 0, false}, | |
| 134 | + | {"negative", f, ports.VideoSettings{FPS: 1, Width: -2, Height: 2}, 0, 0, false}, | |
| 135 | + | {"at frame byte limit", f, ports.VideoSettings{FPS: 1, Width: 8192, Height: limit / 8192}, 8192, limit / 8192, true}, | |
| 136 | + | {"over frame byte limit", f, ports.VideoSettings{FPS: 1, Width: 8192, Height: limit/8192 + 2}, 0, 0, false}, | |
| 137 | + | {"max dimensions", f, ports.VideoSettings{FPS: 1, Width: ports.MaxDimension, Height: ports.MaxDimension}, 0, 0, false}, | |
| 138 | + | } { | |
| 139 | + | t.Run(tc.name, func(t *testing.T) { | |
| 140 | + | w, h, err := outputSize(tc.frame, tc.set) | |
| 141 | + | if (err == nil) != tc.ok || w != tc.wantW || h != tc.wantH { | |
| 142 | + | t.Fatalf("got %dx%d, %v; want %dx%d ok=%v", w, h, err, tc.wantW, tc.wantH, tc.ok) | |
| 143 | + | } | |
| 144 | + | }) | |
| 145 | + | } | |
| 146 | + | } | |
| 147 | + | ||
| 148 | + | func TestOutputFilesArePrivate(t *testing.T) { | |
| 149 | + | w := helperWriter(t, "drain") | |
| 150 | + | w.closeTimeout = 10 * time.Second | |
| 151 | + | path := filepath.Join(t.TempDir(), "v.mp4") | |
| 152 | + | f := newFrame(4, 4, 16, 0) | |
| 153 | + | if err := w.Start(context.Background(), f, path, settings()); err != nil { | |
| 154 | + | t.Fatal(err) | |
| 155 | + | } | |
| 156 | + | if err := w.Write(context.Background(), f); err != nil { | |
| 157 | + | t.Fatal(err) | |
| 158 | + | } | |
| 159 | + | if err := w.Close(); err != nil { | |
| 160 | + | t.Fatal(err) | |
| 161 | + | } | |
| 162 | + | if info, err := os.Stat(path); err != nil || info.Mode().Perm() != 0o600 { | |
| 163 | + | t.Fatalf("mode = %v, err = %v; want 0600", info.Mode(), err) | |
| 113 | 164 | } | |
| 114 | 165 | } | |
| 115 | 166 | ||
| @@ -471,7 +522,7 @@ func TestRealFFmpeg(t *testing.T) { | |||
| 471 | 522 | cancelInstead bool | |
| 472 | 523 | }{ | |
| 473 | 524 | {"retain padded inverted", 64, 48, 64*4 + 16, ports.VideoSettings{FPS: 30}, 64, 48, true, false}, | |
| 474 | - | {"odd source cropped", 63, 47, 63 * 4, ports.VideoSettings{FPS: 30}, 62, 46, false, false}, | |
| 525 | + | {"odd source scaled", 63, 47, 63 * 4, ports.VideoSettings{FPS: 30, Width: 62, Height: 46}, 62, 46, false, false}, | |
| 475 | 526 | {"scaled", 64, 48, 64 * 4, ports.VideoSettings{FPS: 60, Width: 32, Height: 24}, 32, 24, false, false}, | |
| 476 | 527 | {"graceful cancel keeps video", 64, 48, 64 * 4, ports.VideoSettings{FPS: 30}, 64, 48, false, true}, | |
| 477 | 528 | } { | |
| @@ -79,9 +79,13 @@ func TestStartValidation(t *testing.T) { | |||||
| 79 | {"one dimension", f, ports.VideoSettings{FPS: 30, Width: 32}, false}, | 79 | {"one dimension", f, ports.VideoSettings{FPS: 30, Width: 32}, false}, | ||
| 80 | {"odd output", f, ports.VideoSettings{FPS: 30, Width: 33, Height: 32}, false}, | 80 | {"odd output", f, ports.VideoSettings{FPS: 30, Width: 33, Height: 32}, false}, | ||
| 81 | {"huge output", f, ports.VideoSettings{FPS: 30, Width: 16386, Height: 32}, false}, | 81 | {"huge output", f, ports.VideoSettings{FPS: 30, Width: 16386, Height: 32}, false}, | ||
| 82 | - | {"odd source retained", odd, ports.VideoSettings{FPS: 30}, true}, | 82 | + | {"odd source without size", odd, ports.VideoSettings{FPS: 30}, false}, |
| 83 | + | {"odd width source without size", newFrame(63, 48, 256, 0), ports.VideoSettings{FPS: 30}, false}, | |||
| 84 | + | {"odd source explicit scale", odd, ports.VideoSettings{FPS: 30, Width: 32, Height: 24}, true}, | |||
| 85 | + | {"even source retained", f, ports.VideoSettings{FPS: 30}, true}, | |||
| 86 | + | {"output over frame limit", f, ports.VideoSettings{FPS: 30, Width: 16384, Height: 16384}, false}, | |||
| 83 | {"even scale", f, ports.VideoSettings{FPS: 120, Width: 32, Height: 24}, true}, | 87 | {"even scale", f, ports.VideoSettings{FPS: 120, Width: 32, Height: 24}, true}, | ||
| 84 | - | {"1x1 source retained", newFrame(1, 1, 4, 0), ports.VideoSettings{FPS: 30}, false}, | 88 | + | {"1x1 source without size", newFrame(1, 1, 4, 0), ports.VideoSettings{FPS: 30}, false}, |
| 85 | } | 89 | } | ||
| 86 | for _, tc := range cases { | 90 | for _, tc := range cases { | ||
| 87 | t.Run(tc.name, func(t *testing.T) { | 91 | t.Run(tc.name, func(t *testing.T) { | ||
| @@ -102,14 +106,61 @@ func TestStartValidation(t *testing.T) { | |||||
| 102 | 106 | ||||
| 103 | func TestBuildArgs(t *testing.T) { | 107 | func TestBuildArgs(t *testing.T) { | ||
| 104 | f := newFrame(63, 47, 252, 0) | 108 | f := newFrame(63, 47, 252, 0) | ||
| 105 | - | args := strings.Join(buildArgs(f, 62, 46, 30, true), " ") | 109 | + | args := strings.Join(buildArgs(f, 62, 46, 30), " ") |
| 106 | - | for _, want := range []string{"-pix_fmt bgr0", "-video_size 63x47", "crop=62:46:0:0,scale=62:46", "-preset fast", "libx264", "pipe:3", "-an"} { | 110 | + | for _, want := range []string{"-pix_fmt bgr0", "-video_size 63x47", "-vf scale=62:46:", "-preset fast", "libx264", "-threads 2", "-filter_threads 1", "pipe:3", "-an"} { |
| 107 | if !strings.Contains(args, want) { | 111 | if !strings.Contains(args, want) { | ||
| 108 | t.Errorf("args missing %q: %s", want, args) | 112 | t.Errorf("args missing %q: %s", want, args) | ||
| 109 | } | 113 | } | ||
| 110 | } | 114 | } | ||
| 111 | - | if strings.Contains(strings.Join(buildArgs(f, 32, 32, 30, false), " "), "crop") { | 115 | + | if strings.Contains(args, "crop") { |
| 112 | - | t.Error("explicit scaling must not crop") | 116 | + | t.Error("frames must never be cropped") |
| 117 | + | } | |||
| 118 | + | } | |||
| 119 | + | ||||
| 120 | + | func TestOutputSize(t *testing.T) { | |||
| 121 | + | const limit = ports.MaxFrameBytes / ports.BytesPerPixel | |||
| 122 | + | f := newFrame(64, 48, 256, 0) | |||
| 123 | + | for _, tc := range []struct { | |||
| 124 | + | name string | |||
| 125 | + | frame ports.Frame | |||
| 126 | + | set ports.VideoSettings | |||
| 127 | + | wantW, wantH int | |||
| 128 | + | ok bool | |||
| 129 | + | }{ | |||
| 130 | + | {"retain", f, ports.VideoSettings{FPS: 1}, 64, 48, true}, | |||
| 131 | + | {"odd height source", newFrame(64, 47, 256, 0), ports.VideoSettings{FPS: 1}, 0, 0, false}, | |||
| 132 | + | {"explicit scale of odd source", newFrame(63, 47, 252, 0), ports.VideoSettings{FPS: 1, Width: 64, Height: 48}, 64, 48, true}, | |||
| 133 | + | {"only width", f, ports.VideoSettings{FPS: 1, Width: 64}, 0, 0, false}, | |||
| 134 | + | {"negative", f, ports.VideoSettings{FPS: 1, Width: -2, Height: 2}, 0, 0, false}, | |||
| 135 | + | {"at frame byte limit", f, ports.VideoSettings{FPS: 1, Width: 8192, Height: limit / 8192}, 8192, limit / 8192, true}, | |||
| 136 | + | {"over frame byte limit", f, ports.VideoSettings{FPS: 1, Width: 8192, Height: limit/8192 + 2}, 0, 0, false}, | |||
| 137 | + | {"max dimensions", f, ports.VideoSettings{FPS: 1, Width: ports.MaxDimension, Height: ports.MaxDimension}, 0, 0, false}, | |||
| 138 | + | } { | |||
| 139 | + | t.Run(tc.name, func(t *testing.T) { | |||
| 140 | + | w, h, err := outputSize(tc.frame, tc.set) | |||
| 141 | + | if (err == nil) != tc.ok || w != tc.wantW || h != tc.wantH { | |||
| 142 | + | t.Fatalf("got %dx%d, %v; want %dx%d ok=%v", w, h, err, tc.wantW, tc.wantH, tc.ok) | |||
| 143 | + | } | |||
| 144 | + | }) | |||
| 145 | + | } | |||
| 146 | + | } | |||
| 147 | + | ||||
| 148 | + | func TestOutputFilesArePrivate(t *testing.T) { | |||
| 149 | + | w := helperWriter(t, "drain") | |||
| 150 | + | w.closeTimeout = 10 * time.Second | |||
| 151 | + | path := filepath.Join(t.TempDir(), "v.mp4") | |||
| 152 | + | f := newFrame(4, 4, 16, 0) | |||
| 153 | + | if err := w.Start(context.Background(), f, path, settings()); err != nil { | |||
| 154 | + | t.Fatal(err) | |||
| 155 | + | } | |||
| 156 | + | if err := w.Write(context.Background(), f); err != nil { | |||
| 157 | + | t.Fatal(err) | |||
| 158 | + | } | |||
| 159 | + | if err := w.Close(); err != nil { | |||
| 160 | + | t.Fatal(err) | |||
| 161 | + | } | |||
| 162 | + | if info, err := os.Stat(path); err != nil || info.Mode().Perm() != 0o600 { | |||
| 163 | + | t.Fatalf("mode = %v, err = %v; want 0600", info.Mode(), err) | |||
| 113 | } | 164 | } | ||
| 114 | } | 165 | } | ||
| 115 | 166 | ||||
| @@ -471,7 +522,7 @@ func TestRealFFmpeg(t *testing.T) { | |||||
| 471 | cancelInstead bool | 522 | cancelInstead bool | ||
| 472 | }{ | 523 | }{ | ||
| 473 | {"retain padded inverted", 64, 48, 64*4 + 16, ports.VideoSettings{FPS: 30}, 64, 48, true, false}, | 524 | {"retain padded inverted", 64, 48, 64*4 + 16, ports.VideoSettings{FPS: 30}, 64, 48, true, false}, | ||
| 474 | - | {"odd source cropped", 63, 47, 63 * 4, ports.VideoSettings{FPS: 30}, 62, 46, false, false}, | 525 | + | {"odd source scaled", 63, 47, 63 * 4, ports.VideoSettings{FPS: 30, Width: 62, Height: 46}, 62, 46, false, false}, |
| 475 | {"scaled", 64, 48, 64 * 4, ports.VideoSettings{FPS: 60, Width: 32, Height: 24}, 32, 24, false, false}, | 526 | {"scaled", 64, 48, 64 * 4, ports.VideoSettings{FPS: 60, Width: 32, Height: 24}, 32, 24, false, false}, | ||
| 476 | {"graceful cancel keeps video", 64, 48, 64 * 4, ports.VideoSettings{FPS: 30}, 64, 48, false, true}, | 527 | {"graceful cancel keeps video", 64, 48, 64 * 4, ports.VideoSettings{FPS: 30}, 64, 48, false, true}, | ||
| 477 | } { | 528 | } { | ||
M internal/adapters/png/png.go +1 -1
| @@ -41,7 +41,7 @@ func (*Writer) Save(ctx context.Context, f ports.Frame, path string) (err error) | |||
| 41 | 41 | if path == "" { | |
| 42 | 42 | return errors.New("png: empty path") | |
| 43 | 43 | } | |
| 44 | - | file, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o644) | |
| 44 | + | file, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o600) | |
| 45 | 45 | if err != nil { | |
| 46 | 46 | if errors.Is(err, fs.ErrExist) { | |
| 47 | 47 | return fmt.Errorf("%w: %s", ports.ErrPathExists, path) | |
| @@ -41,7 +41,7 @@ func (*Writer) Save(ctx context.Context, f ports.Frame, path string) (err error) | |||||
| 41 | if path == "" { | 41 | if path == "" { | ||
| 42 | return errors.New("png: empty path") | 42 | return errors.New("png: empty path") | ||
| 43 | } | 43 | } | ||
| 44 | - | file, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o644) | 44 | + | file, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o600) |
| 45 | if err != nil { | 45 | if err != nil { | ||
| 46 | if errors.Is(err, fs.ErrExist) { | 46 | if errors.Is(err, fs.ErrExist) { | ||
| 47 | return fmt.Errorf("%w: %s", ports.ErrPathExists, path) | 47 | return fmt.Errorf("%w: %s", ports.ErrPathExists, path) | ||
M internal/adapters/png/png_test.go +3 -0
| @@ -56,6 +56,9 @@ func TestSaveConvertsBGRAWithStrideAndInvert(t *testing.T) { | |||
| 56 | 56 | } | |
| 57 | 57 | } | |
| 58 | 58 | } | |
| 59 | + | if info, err := os.Stat(path); err != nil || info.Mode().Perm() != 0o600 { | |
| 60 | + | t.Fatalf("mode = %v, err = %v; want 0600", info.Mode(), err) | |
| 61 | + | } | |
| 59 | 62 | raw, err := os.ReadFile(path) | |
| 60 | 63 | if err != nil { | |
| 61 | 64 | t.Fatal(err) | |
| @@ -56,6 +56,9 @@ func TestSaveConvertsBGRAWithStrideAndInvert(t *testing.T) { | |||||
| 56 | } | 56 | } | ||
| 57 | } | 57 | } | ||
| 58 | } | 58 | } | ||
| 59 | + | if info, err := os.Stat(path); err != nil || info.Mode().Perm() != 0o600 { | |||
| 60 | + | t.Fatalf("mode = %v, err = %v; want 0600", info.Mode(), err) | |||
| 61 | + | } | |||
| 59 | raw, err := os.ReadFile(path) | 62 | raw, err := os.ReadFile(path) | ||
| 60 | if err != nil { | 63 | if err != nil { | ||
| 61 | t.Fatal(err) | 64 | t.Fatal(err) | ||