ryanheise / audio_service

Flutter plugin to play audio in the background while the screen is off.
797 stars 481 forks source link

[one-isolate] Pause from notification media player remove the player #701

Open BringTheChill opened 3 years ago

BringTheChill commented 3 years ago

Which API doesn't behave as documented, and how does it misbehave? If I have androidStopForegroundOnPause on true and I pause my audio player from notification media player the player is removed, and if I set androidStopForegroundOnPause on false, I can not swipe away or anything (even closing the app) to remove notification media player from my phone.

Minimal reproduction project https://github.com/BringTheChill/audio_service/tree/one-isolate/audio_service/example

To Reproduce (i.e. user steps, not code) Steps to reproduce the behavior:

  1. Click on 'Press to play'
  2. Press pause from notification media player

Expected behavior I expect 2 things: if androidStopForegroundOnPause is true then don't remove notification media player, and if it's false remove the notification media player by swiping away or when the app is closed.

Runtime Environment (please complete the following information if relevant):

Flutter SDK version

[√] Flutter (Channel stable, 2.2.0, on Microsoft Windows [Version 10.0.19042.985], locale en-US)
[√] Android toolchain - develop for Android devices (Android SDK version 30.0.3)
[√] Chrome - develop for the web
[√] Android Studio (version 4.1.0)
[√] IntelliJ IDEA Community Edition (version 2020.3)
[√] VS Code (version 1.55.2)
[√] Connected device (4 available)

• No issues found!
ryanheise commented 3 years ago

I think the Android and iOS sides of this might benefit from being treated as separate issues as the two platforms are completely different in their capabilities in some respects. This particular configuration is Android-specific, so I will focus in this issue on the Android side. I assume the notification disappears on pause only if your app is in the background?

My current working theory here is that Android is clearing the app from memory due to resource constraints. Basically, when you set the androidStopForegroundOnPause option to true, you are giving Android the right to remove your app at any time if it needs to reclaim resources. Why that's happening all the time I'm not completely certain of, but I can only imagine that the app is using a lot of memory. In that case, you would need to set the option to false to tell Android that you will not allow it to remove your app from memory.

I'm sorry I don't have any better working theories than this one at this stage.

BringTheChill commented 3 years ago

I fixed this issue replacing this package with assets_audio_player. Thank you for your support!

ryanheise commented 3 years ago

Reopening until the issue is resolved.

nt4f04uNd commented 3 years ago

Basically, when you set the androidStopForegroundOnPause option to true, you are giving Android the right to remove your app at any time if it needs to reclaim resources.

This is not accurate, it will be stopped each time service exits playing state, and this is expected

and if it's false remove the notification media player by swiping away or when the app is closed.

This is not possible, if foreground service is running, the notification is in place, and you can't remove it unless the service is stopped. It the main difference between service and foreground service. This is something that was recently outlined in docs for this parameter.

if androidStopForegroundOnPause is true then don't remove notification media player

This shouldn't happen, unfortunately, I'm not able to run 0.17.0 example on my phone, because it's too stale for current flutter master, but running example from one-isolate with the given configuration, I don't observe this behavior.

nt4f04uNd commented 3 years ago

@ryanheise could this be related https://github.com/ryanheise/audio_service/issues/367? Is that possible you fixed that along the way of one-isolate?

ryanheise commented 3 years ago

They are related, and the one-isolate branch is intended to solve it, but I still haven't got a good answer to the system destroying the service. Maybe I've made a programming mistake somewhere but I think what's going on here is that when you pause, the service should be stopped but not necessarily destroyed. Although the system ends up going that one step further and destroying it too, and then the notification gets removed. Looking at my implementation of onDestroy, I have this line:

        stopForeground(!config.androidResumeOnClick);

where if the parameter is true the notification will be removed. So here we want it to be false and therefore we want androidResumeOnClick to be true (which it is by default).

So I'm still not sure what is causing the notification to be removed.

nt4f04uNd commented 3 years ago

I think what's going on here is that when you pause, the service should be stopped but not necessarily destroyed

