internal/mux: preserve buffered audio on an unsupported seek - #304
Conversation
hajimehoshi
left a comment
hajimehoshi
left a comment
There was a problem hiding this comment.
The implementation looks correct; I found one minor documentation issue, noted inline.
I recommend keeping TestSeekNonSeekerPreservesBuffer. It provides useful regression coverage by checking preserved samples and playback state for playing and paused players. I confirmed that it fails on the base commit in both cases and passes on this PR. No additional tests seem necessary for this change.
Validation at f6c4eef:
go test ./...passes.go test -race ./internal/muxpasses.- The new regression test passes 20 repetitions.
go vet ./...reports an existingunsafe.Pointerwarning indriver_darwin.go:292; the same warning occurs on the base commit.
AI-generated review by Codex (OpenAI), posted on the user's behalf.
| // Check if the source implements io.Seeker. | ||
| s, ok := p.src.(io.Seeker) | ||
| if !ok { | ||
| return 0, errors.New("mux: the source must implement io.Seeker") |
There was a problem hiding this comment.
[P3] Qualify the public Seek blocking guarantee
This early return means an unsupported seek returns immediately even while a source read is in progress. The public Player.Seek documentation in player.go currently promises that Seek blocks until an ongoing read finishes without qualifying that promise. Please update the documentation so that waiting applies only when the source implements io.Seeker.
There was a problem hiding this comment.
Updated the public Player.Seek documentation in 969d05c to qualify the read-waiting guarantee for sources implementing io.Seeker. Kept the existing regression test. All tests, mux race tests, and 20 repetitions of TestSeekNonSeekerPreservesBuffer pass on macOS. Vet reports the existing unsafe.Pointer warning in driver_darwin.go:292.
2 participants
What issue is this addressing?
None
What type of issue is this addressing?
bug
What this PR does | solves
Seeking a player whose source does not implement
io.Seekercurrently discards its buffered audio before returning an error. Check the source first so that an unsupported seek leaves the buffered data and playback state intact.The regression test covers playing and paused players, and verifies that the buffered samples can still be played after the error. It fails on main because the buffer becomes empty. Successful seeks and errors returned by an underlying
io.Seekerretain their existing behavior.Validation:
go test ./...,go test -race ./internal/mux, andgo vet ./...pass.