feat: support hardware keys for playback control

[andrew-ld]

you can test this change using adb shell input keyevent 87 (It should send you to the next video)

closes #2185

[Bnyro]

For fixing the lint, please run ktlint --code-style=android-studio -F in the root directory of the project.

[Bnyro]

Unfortunately, this introduces some major regressions:

• the thumbnail quality in the notification is very low
• it's not possible to seek with the seekbar in the notification
• the notification doesn't show the correct position

It would be great if we can fix all those, otherwise we have to make this behavior optional and let users choose if they prefer the above functionality or support for media buttons.

Otherwise the PR looks great, thanks for your efforts!

[andrew-ld]

still need to implement MediaMetadataCompat to report playback information, I will try to do it

this should fix the seek and the thumbnail

[andrew-ld]

@Bnyro now seek and location work correctly from the notification

[andrew-ld]

for high resolution artwork it seems that NotificationCompat with MediaStyle does not support artwork uri, it only takes the one provided as bitmap which is low quality

[Bnyro]

Please rebase with proper git config (username and email), you're currently commuting as "ghost", see https://github.com/libre-tube/LibreTube/commit/0557b52ade13a55f751f93ce339019e05073034d.patch

[Bnyro]

Alright, if you share your email here I can rebase the other commits to contain your email as well tomorrow. I assume the git username is the same as on GitHub?

[andrew-ld]

@Bnyro I should have solved with git, it seems that duration is not always available from the player, when changing video sometimes I can't get it, I guess I should listen some other event from the player

[FireMasterK]

Alright, if you share your email here I can rebase the other commits to contain your email as well tomorrow. I assume the git username is the same as on GitHub?

You can just use git log to find that btw!

[andrew-ld]

I squashed all the commits and should have solved the video duration problem as well by listening onIsLoadingChanged

[Bnyro]

You can just use git log to find that btw!

All commits were done as ghost at that point of time :p

[Bnyro]

I'll see if I can find a way to fix the thumbnail quality, otherwise we should make this optional and let users choose whether they prefer media button controls handling or a good notification thumbnail quality.

[Bnyro]

Found a fix for it now, see 698f783 :)

From my side, there's nothing merge-blocking left apart from the one comment I left above.

Since there are some major changes with this, I'd appreciate if we could get some testers first before merging:

@akano12, @luckkmaxx, @Silther, @rozari0, @IndusAryan: Could you please test the CI / debug builds from this PR for regressions in the player notification when you have some time? Thank you in advance, everyone!

[akano12]

On stable and when fast forwarding, the play button changes into circular progress indicator while on this it changes to pause then play again, it was better and more informative previously (specifically on slow networks)

Also there's a bug (not a regression as it also exists on stable and debug). If you play a video and skip to next video from notification controls, the video will play from the same position of the previous video (not sure if you'd like to work on this issue before merging).

Will edit if i find something else

[Bnyro]

Also there's a bug (not a regression as it also exists on stable and debug). If you play a video and skip to next video from notification controls, the video will play from the same position of the previous video (not sure if you'd like to work on this issue before merging).

That's not related to the changes in this PR, so that should be covered in an other issue/PR.

On stable and when fast forwarding, the play button changes into circular progress indicator while on this it changes to pause then play again, it was better and more informative previously (specifically on slow networks)

In the notification or the normal player?

[akano12]

In the notification or the normal player?

Notification control

[Bnyro]

I think I know what's the issue then, will look into it later.

[andrew-ld]

the buffering state of exoplayer should be communicated to mediasession, this state on mediasession side is called STATE_BUFFERING (it is related to playback state), I can try to implement it tomorrow, if someone wants to do it today is welcome

[Bnyro]

That's what I thought of too, I'll implement that later today.

[Bnyro]

Still not 100% fixed, just quickly pushed my changes as I need to leave now

[IndusAryan]

In the notification or the normal player?

Notification control

yes,in this the player is not seeking to the point but while that it is taking a pause.

second one is when we play the next video while in background ,it takes a while even if internet on cellular is fast,this doesnt happen everytime but this is present in stable also.

[IndusAryan]

Also there's a bug (not a regression as it also exists on stable and debug). If you play a video and skip to next video from notification controls, the video will play from the same position of the previous video (not sure if you'd like to work on this issue before merging).

another bug in this is -> after doing above use previous button to go back the previous video,ideally it should continue from where it was ,but it resets to 00:00 and starts from the starting.(this should be done if playback positions and watch history is on)

[Bnyro]

Please only report things that behave different from the stable release here, the PR doesn't aim to fix other existing bugs.

[Silther]

At the beginning it works quite well, but it stops relatively quickly if you do not react regularly with the player (skip, play, pause, start etc.). EDIT: after a reconnect with my headphones it seems to work now (or didn't wait long enough).

As a suggestion, it should briefly display the corresponding symbols after the input

[andrew-ld]

@Bnyro from my side I think there must not be any other change since the bugs reported so far are present on master as well, if you finished the buffering part I think I can remove the draft status to make it to be mergiable

[Bnyro]

@andrew-ld yes, agree. The buffering part is still not working properly, and I probably won't continue work on it too soon, so feel free to finish that up. Then it's good to merge from my side.

[andrew-ld]

@Bnyro can you tell me in which cases your code regarding buffering does not work

[andrew-ld]

I unified the logic to update the playback state, now it should no longer update to an unwanted state

[Bnyro]

I didn't know we can override onIsLoadingChanged, looks much better from a code perspective now. I'll test the changes later and merge it if I don't find any further issues.

[Bnyro]

I fixed one issue I found, seems to work well now.

[Bnyro]

Thanks for your work!

[akano12]

There's still an issue with play button in the notifications, I'm not sure if this was introduced by this pr.

To reproduce

1. Play a video
2. Pause the video from the player buttons.
3. Tap home button.

If you open the notifications, the pause/play button would show circular progress indicator as if the video was buffering

[Bnyro]

Likely related, please move that to a new issue with a reference to this PR.