Plugin.Maui.Audio icon indicating copy to clipboard operation
Plugin.Maui.Audio copied to clipboard

[Question] Properly dispose temporary player

Open ndegheselle opened this issue 1 year ago • 5 comments
trafficstars

Hello,

I want to add small sound effect then you press some buttons, notifications and so on. So I have a method in my MainPage that create a audio player and play the sound imediatly like in the samples :

    public async void PlaySound(EnumSound sound)
    {
        IAudioPlayer player = _audioManager.CreatePlayer(await FileSystem.OpenAppPackageFileAsync(_ressoucesSound[sound]));
        player.Play();
    }

But on Android this produce a warning [MediaPlayer-JNI] MediaPlayer finalized without being released, probably because the object is going out of scope before finishing preventing it from disposing, so I changed it like so :

    private List<IAudioPlayer> _players = [];
    public async void PlaySound(EnumSound sound)
    {
        IAudioPlayer player = _audioManager.CreatePlayer(await FileSystem.OpenAppPackageFileAsync(_ressoucesSound[sound]));
        _players.Add(player);
        player.PlaybackEnded += (s, e) =>
        {
            player.Dispose();
            _players.Remove(player);
        };
        player.Play();
    }

But then I got an error Cannot access a disposed object.. If I remove the player.Dispose(); everything is fine and I don't have the MediaPlayer finalized without being released warning anymore but I am not quite sure that the player is properly disposed since from the source code only Dispose(false) would be called (not so sure about this one) ?

So how would you cleanly dispose of a player that is used for only one short notification sound ?

ndegheselle avatar Jul 03 '24 14:07 ndegheselle

Hello,

I want to add small sound effect then you press some buttons, notifications and so on. So I have a method in my MainPage that create a audio player and play the sound imediatly like in the samples :

    public async void PlaySound(EnumSound sound)
    {
        IAudioPlayer player = _audioManager.CreatePlayer(await FileSystem.OpenAppPackageFileAsync(_ressoucesSound[sound]));
        player.Play();
    }

But on Android this produce a warning [MediaPlayer-JNI] MediaPlayer finalized without being released, probably because the object is going out of scope before finishing preventing it from disposing, so I changed it like so :

    private List<IAudioPlayer> _players = [];
    public async void PlaySound(EnumSound sound)
    {
        IAudioPlayer player = _audioManager.CreatePlayer(await FileSystem.OpenAppPackageFileAsync(_ressoucesSound[sound]));
        _players.Add(player);
        player.PlaybackEnded += (s, e) =>
        {
            player.Dispose();
            _players.Remove(player);
        };
        player.Play();
    }

But then I got an error Cannot access a disposed object.. If I remove the player.Dispose(); everything is fine and I don't have the MediaPlayer finalized without being released warning anymore but I am not quite sure that the player is properly disposed since from the source code only Dispose(false) would be called (not so sure about this one) ?

So how would you cleanly dispose of a player that is used for only one short notification sound ?

Is your error just order of operations?

            player.Dispose();
            _players.Remove(player);

I presume Cannot access a disposed object is from you trying to remove player from _players after it is disposed.

What about instead:

            _players.Remove(player);
            player.Dispose();

jonmdev avatar Aug 05 '24 08:08 jonmdev

Simply having this following code is enough to trigger Cannot access a disposed object.

player.PlaybackEnded += (s, e) =>
{
    player.Dispose();
};

In the source code, in the implementation of AudioPlayer for android there is:

	void OnPlaybackEnded(object? sender, EventArgs e)
	{
		PlaybackEnded?.Invoke(this, e);

		isPlaying = player.IsPlaying;

		//this improves stability on older devices but has minor performance impact
		// We need to check whether the player is null or not as the user might have dipsosed it in an event handler to PlaybackEnded above.
		if (!OperatingSystem.IsAndroidVersionAtLeast(23))
		{
			player.SeekTo(0);
			player.Stop();
			player.Prepare();
		}
	}

So it seems that some checks are missing there to handle disposing in the PlaybackEnded event (I will do a PR to fix that), other platforms don't seems to have this problem.

ndegheselle avatar Aug 05 '24 10:08 ndegheselle

This is an old PR and might not solve the issue however I was hoping to achieve a similar approach to playing sounds effects on button clicks through something like a behavior.

https://github.com/jfversluis/Plugin.Maui.Audio/pull/19

The idea would be that we could dispose of things when the behavior is detached by MAUI

bijington avatar Aug 05 '24 10:08 bijington

This is an old PR and might not solve the issue however I was hoping to achieve a similar approach to playing sounds effects on button clicks through something like a behavior.

#19

The idea would be that we could dispose of things when the behavior is detached by MAUI

If you are playing a sound that continue playing then you change the view (or whenever the behavior is detached) wont you have a similar problem ? OnPlaybackEnded would still be called after the Dispose call and throw the error Cannot access a disposed object.

I created the PR #122 to fix this kind of problems. In the end in my original code I reuse the players to avoid creating them each times. Still, having "temporary" players for sounds that are used once or very rarely seems a common use case

ndegheselle avatar Aug 05 '24 12:08 ndegheselle

This results in a memory leak when using multiple player instances over the time, because not being able to dispose them. Please release the fix in #122 immediately, so that we can use this plugin...

erdmenchen avatar Sep 06 '24 08:09 erdmenchen

Fix released

jfversluis avatar Apr 09 '25 12:04 jfversluis