-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix first item always played in the play queue when reloading play queue manager #7693
Fix first item always played in the play queue when reloading play queue manager #7693
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#7427 was already fixed.
What are the player.setRecovery
blocks doing inside this PR?
See #7427 (comment): the player recovery position needs to be set when clicking on the content thumbnail (so playback position is not lost when you used background or popup player (by pressing the background button) and you are now using main player). Other additions are for cases like this one: you put the app with a content playing in the main player on the background, so background player is used while the player is not visible and you came back to the player with Android's media notification. Playback position was not saved and the content starts to play again from the previous playback position saved (I was able to reproduce this once, now it seems I can't). |
@TiA4f8R Please one bugfix at the time if they aren't directly related. It would be best if you limit this PR to the fixes of #7531 and create a new PR for the additional fixes of #7427. Btw: I think the workaround for #7427 is incomplete because in the workaround should also be applied to |
…lution The issue was caused by an ExoPlayer change, which when setting a media source, resets the current playback position and the current window index by default. Also set player recovery in more places to fix playback position not propely set in some cases after a player type switch.
In fact, no, because there is no need to set player recovery when pressing an element inside the metadata part of the description or when pressing other buttons than popup and background buttons.
Ok, if you want. |
73cc4cd
to
ea07d77
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the documentation what you did is correct, and that's the only place where setMediaSource
is used.
I tested on API 19 emulated, API 31 emulated and API 29 device and the fix worked (i.e. I could reproduce the bug before, but with the same steps now I can't).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Nice and simple 👍
What is it?
Description of the changes in your PR
This PR fixes the playback of the first item in the play queue when a resolution (when switching the quality played in the quality selector) or a player type change occurs (for e.g. switching from background to main player), when playing another item than the first one of the play queue.
The issue was caused by an ExoPlayer change, which when setting a new media source, resets now the current playback position and the current window index to the first item and its default position by default (the position was then set with the first item's recovery position).
It sets now to ExoPlayer to not reset the position when setting a new media source.
Fixes the following issue(s)
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