Skip to content
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

Update to ExoPlayer 2.13.2 #5856

Merged
merged 1 commit into from
Mar 31, 2021
Merged

Update to ExoPlayer 2.13.2 #5856

merged 1 commit into from
Mar 31, 2021

Conversation

Redirion
Copy link
Member

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

Updates ExoPlayer and fixes related deprecations where applicable.

@TacoTheDank I have also changed the setPlayWhenReady calls to play() and pause() where appropriate

APK testing

the APK can be found by going to the "Checks" tab below the title and then on "artifacts" on the right.

Due diligence

@triallax triallax added the player Issues related to any player (main, popup and background) label Mar 19, 2021
@Redirion
Copy link
Member Author

I have tested on my Pixel 4a (Android 11): basic video playback, play, pause and subtitles tested. Works so far, so any more feedback is appreciated.

@TacoTheDank
Copy link
Member

TacoTheDank commented Mar 19, 2021

Overall very nice at a quick glance. Also, thanks for this; I was having trouble figuring out how to update ExoPlayer to 2.13 in another repository, so this will help a ton!

(You can ignore this little nitpick of mine, since it's not important at all, but I feel that the changes concerning the ExoPlayer 2.13.x upgrade should be a separate commit from the changes that concern previous ExoPlayer updates (the MediaItem builders, setPlayWhenReady, setMediaSource, etc.)

Copy link
Member

@Stypox Stypox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good at a firsf glance, thank you!

app/src/main/java/org/schabi/newpipe/player/Player.java Outdated Show resolved Hide resolved
@TacoTheDank TacoTheDank mentioned this pull request Mar 21, 2021
5 tasks
@Stypox
Copy link
Member

Stypox commented Mar 28, 2021

I tested a little bit: functionality-wise everything works fine, but the play-pause buttons always show the "Pause" icon even when the video is paused @Redirion

@Redirion Redirion marked this pull request as draft March 31, 2021 07:38
@Redirion Redirion marked this pull request as ready for review March 31, 2021 08:36
@Redirion
Copy link
Member Author

I have restored the "onPlayerStateChanged" listener. The splitting up into "onPlaybackStateChanged" and "onPlayWhenReadyChanged" was causing the issues. This further simplifies this PR and makes it easy to merge in my opinion.

@Redirion Redirion requested a review from Stypox March 31, 2021 11:00
Copy link
Member

@Stypox Stypox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested, everything works fine. Thanks :-)

@Stypox Stypox merged commit 5739caa into TeamNewPipe:dev Mar 31, 2021
@Redirion Redirion deleted the exo213 branch March 31, 2021 12:30
This was referenced Apr 11, 2021
tossj pushed a commit to tossj/NewPipe-legacy that referenced this pull request Apr 23, 2021
@litetex litetex mentioned this pull request Jun 23, 2021
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
player Issues related to any player (main, popup and background)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants