Various fixes related to MediasessionManager

[litetex]

What is it?

• Bugfix (user facing)
• Feature (user facing)
• Codebase improvement (dev facing)
• Meta improvement to the project (dev facing)

Description of the changes in your PR

The following things are fixed

from #7161 (credits to @Redirion)

Issue 1 is when the option "Show thumbnail" is turned off. It caused setMetadata to never be called.
// setMetadata only updates the metadata when any of the metadata keys are null mediaSessionManager.setMetadata(getVideoTitle(), getUploaderName(), showThumbnail ? getThumbnail() : null, duration);

basically the first line in setMetadata checks, if the provided thumbnail/albumArt is not null.

Issue 2: if no thumbnail could be loaded, the dummy thumbnail resource would be used to be created through a Bitmap factory. Firstly it will be created each time (expensive) and secondly hashCode() will always yield different results. This means that setMetadata was basically executed in its whole every single time (each second since last updates).

I also reworked/de-duplicated the code a bit 😉

Issue 3: #7087
#7087 (comment)

Okay I tested the stuff a bit and found out that the metadata updates - that are done every second - are completely useless.
They should only be done when the metadata changes e.g. when opening a new video.

Issue 4:
Comparisons e.g. if the current title/artist/duration is also the requested one have been missing.
Added those by caching the last requested data using hashcodes.
This also avoids resource-intensive mediaSession.getController().getMetadata() calls.

With the changes applied there is a massive performance improvement

triggerProgressUpdate now runs within 4-13ms / 0,7-2ms (cpu):

Before it has been 30-130ms / 6-12ms (cpu):

So this decreases execution time of the progress update loop by about 10x (at least on my emulated Pixel 3a).

Fixes the following issue(s)

• Fixes Potential performance problem with MediaSessionManager#get/setMetadata #7087

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

• I read the contribution guidelines.

[Redirion]

you could add the result of isLiveEdge to decide whether to check the duration

[litetex]

@Redirion

you could add the result of isLiveEdge to decide whether to check the duration

I can't follow. Where do you mean should I add this?

[Redirion]

sorry isLiveEdge was wrong. But you can use isLive():

NewPipe/app/src/main/java/org/schabi/newpipe/player/Player.java

Line 4158 in ac371e6

private boolean isLive() {

you could use it as an additional parameter:

mediaSessionManager.setMetadata(
                getVideoTitle(),
                getUploaderName(),
                showThumbnail ? Optional.ofNullable(getThumbnail()) : Optional.empty(),
                tag.getMetadata().getDuration(),
                isLive()
        );

this way you can use duration for comparisons if it isn't a livestream

[litetex]

@Redirion

this way you can use duration for comparisons if it isn't a livestream

Duration comparison is already always done:

NewPipe/app/src/main/java/org/schabi/newpipe/player/helper/MediaSessionManager.java

Line 143 in bbc1375

|| duration != lastDuration

So I don't understand the problem in the first place...

Anyway the check for isLive is done by asking the player which is risky as it may not be fully initalized when onMetadataChanged is called. It's also quite resource expensive when compared to simply reading if it's a livestream from the metadata.

I changed the code so that it now considers livestream and always uses duration -1 for them 😄

[Redirion]

Yep sorry, all good

[Redirion]

lgtm

Two personal preferences as a comment:

• I would always prefer equals over hashCode for String comparisons
• I would rather have a bit duplicated code by using more than one method than having a method with many return statements

[litetex]

• I would always prefer equals over hashCode for String comparisons

I also considered that. However the deal with hashCode is that storing an int requires usually way less RAM than a String by - what I think are accaptable odds - of having an collision: Which is technically 1 in 2^32.
Someone tried it with 58k words and got one collision: https://stackoverflow.com/a/9407032