androidx / media

Jetpack Media3 support libraries for media use cases, including ExoPlayer, an extensible media player for Android
Apache License 2.0
1.37k stars 322 forks source link

Swiping away demo-session while playing does not end process #1363

Open TomVarga opened 1 month ago

TomVarga commented 1 month ago

Version

Media3 main branch

More version details

753f607a81287b798863a675b54cb8196665d95f

1.3.1

Devices that reproduce the issue

API 33 emulator

Devices that do not reproduce the issue

No response

Reproducible in the demo app?

Yes

Reproduction steps

Add the following to: DemoPlaybackService

    override fun onTaskRemoved(rootIntent: Intent?) {
        Log.d("DemoPlaybackService", "onTaskRemoved: ")
        val player = mediaLibrarySession.player
        if (player.playWhenReady) {
            // Make sure the service is not in foreground.
            player.pause()
        }
        stopSelf()
    }

Play audio from the demo-session app Background the app Swipe away the app

video showing both pause and swipe away, and just swipe away and pause programatically:

https://github.com/androidx/media/assets/3092675/e8b3784c-0eee-4040-a43d-aaa3cda7218a

logcat logs: process-not-ended.log

adb bugreport will be mailed

Expected result

just like when you pause playback in the background manually before swiping away the process should end

Actual result

now that the pause is done programmatically when swiping away the process does not end

This means that singletons are not cleaned up between app starts and therefore states held by it leak.

I have my changes at https://github.com/TomVarga/media/tree/disabled-playback-resumption-pause-on-taks-removed

I have created a helper class to showcase the problem: object SingletonValueHolder

which has it's value set to true in PlayerActivity#initializeController and the value is printed out in MainActivity#onCreate

from the logs you can see that if I manually pause playback while in the background, then swipe away the app, then the next time I open the app the value correctly resets to false as the process ended, and a new singleton was created

However if I don't pause playback manually, but rely on onTaskRemoved to do the pause programmatically as suggested in the docs https://developer.android.com/media/media3/session/background-playback#service-lifecycle then the process is not stopped, the singleton lives alonge and next time I start the app, the value remains true as it can be seen from the printout of it at MainActivity#onCreate

If this is is not enough to stop playback and release the process, then what else needs to be done?

Media

example: Jazz & Blues in the demo-session sample app

Bug Report

TomVarga commented 1 month ago

I think this is somewhat related to https://github.com/androidx/media/issues/1276 and https://github.com/androidx/media/issues/1059 but I think it's fundamentally merits it's own issue opened.

TomVarga commented 1 month ago

Could you please help me understand what else is needed besides what's written in the docs at https://developer.android.com/media/media3/session/background-playback#service-lifecycle:

    override fun onTaskRemoved(rootIntent: Intent?) {
        Log.d("DemoPlaybackService", "onTaskRemoved: ")
        val player = mediaLibrarySession.player
        if (player.playWhenReady) {
            // Make sure the service is not in foreground.
            player.pause()
        }
        stopSelf()
    }

so that when while playing if we swipe away the app then the process ends, like it'd when you manually pause before swiping away.

marcbaechinger commented 1 month ago

This works for me when I try with the demo app on API 33.

In your bugreport it seems to work as well. At least there isn't a notion from ActivityManager to restarted a crashed service as it would happen if the service isn't terminated properly in started mode.

From the logs you sent I see these lines when grepping for ActvityManager, DemoPlaybackService and ExoPlayerImpl:

05-10 14:10:10.098  1000   561   645 D ActivityManager: freezing 30188 com.google.android.settings.intelligence
05-10 14:10:23.082  1000   561   590 I ActivityManager: Start proc 3194:androidx.media3.demo.session/u0a163 for next-top-activity {androidx.media3.demo.session/androidx.media3.demo.session.MainActivity}
05-10 14:10:23.416 10163  3194  3194 I ExoPlayerImpl: Init 1570f43 [AndroidXMedia3/1.4.0-alpha01] [emu64a, sdk_gphone64_arm64, Google, 33]
05-10 14:10:33.981 10163  3194  3194 D DemoPlaybackService: onTaskRemoved: 
05-10 14:10:33.981 10163  3194  3194 D DemoPlaybackService: onDestroy: 
05-10 14:10:33.983 10163  3194  3194 I ExoPlayerImpl: Release 1570f43 [AndroidXMedia3/1.4.0-alpha01] [emu64a, sdk_gphone64_arm64, Google, 33] [media3.common, media3.exoplayer, media3.decoder, media3.session, media3.datasource, media3.ui, media3.extractor]
05-10 14:10:33.997  1000   561   574 I ActivityManager: Killing 3194:androidx.media3.demo.session/u0a163 (adj 1001): remove task
05-10 14:10:34.015  1000   561   581 W ActivityManager: setHasOverlayUi called on unknown pid: 3194

05-10 14:10:36.236  1000   561   590 I ActivityManager: Start proc 3270:androidx.media3.demo.session/u0a163 for next-top-activity {androidx.media3.demo.session/androidx.media3.demo.session.MainActivity}
05-10 14:10:36.441 10163  3270  3270 W a3.demo.session: Accessing hidden field Landroid/app/ActivityManager;->IActivityManagerSingleton:Landroid/util/Singleton; (unsupported, reflection, allowed)
05-10 14:10:36.538 10163  3270  3270 I ExoPlayerImpl: Init f2387fd [AndroidXMedia3/1.4.0-alpha01] [emu64a, sdk_gphone64_arm64, Google, 33]
05-10 14:10:40.794  1000   561   645 D ActivityManager: freezing 23145 com.google.android.gms
05-10 14:10:47.230 10163  3270  3270 D DemoPlaybackService: onTaskRemoved: 
05-10 14:10:47.242 10163  3270  3270 D DemoPlaybackService: onDestroy: 
05-10 14:10:47.243 10163  3270  3270 I ExoPlayerImpl: Release f2387fd [AndroidXMedia3/1.4.0-alpha01] [emu64a, sdk_gphone64_arm64, Google, 33] [media3.common, media3.exoplayer, media3.decoder, media3.session, media3.datasource, media3.ui, media3.extractor]

This seems working as intended and properly starts and stops the service twice once with process id 3194 and then with 3270. It's evident here that the player is initialized and released from the same process ID which is different on Android 14 where we have the problem described in the other bug.

Please let us know how you evaluate and why you think that the process is not stopped.

Please note that I can not advise on using a DI framework due to me not being familiar with that kind of thing.

TomVarga commented 1 month ago

I think it got confusing that I included two scenarios in the logs, let me just try and show the actual issue:

What I am missing in these unfiltered logcat logs is: ---------------------------- PROCESS ENDED (PID) for package androidx.media3.demo.session ---------------------------- process-not-ended-2.log

you can see in the logs two lines with: SingletonValueHolder D onCreate: one correctly has someValue false 15:37:58.304 28858-28858 SingletonValueHolder D onCreate: someValue false but the other is someValue true 15:38:06.236 28858-28858 SingletonValueHolder D onCreate: someValue true

in between the two events there is: 15:38:04.587 28858-28858 DemoPlaybackService D onTaskRemoved: and also 15:38:04.653 28858-28858 ExoPlayerImpl I Release 1570f43 [AndroidXMedia3/1.4.0-alpha01] [emu64a, sdk_gphone64_arm64, Google, 33] [media3.common, media3.exoplayer, media3.decoder, media3.session, media3.datasource, media3.ui, media3.extractor]

but there is no PROCESS ENDED

so the process is not cleaned up, that is the problem. So the question remains, what has to be done so that the proces gets cleaned up properly? (Remember, It gets cleaned up properly if I paused from the notification manually)

Additionally this has nothing to do with DI framework, I am talking about a static singleton in kotlin an object please see: https://github.com/TomVarga/media/commit/58f81635bf0746aebf98f3b4af67f9cefdfe1caf#diff-6e2c15851ce6e194378fb9b1f3bbdb34d28605c274633dfd30afc98584bc2a53

marcbaechinger commented 1 month ago

Thanks for clarifying!

Yeah, not sure what is missing. I can repro this on API 33 and API 29. Needs investigation. Thanks for reporting.