NewPipe Create media session UI and fix player notification

What is it?

[x] Bugfix (user facing)
[ ] Feature (user facing)
[x] Codebase improvement (dev facing)
[ ] Meta improvement to the project (dev facing)

Description of the changes in your PR

This PR solves one of the points in #8616: it introduces MediaSessionPlayerUi, moving more code out of the Player, and fixes various serious issues related to the player notification and the media session. I tried to put as much info as possible in commit messages and comments, so that it is simpler to debug and git blame.

Some useless classes were removed
- MediaSessionCallback and PlayerMediaSession was just a wrapper for some player functions (half of which used in one place, half in another), but it didn't make any sense not to use the player instance directly bca9543a54b04035a33227d65d6bf6be8dcc1672
- All of MediaSessionManager was moved in MediaSessionPlayerUi 49be75e6a00240480c1b40c596007c35dd472f5b
Make sure to trigger a metadata update after the thumbnail finishes loading, otherwise it ~~could happen~~ almost always happened that the new thumbnail would never be shown. 80df363d11a612826dd4e3e9f71fa00ee62d6548
Once a new video info starts loading in the player, the thumbnail of the previous video info is unset, so that any metadata update would just get (correctly) a null thumbnail, until the new thumbnail loading finishes. The placeholder thumbnail is not used anymore, as it makes more sense to just not set a thumbnail. 25f40a63c92c086ba3de2fe7596f8d86a3a7a65f
Since the builder used to create player notifications is reused, now the thumbnail is set to null when the user disables all thumbnails from settings while the notification is open (this requires a notification update to kick in), otherwise the lastly set thumbnail would be incorrectly shown. c9c55b10803cc7662ae2619e6b226b35f4adaaf7
Let exoplayer decide when to update metadata, instead of deciding ourselves. This seems to fix the wrong seekbar properties (duration and current position) being shown in the notification sometimes. MediaSessionConnector.setMetadataDeduplicationEnabled(true) was used to keep the previous behavior of not updating metadata if nothing changed from the already-set metadata. Now the number of updates is a little bit higher (tested by setting "noisy non-suspending breakpoints"), but there is no random update while playing or with other actions that do not change the current stream info, which would drain battery. E.g. 7 metadata updates instead of 2 when the player is started, and 3 instead of 2 when a new stream info plays. (740ab8af0cd2a8a57f800923d37ebf39f09ef8f8) and most importantly d0677bbb8c8b1ed3ab057817a56dbec47fb63489
Keep strong reference to Picasso thumbnail loading target. Before the Target would sometimes be garbage collected before being called with the loaded thumbnail, since Picasso holds weak references to targets. Also see #8677 for a similar change. e9e9e8096ca6838a259250ed0f83d3c986dc64b2

You can test changes to the media session by installing this neat media session testing app. Some of the Tests provided there are unsuccessful, but I don't know why since the actions they test are actually performed correctly (I included this sentence just so you know, as fixing the tests, which are related to player actions, is out of the scope of this PR).

This is related to #7166, so I'd like @litetex to review.

Before/After Screenshots/Screen Record

I am not providing screenshots of all of the various issues there were before, since they take a while to reproduce and if you have used NewPipe for a while you know about them by now.

Fixes the following issue(s)

Strangely enough I could not find any... Maybe somebody of you has a clue?

APK testing

The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR.

Due diligence

[x] I read the contribution guidelines.

Jul 22 '22 13:07 Stypox

I applied two review suggestions and rebased.

there are build failures

Not a problem in this PR, see #8679

Jul 26 '22 14:07 Stypox

new thumbnail would never be shown

wrong seekbar properties (duration and current position) being shown in the notification sometimes

Anything else apart from these 2 issues?

since they take a while to reproduce and if you have used NewPipe for a while you know about them by now.

Well, yes, but we wouldn't know which ones to look for between the current release and this PR.

Gotta test whatever we can, I guess.

Jul 26 '22 19:07 opusforlife2

In the middle of a video, I suddenly got a buffering indicator for ~half a second, and then the app completely crashed, with no error caught by Newpipe. Scoop's crash report:

FATAL EXCEPTION: main
Process: org.schabi.newpipe.debug.mediasessionui, PID: 3279
java.lang.NullPointerException: Attempt to invoke virtual method 'org.schabi.newpipe.player.ui.PlayerUiList org.schabi.newpipe.player.Player.UIs()' on a null object reference
	at org.schabi.newpipe.fragments.detail.VideoDetailFragment.lambda$addVideoPlayerView$19$org-schabi-newpipe-fragments-detail-VideoDetailFragment(VideoDetailFragment.java:1314)
	at org.schabi.newpipe.fragments.detail.VideoDetailFragment$$ExternalSyntheticLambda9.run(Unknown Source:2)
	at android.os.Handler.handleCallback(Handler.java:883)
	at android.os.Handler.dispatchMessage(Handler.java:100)
	at android.os.Looper.loop(Looper.java:214)
	at android.app.ActivityThread.main(ActivityThread.java:7356)
	at java.lang.reflect.Method.invoke(Native Method)
	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:491)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:940)

Jul 31 '22 19:07 opusforlife2

However you may address opus comment above before merging.