Actually, the service calls the stopForeground and exits foreground state, this doesn't stop the service. Service is stopped with stopSelf, and after that it's marked forever dead for the system, which is guaranteed (though onDestroy might be choosen to be not called by the system)

where if the parameter is true the notification will be removed. So here we want it to be false and therefore we want androidResumeOnClick to be true (which it is by default).

So I'm still not sure what is causing the notification to be removed.

Yep, this is strange

nt4f04uNd commented 3 years ago

Were you able to reproduce it?

ryanheise commented 3 years ago

I think what's going on here is that when you pause, the service should be stopped but not necessarily destroyed

Actually, the service calls the stopForeground and exits foreground state, this doesn't stop the service. Service is stopped with stopSelf, and after that it's marked forever dead for the system, which is guaranteed (though onDestroy might be choosen to be not called by the system)

Sorry I did get a bit mixed up there, yes that's right.

Were you able to reproduce it?

No, although possibly it's because I'm now running on Android 11. It may happen only on specific versions of Android or maybe even on specific phones with less memory, etc.

nt4f04uNd commented 3 years ago

Supposedly on the master this can cause this behavior https://github.com/ryanheise/audio_service/blob/b9c89df1f115c8e304af080afa8c8208c56f258f/android/src/main/java/com/ryanheise/audioservice/AudioService.java#L162

On the one-isolate these two lines

https://github.com/ryanheise/audio_service/blob/5e740fea75767cf82e07481d33b30111016fdd44/audio_service/android/src/main/java/com/ryanheise/audioservice/AudioService.java#L551-L552

kmod-midori commented 3 years ago

I think this might be leftovers when we were trying to solve the ghost notification problems on latest Android?

Though I do see similar behaviors (the notification disappears the instant playback is paused) even on native apps such as YouTube Music, probably due to resource constraints (problematic especially on MIUI).

ryanheise commented 3 years ago

I was also just looking into this now - the example provided above actually has other issues with one-isolate now because the processing state is always idle. In the latest code, that actually causes the service to be stopped.

I think this might be leftovers when we were trying to solve the ghost notification problems on latest Android?

Yes, but I think it's still desirable to keep this. Many users of the plugin have apps where they want to force the notification to be removed once they're finished playing - I just need to ensure it is implemented correctly with respect to androidStopForegroundOnPause and androidNotificationOngoing.

irufano commented 3 years ago

any update ? I have same issue

ryanheise commented 3 years ago

I did some testing but actually didn't reproduce it myself. A minimal reproduction project would be helpful.

kmod-midori commented 3 years ago

I only personally experience this on my phone when it is under high memory pressure, as the system kills the application the instant playback is stopped and the service is no longer foreground.

ryanheise commented 3 years ago

Some general advice would be to build Android 11 media session resumption feature into your app so that even if the process is killed, Android will keep the notification around.

nt4f04uNd commented 3 years ago

Would it make sense to add an additional parameter to control that behavior? It seems like it's a reasonable feature for devices below Android 11

ryanheise commented 3 years ago

I'd like to be able to reproduce the issue before experimenting with this, but there is already an androidNotificationOngoing option, and rather than add a new option, it may be better to just prevent cancelling the notification if this existing option is true.

nt4f04uNd commented 3 years ago

androidNotificationOngoing affects whether the notification can be dismissed by the user, but I'm not sure it should control whether it should control whether notification remains when service is stoped or destroyed by the system. For example someone might want the ongoing notification while service is working, but the don't want the keep the dead service notification, or someone might not want ongoing notification, but have the dead service notification.

ryanheise commented 3 years ago

You're right, I had that confused with the parameter to stopForeground.

kmod-midori commented 3 years ago

This might mot be very reproducible on Pixel devices (and other device that are closer to AOSP), but can be very annoying on Chinese devices with heavy restriction on background tasks (e.g. MIUI and EMUI)

nt4f04uNd commented 3 years ago

@chengyuhui can you provide a reproducible, as mentioned above, since it would greatly simplify determining what the actual issue is? I have a MIUI device and I would be able to test this.