fix(core): refuse Resize while a buffer is borrowed and formats other than BGRx and BGRA
Borrowed buffers hold slots the replacement buffers need, so a resize with every slot borrowed left the stream without buffers. Buffers are sized at 4 bytes per pixel, so other formats are refused. The video errors are exported from the pipewire package for errors.Is.
- commit
- 85ac61edaef6dd070680151ec9fc2cf43f64bb41
- author
- brice · · 2026-10-15 02:40 UTC
- parent
- 5bd5e9f
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +43 -8
M internal/core/video.go +15 -4
| @@ -39,10 +39,11 @@ var ( | |||
| 39 | 39 | ErrVideoStreamCreate = errors.New("failed to create video stream") | |
| 40 | 40 | ErrVideoConnect = errors.New("failed to connect video stream") | |
| 41 | 41 | ErrVideoResize = errors.New("failed to resize video stream") | |
| 42 | + | ErrVideoBorrowed = errors.New("video buffer still borrowed") | |
| 42 | 43 | ) | |
| 43 | 44 | ||
| 44 | 45 | // VideoConfig configures a VideoSource. MaxFPS defaults to 60 and Formats to | |
| 45 | - | // BGRx. | |
| 46 | + | // BGRx. Only BGRx and BGRA are supported: buffers hold 4 bytes per pixel. | |
| 46 | 47 | type VideoConfig struct { | |
| 47 | 48 | Name string | |
| 48 | 49 | Width, Height int | |
| @@ -227,6 +228,11 @@ func (c *Core) NewVideoSource(cfg VideoConfig, h portin.VideoHandler) (*VideoSou | |||
| 227 | 228 | if len(cfg.Formats) > len(s.formats) { | |
| 228 | 229 | return nil, ErrVideoConfig | |
| 229 | 230 | } | |
| 231 | + | for _, f := range cfg.Formats { | |
| 232 | + | if f != spa.VideoFormatBGRx && f != spa.VideoFormatBGRA { | |
| 233 | + | return nil, fmt.Errorf("%w: unsupported format %d", ErrVideoConfig, f) | |
| 234 | + | } | |
| 235 | + | } | |
| 230 | 236 | s.nformats = copy(s.formats[:], cfg.Formats) | |
| 231 | 237 | } | |
| 232 | 238 | s.cfg.Formats = nil // the slot-owned array is the source of truth | |
| @@ -394,9 +400,9 @@ func validVideoSize(w, h int) bool { | |||
| 394 | 400 | return w > 0 && h > 0 && w <= 1<<15 && h <= 1<<15 && w*4*h <= math.MaxInt32 | |
| 395 | 401 | } | |
| 396 | 402 | ||
| 397 | - | // Resize renegotiates the stream with a new size. Buffers PipeWire replaces | |
| 398 | - | // stay valid while borrowed; hand them back with Queue or Release (ideally | |
| 399 | - | // resize when none is borrowed). | |
| 403 | + | // Resize renegotiates the stream with a new size. It fails with | |
| 404 | + | // ErrVideoBorrowed while a buffer is borrowed: PipeWire replaces every | |
| 405 | + | // buffer, and the replacements need the slots the borrowed ones hold. | |
| 400 | 406 | func (s *VideoSource) Resize(w, h int) error { | |
| 401 | 407 | if s.stream == nil { | |
| 402 | 408 | return ErrVideoResize | |
| @@ -404,6 +410,11 @@ func (s *VideoSource) Resize(w, h int) error { | |||
| 404 | 410 | if !validVideoSize(w, h) { | |
| 405 | 411 | return ErrVideoConfig | |
| 406 | 412 | } | |
| 413 | + | for i := range s.slots { | |
| 414 | + | if s.slots[i].borrowed { | |
| 415 | + | return ErrVideoBorrowed | |
| 416 | + | } | |
| 417 | + | } | |
| 407 | 418 | if _, err := spa.VideoEnumFormat(s.enumPod[:], w, h, s.cfg.MaxFPS, s.formats[:s.nformats]...); err != nil { | |
| 408 | 419 | return fmt.Errorf("%w: %v", ErrVideoResize, err) | |
| 409 | 420 | } | |
| @@ -39,10 +39,11 @@ var ( | |||||
| 39 | ErrVideoStreamCreate = errors.New("failed to create video stream") | 39 | ErrVideoStreamCreate = errors.New("failed to create video stream") | ||
| 40 | ErrVideoConnect = errors.New("failed to connect video stream") | 40 | ErrVideoConnect = errors.New("failed to connect video stream") | ||
| 41 | ErrVideoResize = errors.New("failed to resize video stream") | 41 | ErrVideoResize = errors.New("failed to resize video stream") | ||
| 42 | + | ErrVideoBorrowed = errors.New("video buffer still borrowed") | |||
| 42 | ) | 43 | ) | ||
| 43 | 44 | ||||
| 44 | // VideoConfig configures a VideoSource. MaxFPS defaults to 60 and Formats to | 45 | // VideoConfig configures a VideoSource. MaxFPS defaults to 60 and Formats to | ||
| 45 | - | // BGRx. | 46 | + | // BGRx. Only BGRx and BGRA are supported: buffers hold 4 bytes per pixel. |
| 46 | type VideoConfig struct { | 47 | type VideoConfig struct { | ||
| 47 | Name string | 48 | Name string | ||
| 48 | Width, Height int | 49 | Width, Height int | ||
| @@ -227,6 +228,11 @@ func (c *Core) NewVideoSource(cfg VideoConfig, h portin.VideoHandler) (*VideoSou | |||||
| 227 | if len(cfg.Formats) > len(s.formats) { | 228 | if len(cfg.Formats) > len(s.formats) { | ||
| 228 | return nil, ErrVideoConfig | 229 | return nil, ErrVideoConfig | ||
| 229 | } | 230 | } | ||
| 231 | + | for _, f := range cfg.Formats { | |||
| 232 | + | if f != spa.VideoFormatBGRx && f != spa.VideoFormatBGRA { | |||
| 233 | + | return nil, fmt.Errorf("%w: unsupported format %d", ErrVideoConfig, f) | |||
| 234 | + | } | |||
| 235 | + | } | |||
| 230 | s.nformats = copy(s.formats[:], cfg.Formats) | 236 | s.nformats = copy(s.formats[:], cfg.Formats) | ||
| 231 | } | 237 | } | ||
| 232 | s.cfg.Formats = nil // the slot-owned array is the source of truth | 238 | s.cfg.Formats = nil // the slot-owned array is the source of truth | ||
| @@ -394,9 +400,9 @@ func validVideoSize(w, h int) bool { | |||||
| 394 | return w > 0 && h > 0 && w <= 1<<15 && h <= 1<<15 && w*4*h <= math.MaxInt32 | 400 | return w > 0 && h > 0 && w <= 1<<15 && h <= 1<<15 && w*4*h <= math.MaxInt32 | ||
| 395 | } | 401 | } | ||
| 396 | 402 | ||||
| 397 | - | // Resize renegotiates the stream with a new size. Buffers PipeWire replaces | 403 | + | // Resize renegotiates the stream with a new size. It fails with |
| 398 | - | // stay valid while borrowed; hand them back with Queue or Release (ideally | 404 | + | // ErrVideoBorrowed while a buffer is borrowed: PipeWire replaces every |
| 399 | - | // resize when none is borrowed). | 405 | + | // buffer, and the replacements need the slots the borrowed ones hold. |
| 400 | func (s *VideoSource) Resize(w, h int) error { | 406 | func (s *VideoSource) Resize(w, h int) error { | ||
| 401 | if s.stream == nil { | 407 | if s.stream == nil { | ||
| 402 | return ErrVideoResize | 408 | return ErrVideoResize | ||
| @@ -404,6 +410,11 @@ func (s *VideoSource) Resize(w, h int) error { | |||||
| 404 | if !validVideoSize(w, h) { | 410 | if !validVideoSize(w, h) { | ||
| 405 | return ErrVideoConfig | 411 | return ErrVideoConfig | ||
| 406 | } | 412 | } | ||
| 413 | + | for i := range s.slots { | |||
| 414 | + | if s.slots[i].borrowed { | |||
| 415 | + | return ErrVideoBorrowed | |||
| 416 | + | } | |||
| 417 | + | } | |||
| 407 | if _, err := spa.VideoEnumFormat(s.enumPod[:], w, h, s.cfg.MaxFPS, s.formats[:s.nformats]...); err != nil { | 418 | if _, err := spa.VideoEnumFormat(s.enumPod[:], w, h, s.cfg.MaxFPS, s.formats[:s.nformats]...); err != nil { | ||
| 408 | return fmt.Errorf("%w: %v", ErrVideoResize, err) | 419 | return fmt.Errorf("%w: %v", ErrVideoResize, err) | ||
| 409 | } | 420 | } | ||
M internal/core/video_test.go +15 -1
| @@ -427,11 +427,22 @@ func TestVideoSourceResize(t *testing.T) { | |||
| 427 | 427 | if err := r.src.Resize(0, 6); err == nil { | |
| 428 | 428 | t.Fatal("Resize(0, 6) accepted") | |
| 429 | 429 | } | |
| 430 | + | // A borrowed buffer holds a slot the replacements need. | |
| 431 | + | r.negotiate() | |
| 432 | + | f := newFakeBuf(1 << spa.DataMemFd) | |
| 433 | + | r.add(f, 42) | |
| 434 | + | r.api.EXPECT().PWStreamDequeueBuffer(r.stream).Return(f.ptr()).Once() | |
| 435 | + | b := r.src.Dequeue() | |
| 436 | + | if err := r.src.Resize(8, 6); !errors.Is(err, ErrVideoBorrowed) { | |
| 437 | + | t.Fatalf("Resize with a borrowed buffer: %v, want ErrVideoBorrowed", err) | |
| 438 | + | } | |
| 439 | + | r.api.EXPECT().PWStreamReturnBuffer(r.stream, f.ptr()).Return(0).Once() | |
| 440 | + | r.src.Release(b) | |
| 430 | 441 | r.api.EXPECT().PWStreamUpdateParams(r.stream, mock.Anything, uint32(1)).Return(-22).Once() | |
| 431 | 442 | if err := r.src.Resize(8, 6); err == nil { | |
| 432 | 443 | t.Fatal("Resize accepted a failing update_params") | |
| 433 | 444 | } | |
| 434 | - | r.expectDestroy(0) | |
| 445 | + | r.expectDestroy(1) | |
| 435 | 446 | r.src.Destroy() | |
| 436 | 447 | if err := r.src.Resize(8, 6); err == nil { | |
| 437 | 448 | t.Fatal("Resize after Destroy accepted") | |
| @@ -450,6 +461,9 @@ func TestNewVideoSourceErrors(t *testing.T) { | |||
| 450 | 461 | if _, err := core.NewVideoSource(VideoConfig{Width: 1 << 15, Height: 1 << 14}, h); !errors.Is(err, ErrVideoConfig) { | |
| 451 | 462 | t.Errorf("frame size over int32: %v", err) | |
| 452 | 463 | } | |
| 464 | + | if _, err := core.NewVideoSource(VideoConfig{Width: 10, Height: 10, Formats: []spa.VideoFormat{spa.VideoFormatBGRx, 18}}, h); !errors.Is(err, ErrVideoConfig) { | |
| 465 | + | t.Errorf("unsupported format: %v", err) | |
| 466 | + | } | |
| 453 | 467 | if _, err := core.NewVideoSource(VideoConfig{Width: 10, Height: 10}, nil); !errors.Is(err, ErrVideoConfig) { | |
| 454 | 468 | t.Errorf("nil handler: %v", err) | |
| 455 | 469 | } | |
| @@ -427,11 +427,22 @@ func TestVideoSourceResize(t *testing.T) { | |||||
| 427 | if err := r.src.Resize(0, 6); err == nil { | 427 | if err := r.src.Resize(0, 6); err == nil { | ||
| 428 | t.Fatal("Resize(0, 6) accepted") | 428 | t.Fatal("Resize(0, 6) accepted") | ||
| 429 | } | 429 | } | ||
| 430 | + | // A borrowed buffer holds a slot the replacements need. | |||
| 431 | + | r.negotiate() | |||
| 432 | + | f := newFakeBuf(1 << spa.DataMemFd) | |||
| 433 | + | r.add(f, 42) | |||
| 434 | + | r.api.EXPECT().PWStreamDequeueBuffer(r.stream).Return(f.ptr()).Once() | |||
| 435 | + | b := r.src.Dequeue() | |||
| 436 | + | if err := r.src.Resize(8, 6); !errors.Is(err, ErrVideoBorrowed) { | |||
| 437 | + | t.Fatalf("Resize with a borrowed buffer: %v, want ErrVideoBorrowed", err) | |||
| 438 | + | } | |||
| 439 | + | r.api.EXPECT().PWStreamReturnBuffer(r.stream, f.ptr()).Return(0).Once() | |||
| 440 | + | r.src.Release(b) | |||
| 430 | r.api.EXPECT().PWStreamUpdateParams(r.stream, mock.Anything, uint32(1)).Return(-22).Once() | 441 | r.api.EXPECT().PWStreamUpdateParams(r.stream, mock.Anything, uint32(1)).Return(-22).Once() | ||
| 431 | if err := r.src.Resize(8, 6); err == nil { | 442 | if err := r.src.Resize(8, 6); err == nil { | ||
| 432 | t.Fatal("Resize accepted a failing update_params") | 443 | t.Fatal("Resize accepted a failing update_params") | ||
| 433 | } | 444 | } | ||
| 434 | - | r.expectDestroy(0) | 445 | + | r.expectDestroy(1) |
| 435 | r.src.Destroy() | 446 | r.src.Destroy() | ||
| 436 | if err := r.src.Resize(8, 6); err == nil { | 447 | if err := r.src.Resize(8, 6); err == nil { | ||
| 437 | t.Fatal("Resize after Destroy accepted") | 448 | t.Fatal("Resize after Destroy accepted") | ||
| @@ -450,6 +461,9 @@ func TestNewVideoSourceErrors(t *testing.T) { | |||||
| 450 | if _, err := core.NewVideoSource(VideoConfig{Width: 1 << 15, Height: 1 << 14}, h); !errors.Is(err, ErrVideoConfig) { | 461 | if _, err := core.NewVideoSource(VideoConfig{Width: 1 << 15, Height: 1 << 14}, h); !errors.Is(err, ErrVideoConfig) { | ||
| 451 | t.Errorf("frame size over int32: %v", err) | 462 | t.Errorf("frame size over int32: %v", err) | ||
| 452 | } | 463 | } | ||
| 464 | + | if _, err := core.NewVideoSource(VideoConfig{Width: 10, Height: 10, Formats: []spa.VideoFormat{spa.VideoFormatBGRx, 18}}, h); !errors.Is(err, ErrVideoConfig) { | |||
| 465 | + | t.Errorf("unsupported format: %v", err) | |||
| 466 | + | } | |||
| 453 | if _, err := core.NewVideoSource(VideoConfig{Width: 10, Height: 10}, nil); !errors.Is(err, ErrVideoConfig) { | 467 | if _, err := core.NewVideoSource(VideoConfig{Width: 10, Height: 10}, nil); !errors.Is(err, ErrVideoConfig) { | ||
| 454 | t.Errorf("nil handler: %v", err) | 468 | t.Errorf("nil handler: %v", err) | ||
| 455 | } | 469 | } | ||
M pipewire/video.go +13 -3
| @@ -24,9 +24,19 @@ const ( | |||
| 24 | 24 | StreamStateStreaming int32 = 3 | |
| 25 | 25 | ) | |
| 26 | 26 | ||
| 27 | + | // Errors returned by NewVideoSource and VideoSource, for errors.Is. | |
| 28 | + | var ( | |
| 29 | + | ErrVideoConfig = core.ErrVideoConfig | |
| 30 | + | ErrVideoTableFull = core.ErrVideoTableFull | |
| 31 | + | ErrVideoStreamCreate = core.ErrVideoStreamCreate | |
| 32 | + | ErrVideoConnect = core.ErrVideoConnect | |
| 33 | + | ErrVideoResize = core.ErrVideoResize | |
| 34 | + | ErrVideoBorrowed = core.ErrVideoBorrowed | |
| 35 | + | ) | |
| 36 | + | ||
| 27 | 37 | // VideoConfig configures a VideoSource: the node name, the initial size, the | |
| 28 | 38 | // maximum frame rate (60 when zero) and the accepted formats (BGRx when | |
| 29 | - | // empty; at most 4, the first is preferred). | |
| 39 | + | // empty, else BGRx and/or BGRA; the first is preferred). | |
| 30 | 40 | type VideoConfig = core.VideoConfig | |
| 31 | 41 | ||
| 32 | 42 | // VideoHandler receives VideoSource events. Every method runs on the loop | |
| @@ -64,6 +74,6 @@ type VideoBuffer = core.VideoBuffer | |||
| 64 | 74 | // | |
| 65 | 75 | // Buffers PipeWire removes (renegotiation, Resize, Destroy, consumer | |
| 66 | 76 | // disconnect) stay mapped while borrowed and are unmapped when given back with | |
| 67 | - | // Queue or Release. Resize and Destroy are best called when no buffer is | |
| 68 | - | // borrowed. DMA-BUF is not supported. | |
| 77 | + | // Queue or Release. Resize fails with ErrVideoBorrowed while a buffer is | |
| 78 | + | // borrowed; Destroy is best called when none is. DMA-BUF is not supported. | |
| 69 | 79 | type VideoSource = core.VideoSource | |
| @@ -24,9 +24,19 @@ const ( | |||||
| 24 | StreamStateStreaming int32 = 3 | 24 | StreamStateStreaming int32 = 3 | ||
| 25 | ) | 25 | ) | ||
| 26 | 26 | ||||
| 27 | + | // Errors returned by NewVideoSource and VideoSource, for errors.Is. | |||
| 28 | + | var ( | |||
| 29 | + | ErrVideoConfig = core.ErrVideoConfig | |||
| 30 | + | ErrVideoTableFull = core.ErrVideoTableFull | |||
| 31 | + | ErrVideoStreamCreate = core.ErrVideoStreamCreate | |||
| 32 | + | ErrVideoConnect = core.ErrVideoConnect | |||
| 33 | + | ErrVideoResize = core.ErrVideoResize | |||
| 34 | + | ErrVideoBorrowed = core.ErrVideoBorrowed | |||
| 35 | + | ) | |||
| 36 | + | ||||
| 27 | // VideoConfig configures a VideoSource: the node name, the initial size, the | 37 | // VideoConfig configures a VideoSource: the node name, the initial size, the | ||
| 28 | // maximum frame rate (60 when zero) and the accepted formats (BGRx when | 38 | // maximum frame rate (60 when zero) and the accepted formats (BGRx when | ||
| 29 | - | // empty; at most 4, the first is preferred). | 39 | + | // empty, else BGRx and/or BGRA; the first is preferred). |
| 30 | type VideoConfig = core.VideoConfig | 40 | type VideoConfig = core.VideoConfig | ||
| 31 | 41 | ||||
| 32 | // VideoHandler receives VideoSource events. Every method runs on the loop | 42 | // VideoHandler receives VideoSource events. Every method runs on the loop | ||
| @@ -64,6 +74,6 @@ type VideoBuffer = core.VideoBuffer | |||||
| 64 | // | 74 | // | ||
| 65 | // Buffers PipeWire removes (renegotiation, Resize, Destroy, consumer | 75 | // Buffers PipeWire removes (renegotiation, Resize, Destroy, consumer | ||
| 66 | // disconnect) stay mapped while borrowed and are unmapped when given back with | 76 | // disconnect) stay mapped while borrowed and are unmapped when given back with | ||
| 67 | - | // Queue or Release. Resize and Destroy are best called when no buffer is | 77 | + | // Queue or Release. Resize fails with ErrVideoBorrowed while a buffer is |
| 68 | - | // borrowed. DMA-BUF is not supported. | 78 | + | // borrowed; Destroy is best called when none is. DMA-BUF is not supported. |
| 69 | type VideoSource = core.VideoSource | 79 | type VideoSource = core.VideoSource | ||