@opusforlife2's problem is unrelated to this PR. It is instead another instance of #8661. ~~I'm gonna open yet another PR...~~ Actually, I'm including the fix in #8731. _{Which problems does your phone have with that little piece of code? :joy:}

And btw you may also cleanup your fork-repo, there are a ton of unused branches there which are fetched by Git when I check it out :/

@litetex Ok, I'll do it, I never cared about old branches :sweat_smile:. But did you know you don't need to check my fork-repo out? You can just add this to your ~/.gitconfig:

[alias]
    pr = "!_() { git fetch upstream pull/\"$1\"/head && git checkout -b pr\"$1\" FETCH_HEAD; }; _"
    pr-f = "!_() { git checkout master; git branch -D pr\"$1\"; git fetch upstream pull/\"$1\"/head && git checkout -b pr\"$1\" FETCH_HEAD ; }; _"

And then you can do git pr 8678 to switch to the branch pr8678 containing this PR's code (pr-f is for force-fetching). If you then want to push to this branch you can do git push [email protected]:Stypox/NewPipe.git pr8678:media-session-ui (a little more cumbersome but still doable fast).

Aug 04 '22 09:08 Stypox

Which problems does your phone have with that little piece of code? 😂

Here's possibly another. <3

Rough STR (might not work): ~~Opened a video, it didn't load. Realised internet was off. Turned it on. Tried to close and reopen the video details screen. <- crash occurred somewhere here.~~ Happened again. Not network-related. Actual STR:

Have "Start in full screen" toggled on, Autoplay toggled off, and rotation locked to portrait. (Not sure if needed, but that's my setup.)
Open a video details page.
ASAP, before the video details can load, tap the thumbnail area. The player will go into full screen. (BTW, can it be made so that the "thumbnail area" cannot be tapped until the thumbnail itself has loaded, along with video details? I think that would fix a lot of bugs.)
Before it can start playing (while it is still fetching the streams), tap Back ASAP to rotate back to portrait, and then close the minimised player.

Tested on 2 devices, so fully reproducible. Doesn't occur on 0.23.1.

Exception

User Action: ui error
Request: ACRA report
Content Country: CH
Content Language: en-IN
App Language: en_IN
Service: none
Version: 0.23.1
OS: Linux Android 12 - 32

Crash log

android.app.RemoteServiceException$ForegroundServiceDidNotStartInTimeException: Context.startForegroundService() did not then call Service.startForeground(): ServiceRecord{2fc9070 u0 org.schabi.newpipe.debug.mediasessionui/org.schabi.newpipe.player.PlayerService}
	at android.app.ActivityThread.generateForegroundServiceDidNotStartInTimeException(ActivityThread.java:1965)
	at android.app.ActivityThread.throwRemoteServiceException(ActivityThread.java:1936)
	at android.app.ActivityThread.access$2700(ActivityThread.java:256)
	at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2190)
	at android.os.Handler.dispatchMessage(Handler.java:106)
	at android.os.Looper.loopOnce(Looper.java:201)
	at android.os.Looper.loop(Looper.java:288)
	at android.app.ActivityThread.main(ActivityThread.java:7870)
	at java.lang.reflect.Method.invoke(Native Method)
	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1003)
Caused by: android.app.StackTrace: Last startServiceCommon() call for this service was made here
	at android.app.ContextImpl.startServiceCommon(ContextImpl.java:1877)
	at android.app.ContextImpl.startForegroundService(ContextImpl.java:1832)
	at android.content.ContextWrapper.startForegroundService(ContextWrapper.java:781)
	at androidx.core.content.ContextCompat$Api26Impl.startForegroundService(ContextCompat.java:931)
	at androidx.core.content.ContextCompat.startForegroundService(ContextCompat.java:703)
	at org.schabi.newpipe.player.helper.PlayerHolder.startService(PlayerHolder.java:126)
	at org.schabi.newpipe.fragments.detail.VideoDetailFragment.openMainPlayer(VideoDetailFragment.java:1217)
	at org.schabi.newpipe.fragments.detail.VideoDetailFragment.$r8$lambda$7cA0nfOW4EXxLpA03fs92UQohwM(Unknown Source:0)
	at org.schabi.newpipe.fragments.detail.VideoDetailFragment$$ExternalSyntheticLambda13.run(Unknown Source:2)
	at org.schabi.newpipe.fragments.detail.VideoDetailFragment.replaceQueueIfUserConfirms(VideoDetailFragment.java:2130)
	at org.schabi.newpipe.fragments.detail.VideoDetailFragment.openVideoPlayer(VideoDetailFragment.java:1184)
	at org.schabi.newpipe.fragments.detail.VideoDetailFragment.openVideoPlayerAutoFullscreen(VideoDetailFragment.java:1197)
	at org.schabi.newpipe.fragments.detail.VideoDetailFragment.onClick(VideoDetailFragment.java:511)
	at android.view.View.performClick(View.java:7455)
	at android.view.View.performClickInternal(View.java:7432)
	at android.view.View.access$3700(View.java:835)
	at android.view.View$PerformClick.run(View.java:28810)
	at android.os.Handler.handleCallback(Handler.java:938)
	at android.os.Handler.dispatchMessage(Handler.java:99)
	... 6 more

Aug 06 '22 19:08 opusforlife2

Kudos, SonarCloud Quality Gate passed!