diff --git a/audio/audio.go b/audio/audio.go index 8f5e65e..ded88b2 100644 --- a/audio/audio.go +++ b/audio/audio.go @@ -73,9 +73,17 @@ type Context struct { playingPlayers map[*playerImpl]struct{} m sync.Mutex - // cond is signalled under m when a player is added to playingPlayers. The goroutine - // updating the players waits on it while playingPlayers is empty. + // cond is signalled under m when a player is added to playingPlayers or when the context is + // disposed. The goroutine updating the players waits on it while playingPlayers is empty. cond *sync.Cond + + // disposed reports that the goroutine updating the players must exit. A context lives until the + // process exits, so this is set only when a test discards the context. + disposed bool + + // updater tracks the goroutine updating the players, so that a test discarding the context can wait + // for the goroutine to exit. + updater sync.WaitGroup } var ( @@ -162,12 +170,14 @@ func NewContext(sampleRate int) *Context { // In the current Ebitengine implementation, update might not be called when the window is in the background (#3154). // In this case, an audio player's position is not updated correctly with AppendHookOnBeforeUpdateWithVMGuestInfo. // Use a distinct goroutine to update the player states. - go func() { + c.updater.Go(func() { for { // Block while there is no player to update, so that an idle context does not wake // up periodically. The sweep itself can remove the last player, so the check is // repeated after every sweep. - c.waitForPlayingPlayers() + if !c.waitForPlayingPlayers() { + return + } // A failure is local to the player which reported it. Record it and keep updating // the other players (#3647). if err := c.updatePlayers(); err != nil { @@ -175,7 +185,7 @@ func NewContext(sampleRate int) *Context { } time.Sleep(time.Second / 100) } - }() + }) return c } @@ -212,13 +222,15 @@ func (c *Context) setReady() { c.m.Unlock() } -// waitForPlayingPlayers blocks until at least one player is registered as playing. -func (c *Context) waitForPlayingPlayers() { +// waitForPlayingPlayers blocks until at least one player is registered as playing, and reports +// whether the players should be updated. It returns false once the context is disposed. +func (c *Context) waitForPlayingPlayers() bool { c.m.Lock() defer c.m.Unlock() - for len(c.playingPlayers) == 0 { + for len(c.playingPlayers) == 0 && !c.disposed { c.cond.Wait() } + return !c.disposed } func (c *Context) addPlayingPlayer(p *playerImpl) { diff --git a/audio/audio_test.go b/audio/audio_test.go index f0ab5c8..8526f91 100644 --- a/audio/audio_test.go +++ b/audio/audio_test.go @@ -24,6 +24,7 @@ import ( "sync" "sync/atomic" "testing" + "testing/synctest" "time" "github.com/hajimehoshi/ebiten/v2/audio" @@ -42,38 +43,40 @@ func teardown() { // Issue #746 func TestGC(t *testing.T) { - setup() - defer teardown() + synctest.Test(t, func(t *testing.T) { + setup() + defer teardown() - p, _ := context.NewPlayer(bytes.NewReader(make([]byte, 4))) - got := audio.PlayersCountForTesting() - if want := 0; got != want { - t.Errorf("PlayersCountForTesting(): got: %d, want: %d", got, want) - } + p, _ := context.NewPlayer(bytes.NewReader(make([]byte, 4))) + got := audio.PlayersCountForTesting() + if want := 0; got != want { + t.Errorf("PlayersCountForTesting(): got: %d, want: %d", got, want) + } - p.Play() - got = audio.PlayersCountForTesting() - if want := 1; got != want { - t.Errorf("PlayersCountForTesting() after Play: got: %d, want: %d", got, want) - } - - runtime.KeepAlive(p) - p = nil - runtime.GC() - - for range 10 { + p.Play() got = audio.PlayersCountForTesting() - if want := 0; got == want { - return + if want := 1; got != want { + t.Errorf("PlayersCountForTesting() after Play: got: %d, want: %d", got, want) } - if err := audio.UpdateForTesting(); err != nil { - t.Error(err) + + runtime.KeepAlive(p) + p = nil + runtime.GC() + + for range 10 { + got = audio.PlayersCountForTesting() + if want := 0; got == want { + return + } + if err := audio.UpdateForTesting(); err != nil { + t.Error(err) + } + // 200[ms] should be enough for all the bytes to be consumed. The test runs on the fake + // clock of synctest, so this does not take real time. + time.Sleep(200 * time.Millisecond) } - // 200[ms] should be enough for all the bytes to be consumed. - // TODO: This is a dirty hack. Would it be possible to use virtual time? - time.Sleep(200 * time.Millisecond) - } - t.Errorf("time out") + t.Errorf("time out") + }) } type infiniteReader struct{} @@ -87,35 +90,37 @@ func (i *infiniteReader) Read(p []byte) (int, error) { // Issue #853 func TestSameSourcePlayers(t *testing.T) { - setup() - defer teardown() + synctest.Test(t, func(t *testing.T) { + setup() + defer teardown() - src := &infiniteReader{} - p0, err := context.NewPlayer(src) - if err != nil { - t.Fatal(err) - } - p1, err := context.NewPlayer(src) - if err != nil { - t.Fatal(err) - } - - // As the player does not play yet, error doesn't happen. - if err := audio.UpdateForTesting(); err != nil { - t.Error(err) - } - - p0.Play() - p1.Play() - - for range 10 { - if err := audio.UpdateForTesting(); err != nil { - // An error is expected. - return + src := &infiniteReader{} + p0, err := context.NewPlayer(src) + if err != nil { + t.Fatal(err) } - time.Sleep(200 * time.Millisecond) - } - t.Errorf("time out") + p1, err := context.NewPlayer(src) + if err != nil { + t.Fatal(err) + } + + // As the player does not play yet, error doesn't happen. + if err := audio.UpdateForTesting(); err != nil { + t.Error(err) + } + + p0.Play() + p1.Play() + + for range 10 { + if err := audio.UpdateForTesting(); err != nil { + // An error is expected. + return + } + time.Sleep(200 * time.Millisecond) + } + t.Errorf("time out") + }) } func TestPauseBeforeInit(t *testing.T) { @@ -166,53 +171,55 @@ func TestNonSeekableSource(t *testing.T) { // Issue #3438 func TestDeferredDeviceCreation(t *testing.T) { - setup() - defer teardown() + synctest.Test(t, func(t *testing.T) { + setup() + defer teardown() - p, err := context.NewPlayer(bytes.NewReader(make([]byte, 4))) - if err != nil { - t.Fatal(err) - } - - // Touching a player before the first update must not create the audio device. - p.SetVolume(0.5) - p.Play() - - if audio.ContextCreatedForTesting() { - t.Errorf("the audio device must not be created before the first update") - } - - // The state set before the device is created is recorded. - if got, want := p.Volume(), 0.5; got != want { - t.Errorf("Volume(): got: %v, want: %v", got, want) - } - if got, want := p.IsPlaying(), true; got != want { - t.Errorf("IsPlaying(): got: %t, want: %t", got, want) - } - if got, want := audio.PlayersCountForTesting(), 1; got != want { - t.Errorf("PlayersCountForTesting(): got: %d, want: %d", got, want) - } - - // The first update creates the audio device. - if err := audio.UpdateForTesting(); err != nil { - t.Error(err) - } - if !audio.ContextCreatedForTesting() { - t.Errorf("the audio device must be created after the first update") - } - - // The pending player eventually starts playing and finishes, which is only possible - // once the device is created and the player is materialized. - for range 10 { - if audio.PlayersCountForTesting() == 0 { - return + p, err := context.NewPlayer(bytes.NewReader(make([]byte, 4))) + if err != nil { + t.Fatal(err) } + + // Touching a player before the first update must not create the audio device. + p.SetVolume(0.5) + p.Play() + + if audio.ContextCreatedForTesting() { + t.Errorf("the audio device must not be created before the first update") + } + + // The state set before the device is created is recorded. + if got, want := p.Volume(), 0.5; got != want { + t.Errorf("Volume(): got: %v, want: %v", got, want) + } + if got, want := p.IsPlaying(), true; got != want { + t.Errorf("IsPlaying(): got: %t, want: %t", got, want) + } + if got, want := audio.PlayersCountForTesting(), 1; got != want { + t.Errorf("PlayersCountForTesting(): got: %d, want: %d", got, want) + } + + // The first update creates the audio device. if err := audio.UpdateForTesting(); err != nil { t.Error(err) } - time.Sleep(200 * time.Millisecond) - } - t.Errorf("time out") + if !audio.ContextCreatedForTesting() { + t.Errorf("the audio device must be created after the first update") + } + + // The pending player eventually starts playing and finishes, which is only possible + // once the device is created and the player is materialized. + for range 10 { + if audio.PlayersCountForTesting() == 0 { + return + } + if err := audio.UpdateForTesting(); err != nil { + t.Error(err) + } + time.Sleep(200 * time.Millisecond) + } + t.Errorf("time out") + }) } // Issue #3438 @@ -450,43 +457,45 @@ func TestPauseAndStopReading(t *testing.T) { t.Skip("infinite streams in tests cannot be treated well on browsers") } - setup() - defer teardown() + synctest.Test(t, func(t *testing.T) { + setup() + defer teardown() - src := &countingSource{} - p, err := context.NewPlayerF32(src) - if err != nil { - t.Fatal(err) - } + src := &countingSource{} + p, err := context.NewPlayerF32(src) + if err != nil { + t.Fatal(err) + } - p.Play() + p.Play() - // The first update creates the audio device and starts the pending player. - if err := audio.UpdateForTesting(); err != nil { - t.Error(err) - } + // The first update creates the audio device and starts the pending player. + if err := audio.UpdateForTesting(); err != nil { + t.Error(err) + } - waitForRead(t, src, 0) + waitForRead(t, src, 0) - p.PauseAndStopReading() + p.PauseAndStopReading() - if p.IsPlaying() { - t.Error("IsPlaying() after PauseAndStopReading: got: true, want: false") - } - if got, want := audio.PlayersCountForTesting(), 0; got != want { - t.Errorf("PlayersCountForTesting() after PauseAndStopReading: got: %d, want: %d", got, want) - } + if p.IsPlaying() { + t.Error("IsPlaying() after PauseAndStopReading: got: true, want: false") + } + if got, want := audio.PlayersCountForTesting(), 0; got != want { + t.Errorf("PlayersCountForTesting() after PauseAndStopReading: got: %d, want: %d", got, want) + } - // After PauseAndStopReading, the source must not be read. - reads := src.reads.Load() - time.Sleep(100 * time.Millisecond) - if got := src.reads.Load(); got != reads { - t.Errorf("the source was read after PauseAndStopReading: got: %d reads, want: %d", got, reads) - } + // After PauseAndStopReading, the source must not be read. + reads := src.reads.Load() + time.Sleep(100 * time.Millisecond) + if got := src.reads.Load(); got != reads { + t.Errorf("the source was read after PauseAndStopReading: got: %d reads, want: %d", got, reads) + } - // The player is reusable after PauseAndStopReading. - p.Play() - waitForRead(t, src, reads) + // The player is reusable after PauseAndStopReading. + p.Play() + waitForRead(t, src, reads) + }) } // Issue #3510 @@ -495,35 +504,37 @@ func TestPauseAndStopReadingWhilePaused(t *testing.T) { t.Skip("infinite streams in tests cannot be treated well on browsers") } - setup() - defer teardown() + synctest.Test(t, func(t *testing.T) { + setup() + defer teardown() - src := &countingSource{} - p, err := context.NewPlayerF32(src) - if err != nil { - t.Fatal(err) - } + src := &countingSource{} + p, err := context.NewPlayerF32(src) + if err != nil { + t.Fatal(err) + } - p.Play() + p.Play() - // The first update creates the audio device and starts the pending player. - if err := audio.UpdateForTesting(); err != nil { - t.Error(err) - } + // The first update creates the audio device and starts the pending player. + if err := audio.UpdateForTesting(); err != nil { + t.Error(err) + } - reads := waitForRead(t, src, 0) + reads := waitForRead(t, src, 0) - // A paused player keeps reading the source to fill its buffer. - p.Pause() - waitForRead(t, src, reads) + // A paused player keeps reading the source to fill its buffer. + p.Pause() + waitForRead(t, src, reads) - // PauseAndStopReading must stop the reads even though the player is already paused. - p.PauseAndStopReading() - reads = src.reads.Load() - time.Sleep(100 * time.Millisecond) - if got := src.reads.Load(); got != reads { - t.Errorf("the source was read after PauseAndStopReading: got: %d reads, want: %d", got, reads) - } + // PauseAndStopReading must stop the reads even though the player is already paused. + p.PauseAndStopReading() + reads = src.reads.Load() + time.Sleep(100 * time.Millisecond) + if got := src.reads.Load(); got != reads { + t.Errorf("the source was read after PauseAndStopReading: got: %d reads, want: %d", got, reads) + } + }) } // Issue #3510 @@ -635,51 +646,53 @@ func TestSetBufferSize(t *testing.T) { } func TestPositionNotGrowingAfterFinished(t *testing.T) { - setup() - defer teardown() + synctest.Test(t, func(t *testing.T) { + setup() + defer teardown() - // 44100 [Hz] * 8 [bytes/sample] is one second of 32bit float stereo audio. - src := bytes.NewReader(make([]byte, 44100*8*4)) - p, err := context.NewPlayerF32(src) - if err != nil { - t.Fatal(err) - } - - p.Play() - // Wait until the player finishes its source. The deadline is generous because time.Sleep has - // a several-millisecond floor on browsers, which slows draining the source down. - var finished bool - deadline := time.Now().Add(10 * time.Second) - for time.Now().Before(deadline) { - if err := audio.UpdateForTesting(); err != nil { + // 44100 [Hz] * 8 [bytes/sample] is one second of 32bit float stereo audio. + src := bytes.NewReader(make([]byte, 44100*8*4)) + p, err := context.NewPlayerF32(src) + if err != nil { t.Fatal(err) } - if !p.IsPlaying() { - finished = true - break - } - time.Sleep(5 * time.Millisecond) - } - if !finished { - t.Fatal("time out: the player did not finish") - } - end := p.Position() - if got, want := end, 4*time.Second; got < want { - t.Errorf("Position() after the player finished: got: %v, want: at least %v", got, want) - } - - // Play on a finished player does nothing, so the position must not grow anymore. - for range 20 { p.Play() - if err := audio.UpdateForTesting(); err != nil { - t.Fatal(err) + // Wait until the player finishes its source. The test runs on the fake clock of synctest, so + // the deadline does not take real time. + var finished bool + deadline := time.Now().Add(10 * time.Second) + for time.Now().Before(deadline) { + if err := audio.UpdateForTesting(); err != nil { + t.Fatal(err) + } + if !p.IsPlaying() { + finished = true + break + } + time.Sleep(5 * time.Millisecond) } - time.Sleep(5 * time.Millisecond) - } - if got := p.Position(); got != end { - t.Errorf("position grew after the player finished: %v -> %v", end, got) - } + if !finished { + t.Fatal("time out: the player did not finish") + } + + end := p.Position() + if got, want := end, 4*time.Second; got < want { + t.Errorf("Position() after the player finished: got: %v, want: at least %v", got, want) + } + + // Play on a finished player does nothing, so the position must not grow anymore. + for range 20 { + p.Play() + if err := audio.UpdateForTesting(); err != nil { + t.Fatal(err) + } + time.Sleep(5 * time.Millisecond) + } + if got := p.Position(); got != end { + t.Errorf("position grew after the player finished: %v -> %v", end, got) + } + }) } func TestNewPlayerWithSourceOfClosedPlayer(t *testing.T) { @@ -797,70 +810,72 @@ func TestPlayerErrorDoesNotStopOtherPlayers(t *testing.T) { t.Skip("infinite streams in tests cannot be treated well on browsers") } - setup() - defer teardown() + synctest.Test(t, func(t *testing.T) { + setup() + defer teardown() - // The first update creates the audio device, so that the players below play for real. - if err := audio.UpdateForTesting(); err != nil { - t.Fatal(err) - } - - failing, err := context.NewPlayerF32(&failingSource{remaining: 4096 * 4}) - if err != nil { - t.Fatal(err) - } - // 44100 [Hz] * 8 [bytes/sample] is one second of 32bit float stereo audio. - healthy, err := context.NewPlayerF32(audio.NewInfiniteLoopF32(bytes.NewReader(make([]byte, 44100*8)), 44100*8)) - if err != nil { - t.Fatal(err) - } - - failing.Play() - healthy.Play() - - // Wait until the failing player's source fails and the failure is reported as the context - // error. - var failed bool - deadline := time.Now().Add(5 * time.Second) - for time.Now().Before(deadline) { + // The first update creates the audio device, so that the players below play for real. if err := audio.UpdateForTesting(); err != nil { - if !errors.Is(err, errSourceFailed) { - t.Fatal(err) - } - failed = true - break + t.Fatal(err) } - time.Sleep(5 * time.Millisecond) - } - if !failed { - t.Fatal("time out: the failing player's error was not reported") - } - // The failed player is no longer tracked as playing, so that its error is reported once and - // the healthy player is the only playing player left. - deadline = time.Now().Add(time.Second) - for audio.PlayersCountForTesting() != 1 && time.Now().Before(deadline) { - time.Sleep(5 * time.Millisecond) - } - if got, want := audio.PlayersCountForTesting(), 1; got != want { - t.Errorf("PlayersCountForTesting() after a player failed: got: %d, want: %d", got, want) - } + failing, err := context.NewPlayerF32(&failingSource{remaining: 4096 * 4}) + if err != nil { + t.Fatal(err) + } + // 44100 [Hz] * 8 [bytes/sample] is one second of 32bit float stereo audio. + healthy, err := context.NewPlayerF32(audio.NewInfiniteLoopF32(bytes.NewReader(make([]byte, 44100*8)), 44100*8)) + if err != nil { + t.Fatal(err) + } - // The healthy player must keep playing and its position must keep advancing: one player's - // failure must not stop the context from updating the other players. - if !healthy.IsPlaying() { - t.Error("IsPlaying() of the healthy player after another player failed: got: false, want: true") - } - pos0 := healthy.Position() - time.Sleep(100 * time.Millisecond) - if pos1 := healthy.Position(); pos1 <= pos0 { - t.Errorf("Position() of the healthy player did not advance after another player failed: %v -> %v", pos0, pos1) - } + failing.Play() + healthy.Play() - // The context error is sticky. - if err := audio.UpdateForTesting(); !errors.Is(err, errSourceFailed) { - t.Errorf("UpdateForTesting() after a player failed: got: %v, want: %v", err, errSourceFailed) - } + // Wait until the failing player's source fails and the failure is reported as the context + // error. + var failed bool + deadline := time.Now().Add(5 * time.Second) + for time.Now().Before(deadline) { + if err := audio.UpdateForTesting(); err != nil { + if !errors.Is(err, errSourceFailed) { + t.Fatal(err) + } + failed = true + break + } + time.Sleep(5 * time.Millisecond) + } + if !failed { + t.Fatal("time out: the failing player's error was not reported") + } + + // The failed player is no longer tracked as playing, so that its error is reported once and + // the healthy player is the only playing player left. + deadline = time.Now().Add(time.Second) + for audio.PlayersCountForTesting() != 1 && time.Now().Before(deadline) { + time.Sleep(5 * time.Millisecond) + } + if got, want := audio.PlayersCountForTesting(), 1; got != want { + t.Errorf("PlayersCountForTesting() after a player failed: got: %d, want: %d", got, want) + } + + // The healthy player must keep playing and its position must keep advancing: one player's + // failure must not stop the context from updating the other players. + if !healthy.IsPlaying() { + t.Error("IsPlaying() of the healthy player after another player failed: got: false, want: true") + } + pos0 := healthy.Position() + time.Sleep(100 * time.Millisecond) + if pos1 := healthy.Position(); pos1 <= pos0 { + t.Errorf("Position() of the healthy player did not advance after another player failed: %v -> %v", pos0, pos1) + } + + // The context error is sticky. + if err := audio.UpdateForTesting(); !errors.Is(err, errSourceFailed) { + t.Errorf("UpdateForTesting() after a player failed: got: %v, want: %v", err, errSourceFailed) + } + }) } func TestPlayOnIdleContext(t *testing.T) { diff --git a/audio/export_test.go b/audio/export_test.go index 654df73..e2638ee 100644 --- a/audio/export_test.go +++ b/audio/export_test.go @@ -27,6 +27,10 @@ type ( suspendErr error resumeErr error + // players are the players created since the context was reset last. ResetContextForTesting + // stops them, so that a player reading an infinite source does not keep reading after its test. + players []*dummyPlayer + mu sync.Mutex } dummyPlayer struct { @@ -45,6 +49,9 @@ type ( // readGen is incremented by PauseAndStopReading to stop the goroutine reading r. readGen int + // readers tracks the goroutines reading r, so that a test can wait for them to stop. + readers sync.WaitGroup + // err is the first non-EOF error the source returned. Like the real players, a player // whose source failed stops for good and reports the error from Err. err error @@ -54,10 +61,27 @@ type ( ) func (c *dummyContext) NewPlayer(r io.Reader) player { - return &dummyPlayer{ + p := &dummyPlayer{ r: r, volume: 1, } + c.mu.Lock() + defer c.mu.Unlock() + c.players = append(c.players, p) + return p +} + +// stopPlayers stops the players' goroutines reading their sources, and waits for them to exit. +func (c *dummyContext) stopPlayers() { + c.mu.Lock() + players := c.players + c.players = nil + c.mu.Unlock() + + for _, p := range players { + p.PauseAndStopReading() + p.readers.Wait() + } } func (c *dummyContext) MaxBufferSize() int { @@ -95,7 +119,7 @@ func (p *dummyPlayer) Play() { } p.playing = true gen := p.readGen - go func() { + p.readers.Go(func() { var buf [4096]byte for { stopped, err := p.readOnce(gen, buf[:]) @@ -114,7 +138,7 @@ func (p *dummyPlayer) Play() { if p.playing { p.eof = true } - }() + }) } // readOnce performs one read from the source with the mutex held, so that PauseAndStopReading waits @@ -324,10 +348,24 @@ func ContextCreatedForTesting() bool { return c.playerFactory.currentContext() != nil } +// ResetContextForTesting discards the current context, so that a test can create a new one. +// ResetContextForTesting waits for the goroutines the context and its players started to exit, so +// that they do not outlive the test. func ResetContextForTesting() { theContextLock.Lock() - defer theContextLock.Unlock() + c := theContext theContext = nil + theContextLock.Unlock() + + if c != nil { + c.m.Lock() + c.disposed = true + c.cond.Broadcast() + c.m.Unlock() + c.updater.Wait() + } + + dummyContextForTesting.stopPlayers() } func (i *InfiniteLoop) SetNoBlendForTesting(value bool) {