Race condition when playing/pausing audio

[filiph]

Checklist

• I read the troubleshooting guide before raising this issue
• I made sure that the issue I am raising doesn't already exist

Current bug behaviour

When AudioPlayer.pause() is called a short time after AudioPlayers.play() is called, the pause will not take effect. The sound file will play in its entirety.

Same goes for AudioPlayer.stop() instead of .pause().

Expected behaviour

When pause() is the latest call to an AudioPlayer, the audio doesn't play.

Steps to reproduce

1. Execute flutter run on the code sample
2. Play any sound from the Sources tab. You'd expect the audio won't play since it's paused 5 milliseconds after being played. But in fact, it plays to completion.

Code sample

Code sample

Go to the official example's lib/tabs/sources.dart file and replace the _play() method with the following:

  Future<void> _play(Source source) async {
    await player.stop();
    Timer(const Duration(milliseconds: 5), () {
      // Imagine this is a handler to some event that 
      // happens independently.
      player.pause();
    });
    await player.play(source);
    toast(
      'Set and playing source.',
      textKey: const Key('toast-set-play'),
    );
  }

Affected platforms

Android, iOS, web, Windows, Linux, macOS

Platform details

No response

AudioPlayers Version

main, 5.2.0

Build mode

debug

Audio Files/URLs/Sources

No response

Screenshots

No response

Logs

No response

Related issues / more information

Note that everything works as expected if you do something like:

await player.play(source);
await player.pause();

But in an app or game, events come from different sources. For example, settings can be updated (read from file) on startup, at almost the exact same moment as music is started. This can lead to the following:

1. Game starts
2. Game asks for settings (async)
3. Music starts (because that's the default) --- play(...)
4. Settings are finished reading from something like shared_preferences, and they say "no audio"
5. Music is immediately asked to stop --- stop()
6. Music continues to play indefinitely because the stop() finishes before the play() can get to the resume() step

FWIW, I couldn't replicate the behavior when using resume() (i.e. the source is already loaded). So I think the problem stems from the steps included in the play() method, which is not atomic.

Related Discord discussion: https://discord.com/channels/509714518008528896/533299043686940692/1168866344742162463

Working on PR

no way

[Gustl22]

Despite my arguments on Discord (no network, error, etc.) I think it's more consistent to set the state at the beginning before any async processing is happening (as you proposed). If an error occurs the the playing state should be reset. Tests for both of the cases should be included, if possible.

[Gustl22]

FWIW, I couldn't replicate the behavior when using resume() (i.e. the source is already loaded). So I think the problem stems from the steps included in the play() method, which is not atomic.

Now that I think about: setting the source simply takes much longer than just doing the resume, so pause is called before finishing setting the source in the event loop. So it might not even help to move setting to playing state at the beginning of the resume method.
So we may introduce another variable, which stores the intention to play and check it if it's still valid right before the native resume call.

[filiph]

That works for me, thanks!

FWIW, my current workaround is something like this:

  Future<void> _playCurrentSongInPlaylist() async {
    _log.info(() => 'Playing ${_playlist.first} now.');
    try {
      await _musicPlayer.play(AssetSource('music/${_playlist.first.filename}'));
    } catch (e) {
      _log.severe('Could not play song ${_playlist.first}', e);
    }

    // Settings can change while the music player is preparing
    // to play a song (i.e. during the `await` above).
    if (!_settings!.audioOn.value || !_settings!.musicOn.value) {
      try {
        _log.fine('Settings changed while preparing to play song. '
            'Pausing music.');
        await _musicPlayer.pause();
      } catch (e) {
        _log.severe('Could not pause music player', e);
      }
    }
  }

[Gustl22]

@filiph can you confirm that #1705 fixes your issue? Thx ;D

Edit: As a note, one can also avoid this issue by using setSource and resume separately instead of play, which also avoids setting the source each time for a single player.

[filiph]

I'm not able to run this at this time to confirm, but looking at the code and at the test — yes, this will solve my issue.