Skip to content

oto: Reset loses buffered data; add PauseAndStopReading instead #290

Description

@hajimehoshi

Player.Reset was undeprecated in a6fe004 (#288) with a stronger contract: after Reset returns, the player does not use the source until Play or Seek is called, so the source can be closed safely.

That contract fixes #288, but it is attached to the wrong operation. Reset also clears the player's buffer, so a player resumed with Play after Reset skips every byte that was already read from the source but not yet played.

Reproduction

With the default buffer size (48000 Hz, 2 channels, signed 16-bit):

before Reset: consumed from source = 96000 bytes, buffered = 96000 bytes (0.500[s])
after Reset:  consumed from source = 96000 bytes, buffered = 0 bytes
after Play:   consumed from source = 192000 bytes; 96000 bytes were consumed but never played

0.5 seconds of audio is read from the source, discarded, and playing resumes from the source position after the gap. Pause keeps the buffered data; Reset does not.

Cause

Two orthogonal operations are bundled together:

  1. Stop touching the source: wait for an in-flight read to finish, and forbid further reads. This is what Player still reads after Pause #288 needed.
  2. Discard the buffered data. This is what Reset has always meant, and it is why Reset was deprecated as of v2.3.

Only (1) is required to close a source safely. Undeprecating Reset made the lossy path the documented way to release a source.

Proposal

Add Player.PauseAndStopReading, which gives the guarantee without the data loss:

// PauseAndStopReading pauses its playing and stops reading the source.
// After PauseAndStopReading returns, this player does not read the source until Play or Seek is called,
// so the source can be closed safely.
// Unlike Reset, the buffered data is kept and playing can be resumed without a gap.
// PauseAndStopReading blocks until an ongoing read from the source finishes, if any.
func (p *Player) PauseAndStopReading()

Pause stays non-blocking, as it is called from a game loop. The blocking wait is the reason these are separate methods (cf. #270 for Seek).

Reset becomes PauseAndStopReading plus clearing the buffer, and is deprecated again in favor of Seek to reposition the source, or PauseAndStopReading to release it.

Implementation notes

  • readSourceToBuffer discards the result of an in-flight read whenever the state is playerPausedAndStopReading. That is correct for Seek, but would lose another buffer's worth of data for PauseAndStopReading. The discard should be keyed on a seek generation counter rather than on the state.
  • eof must be preserved by PauseAndStopReading. Reset clears it because it also drops the buffer.
  • Pause acts only when the state is playerPlay. PauseAndStopReading must also work from playerPaused, which is the Player still reads after Pause #288 case: a paused player keeps filling its buffer from the source.
  • The NewPlayer doc in context.go tells users to call Reset when they want to seek the source; it should point at Seek.

a6fe004 is not part of any tagged release (the newest tag is v3.5.0-alpha.10), so this can be corrected before v3.5.0.


Filed by Claude (Claude Code) on behalf of @hajimehoshi.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions