refactor: tighten temporary playback format contract
- commit
- ae5412aae6e54b0b6bd5e169f9fb319e5ab4b7f7
- author
- brice · · 2026-04-10 12:51 UTC
- parent
- d43cf59
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +24 -19
M internal/capi/stream_ops.go +9 -2
| @@ -1,6 +1,7 @@ | |||
| 1 | 1 | package capi | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "errors" | |
| 4 | 5 | "fmt" | |
| 5 | 6 | "sync" | |
| 6 | 7 | "unsafe" | |
| @@ -18,6 +19,11 @@ func (e *PWError) Error() string { | |||
| 18 | 19 | return fmt.Sprintf("%s failed: %d", e.Func, e.Code) | |
| 19 | 20 | } | |
| 20 | 21 | ||
| 22 | + | // ErrInvalidPlaybackFormat is returned by ConnectPlaybackStream when the | |
| 23 | + | // provided PlaybackFormat has zero or negative fields. Callers can use | |
| 24 | + | // errors.Is to check for this condition specifically. | |
| 25 | + | var ErrInvalidPlaybackFormat = errors.New("invalid playback format") | |
| 26 | + | ||
| 21 | 27 | var ( | |
| 22 | 28 | errStreamCreate = &PWError{Func: "pw_stream_new_simple", Code: 0} | |
| 23 | 29 | errMainLoopCreate = &PWError{Func: "pw_main_loop_new", Code: 0} | |
| @@ -90,9 +96,10 @@ func (s *streamOpsImpl) ConnectPlaybackStream(streamPtr unsafe.Pointer, format p | |||
| 90 | 96 | // NOTE: The format fields are not yet forwarded to PipeWire as SPA params. | |
| 91 | 97 | // This validation is a temporary guard that will be superseded once SPA | |
| 92 | 98 | // param building is wired in a subsequent task. | |
| 99 | + | // Returns ErrInvalidPlaybackFormat (wrappable via errors.Is) for bad values. | |
| 93 | 100 | if format.SampleRate <= 0 || format.Channels <= 0 || format.FramesPerBuffer <= 0 { | |
| 94 | - | return fmt.Errorf("invalid PlaybackFormat: SampleRate=%d, Channels=%d, FramesPerBuffer=%d", | |
| 95 | - | format.SampleRate, format.Channels, format.FramesPerBuffer) | |
| 101 | + | return fmt.Errorf("%w: SampleRate=%d, Channels=%d, FramesPerBuffer=%d", | |
| 102 | + | ErrInvalidPlaybackFormat, format.SampleRate, format.Channels, format.FramesPerBuffer) | |
| 96 | 103 | } | |
| 97 | 104 | // PW_DIRECTION_OUTPUT = 1, PW_ID_ANY = 0xffffffff | |
| 98 | 105 | // PW_STREAM_FLAG_AUTOCONNECT | PW_STREAM_FLAG_MAP_BUFFERS = 0x01 | 0x04 | |
| @@ -1,6 +1,7 @@ | |||||
| 1 | package capi | 1 | package capi | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "errors" | |||
| 4 | "fmt" | 5 | "fmt" | ||
| 5 | "sync" | 6 | "sync" | ||
| 6 | "unsafe" | 7 | "unsafe" | ||
| @@ -18,6 +19,11 @@ func (e *PWError) Error() string { | |||||
| 18 | return fmt.Sprintf("%s failed: %d", e.Func, e.Code) | 19 | return fmt.Sprintf("%s failed: %d", e.Func, e.Code) | ||
| 19 | } | 20 | } | ||
| 20 | 21 | ||||
| 22 | + | // ErrInvalidPlaybackFormat is returned by ConnectPlaybackStream when the | |||
| 23 | + | // provided PlaybackFormat has zero or negative fields. Callers can use | |||
| 24 | + | // errors.Is to check for this condition specifically. | |||
| 25 | + | var ErrInvalidPlaybackFormat = errors.New("invalid playback format") | |||
| 26 | + | ||||
| 21 | var ( | 27 | var ( | ||
| 22 | errStreamCreate = &PWError{Func: "pw_stream_new_simple", Code: 0} | 28 | errStreamCreate = &PWError{Func: "pw_stream_new_simple", Code: 0} | ||
| 23 | errMainLoopCreate = &PWError{Func: "pw_main_loop_new", Code: 0} | 29 | errMainLoopCreate = &PWError{Func: "pw_main_loop_new", Code: 0} | ||
| @@ -90,9 +96,10 @@ func (s *streamOpsImpl) ConnectPlaybackStream(streamPtr unsafe.Pointer, format p | |||||
| 90 | // NOTE: The format fields are not yet forwarded to PipeWire as SPA params. | 96 | // NOTE: The format fields are not yet forwarded to PipeWire as SPA params. | ||
| 91 | // This validation is a temporary guard that will be superseded once SPA | 97 | // This validation is a temporary guard that will be superseded once SPA | ||
| 92 | // param building is wired in a subsequent task. | 98 | // param building is wired in a subsequent task. | ||
| 99 | + | // Returns ErrInvalidPlaybackFormat (wrappable via errors.Is) for bad values. | |||
| 93 | if format.SampleRate <= 0 || format.Channels <= 0 || format.FramesPerBuffer <= 0 { | 100 | if format.SampleRate <= 0 || format.Channels <= 0 || format.FramesPerBuffer <= 0 { | ||
| 94 | - | return fmt.Errorf("invalid PlaybackFormat: SampleRate=%d, Channels=%d, FramesPerBuffer=%d", | 101 | + | return fmt.Errorf("%w: SampleRate=%d, Channels=%d, FramesPerBuffer=%d", |
| 95 | - | format.SampleRate, format.Channels, format.FramesPerBuffer) | 102 | + | ErrInvalidPlaybackFormat, format.SampleRate, format.Channels, format.FramesPerBuffer) |
| 96 | } | 103 | } | ||
| 97 | // PW_DIRECTION_OUTPUT = 1, PW_ID_ANY = 0xffffffff | 104 | // PW_DIRECTION_OUTPUT = 1, PW_ID_ANY = 0xffffffff | ||
| 98 | // PW_STREAM_FLAG_AUTOCONNECT | PW_STREAM_FLAG_MAP_BUFFERS = 0x01 | 0x04 | 105 | // PW_STREAM_FLAG_AUTOCONNECT | PW_STREAM_FLAG_MAP_BUFFERS = 0x01 | 0x04 | ||
M internal/capi/stream_ops_test.go +10 -7
| @@ -268,9 +268,9 @@ func TestDisconnectStreamReturnsNilOnSuccess(t *testing.T) { | |||
| 268 | 268 | } | |
| 269 | 269 | ||
| 270 | 270 | // TestConnectPlaybackStreamRejectsMissingFormat verifies that ConnectPlaybackStream | |
| 271 | - | // rejects a zero-valued PlaybackFormat at the StreamOps layer. This is a temporary | |
| 272 | - | // guard; once SPA params are wired, the rejection will also cover missing/invalid | |
| 273 | - | // SPA param construction. | |
| 271 | + | // rejects a zero-valued PlaybackFormat at the StreamOps layer by returning | |
| 272 | + | // ErrInvalidPlaybackFormat. This is a temporary guard; once SPA params are | |
| 273 | + | // wired, the rejection will also cover missing/invalid SPA param construction. | |
| 274 | 274 | func TestConnectPlaybackStreamRejectsMissingFormat(t *testing.T) { | |
| 275 | 275 | origConnect := pw_stream_connect | |
| 276 | 276 | t.Cleanup(func() { pw_stream_connect = origConnect }) | |
| @@ -290,14 +290,17 @@ func TestConnectPlaybackStreamRejectsMissingFormat(t *testing.T) { | |||
| 290 | 290 | if err == nil { | |
| 291 | 291 | t.Fatal("expected error for zero-valued PlaybackFormat, got nil") | |
| 292 | 292 | } | |
| 293 | + | if !errors.Is(err, ErrInvalidPlaybackFormat) { | |
| 294 | + | t.Fatalf("expected error wrapping ErrInvalidPlaybackFormat, got: %v", err) | |
| 295 | + | } | |
| 293 | 296 | } | |
| 294 | 297 | ||
| 295 | - | // TestConnectPlaybackStreamBuildsParamsFromFormat verifies that ConnectPlaybackStream | |
| 298 | + | // TestConnectPlaybackStreamAcceptsValidFormat verifies that ConnectPlaybackStream | |
| 296 | 299 | // accepts a valid PlaybackFormat and calls pw_stream_connect successfully. | |
| 297 | 300 | // NOTE: The format is not yet forwarded to PipeWire as SPA params; this test | |
| 298 | - | // only confirms that the format is accepted at the seam. A subsequent task | |
| 299 | - | // will add tests verifying that the format values reach the C call. | |
| 300 | - | func TestConnectPlaybackStreamBuildsParamsFromFormat(t *testing.T) { | |
| 301 | + | // only confirms that the format is accepted at the validation seam. A subsequent | |
| 302 | + | // task will add tests verifying that the format values reach the C call. | |
| 303 | + | func TestConnectPlaybackStreamAcceptsValidFormat(t *testing.T) { | |
| 301 | 304 | origConnect := pw_stream_connect | |
| 302 | 305 | t.Cleanup(func() { pw_stream_connect = origConnect }) | |
| 303 | 306 | ||
| @@ -268,9 +268,9 @@ func TestDisconnectStreamReturnsNilOnSuccess(t *testing.T) { | |||||
| 268 | } | 268 | } | ||
| 269 | 269 | ||||
| 270 | // TestConnectPlaybackStreamRejectsMissingFormat verifies that ConnectPlaybackStream | 270 | // TestConnectPlaybackStreamRejectsMissingFormat verifies that ConnectPlaybackStream | ||
| 271 | - | // rejects a zero-valued PlaybackFormat at the StreamOps layer. This is a temporary | 271 | + | // rejects a zero-valued PlaybackFormat at the StreamOps layer by returning |
| 272 | - | // guard; once SPA params are wired, the rejection will also cover missing/invalid | 272 | + | // ErrInvalidPlaybackFormat. This is a temporary guard; once SPA params are |
| 273 | - | // SPA param construction. | 273 | + | // wired, the rejection will also cover missing/invalid SPA param construction. |
| 274 | func TestConnectPlaybackStreamRejectsMissingFormat(t *testing.T) { | 274 | func TestConnectPlaybackStreamRejectsMissingFormat(t *testing.T) { | ||
| 275 | origConnect := pw_stream_connect | 275 | origConnect := pw_stream_connect | ||
| 276 | t.Cleanup(func() { pw_stream_connect = origConnect }) | 276 | t.Cleanup(func() { pw_stream_connect = origConnect }) | ||
| @@ -290,14 +290,17 @@ func TestConnectPlaybackStreamRejectsMissingFormat(t *testing.T) { | |||||
| 290 | if err == nil { | 290 | if err == nil { | ||
| 291 | t.Fatal("expected error for zero-valued PlaybackFormat, got nil") | 291 | t.Fatal("expected error for zero-valued PlaybackFormat, got nil") | ||
| 292 | } | 292 | } | ||
| 293 | + | if !errors.Is(err, ErrInvalidPlaybackFormat) { | |||
| 294 | + | t.Fatalf("expected error wrapping ErrInvalidPlaybackFormat, got: %v", err) | |||
| 295 | + | } | |||
| 293 | } | 296 | } | ||
| 294 | 297 | ||||
| 295 | - | // TestConnectPlaybackStreamBuildsParamsFromFormat verifies that ConnectPlaybackStream | 298 | + | // TestConnectPlaybackStreamAcceptsValidFormat verifies that ConnectPlaybackStream |
| 296 | // accepts a valid PlaybackFormat and calls pw_stream_connect successfully. | 299 | // accepts a valid PlaybackFormat and calls pw_stream_connect successfully. | ||
| 297 | // NOTE: The format is not yet forwarded to PipeWire as SPA params; this test | 300 | // NOTE: The format is not yet forwarded to PipeWire as SPA params; this test | ||
| 298 | - | // only confirms that the format is accepted at the seam. A subsequent task | 301 | + | // only confirms that the format is accepted at the validation seam. A subsequent |
| 299 | - | // will add tests verifying that the format values reach the C call. | 302 | + | // task will add tests verifying that the format values reach the C call. |
| 300 | - | func TestConnectPlaybackStreamBuildsParamsFromFormat(t *testing.T) { | 303 | + | func TestConnectPlaybackStreamAcceptsValidFormat(t *testing.T) { |
| 301 | origConnect := pw_stream_connect | 304 | origConnect := pw_stream_connect | ||
| 302 | t.Cleanup(func() { pw_stream_connect = origConnect }) | 305 | t.Cleanup(func() { pw_stream_connect = origConnect }) | ||
| 303 | 306 | ||||
M internal/ports/out/capi.go +2 -7
| @@ -1,11 +1,6 @@ | |||
| 1 | 1 | // Temporary manual composition of the outbound CAPI interface. | |
| 2 | 2 | // This file bridges the gap until the generator emits a composite CAPI interface. | |
| 3 | + | // NOTE: The CAPI type is now generated in capi_gen.go — this file only holds | |
| 4 | + | // the narrative comment. The actual composite interface is capi_gen.go CAPI. | |
| 3 | 5 | ||
| 4 | 6 | package out | |
| 5 | - | ||
| 6 | - | // CAPI is a composite interface combining all PipeWire API groups. | |
| 7 | - | // It embeds InitAPI and LoopAPI for unified access to all bound functions. | |
| 8 | - | type CAPI interface { | |
| 9 | - | InitAPI | |
| 10 | - | LoopAPI | |
| 11 | - | } | |
| @@ -1,11 +1,6 @@ | |||||
| 1 | // Temporary manual composition of the outbound CAPI interface. | 1 | // Temporary manual composition of the outbound CAPI interface. | ||
| 2 | // This file bridges the gap until the generator emits a composite CAPI interface. | 2 | // This file bridges the gap until the generator emits a composite CAPI interface. | ||
| 3 | + | // NOTE: The CAPI type is now generated in capi_gen.go — this file only holds | |||
| 4 | + | // the narrative comment. The actual composite interface is capi_gen.go CAPI. | |||
| 3 | 5 | ||||
| 4 | package out | 6 | package out | ||
| 5 | - | ||||
| 6 | - | // CAPI is a composite interface combining all PipeWire API groups. | |||
| 7 | - | // It embeds InitAPI and LoopAPI for unified access to all bound functions. | |||
| 8 | - | type CAPI interface { | |||
| 9 | - | InitAPI | |||
| 10 | - | LoopAPI | |||
| 11 | - | } | |||
M internal/ports/out/stream_ops.go +3 -3
| @@ -22,9 +22,9 @@ type StreamOps interface { | |||
| 22 | 22 | // stream needs more audio data. Returns the stream pointer or an error. | |
| 23 | 23 | CreatePlaybackStream(loopPtr unsafe.Pointer, name string, onProcess func()) (streamPtr unsafe.Pointer, err error) | |
| 24 | 24 | ||
| 25 | - | // ConnectPlaybackStream connects the stream for playback output using the | |
| 26 | - | // given format. The format is currently validated but not yet forwarded to | |
| 27 | - | // PipeWire as SPA params; that wiring will happen in a subsequent task. | |
| 25 | + | // ConnectPlaybackStream connects the stream for playback output. | |
| 26 | + | // The format is validated but not yet forwarded to PipeWire as SPA params; | |
| 27 | + | // that wiring will happen in a subsequent task. | |
| 28 | 28 | ConnectPlaybackStream(streamPtr unsafe.Pointer, format PlaybackFormat) error | |
| 29 | 29 | ||
| 30 | 30 | // SetStreamActive activates or deactivates the stream. | |
| @@ -22,9 +22,9 @@ type StreamOps interface { | |||||
| 22 | // stream needs more audio data. Returns the stream pointer or an error. | 22 | // stream needs more audio data. Returns the stream pointer or an error. | ||
| 23 | CreatePlaybackStream(loopPtr unsafe.Pointer, name string, onProcess func()) (streamPtr unsafe.Pointer, err error) | 23 | CreatePlaybackStream(loopPtr unsafe.Pointer, name string, onProcess func()) (streamPtr unsafe.Pointer, err error) | ||
| 24 | 24 | ||||
| 25 | - | // ConnectPlaybackStream connects the stream for playback output using the | 25 | + | // ConnectPlaybackStream connects the stream for playback output. |
| 26 | - | // given format. The format is currently validated but not yet forwarded to | 26 | + | // The format is validated but not yet forwarded to PipeWire as SPA params; |
| 27 | - | // PipeWire as SPA params; that wiring will happen in a subsequent task. | 27 | + | // that wiring will happen in a subsequent task. |
| 28 | ConnectPlaybackStream(streamPtr unsafe.Pointer, format PlaybackFormat) error | 28 | ConnectPlaybackStream(streamPtr unsafe.Pointer, format PlaybackFormat) error | ||
| 29 | 29 | ||||
| 30 | // SetStreamActive activates or deactivates the stream. | 30 | // SetStreamActive activates or deactivates the stream. | ||