Skip to content

Conversation

@Douile
Copy link
Contributor

@Douile Douile commented Mar 27, 2021

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

  • Allow the user to pause/unpause while a video is buffering
    • Check whether the playWhenReady is true instead of isPlaying: isPlaying returns false if the video is buffering

Fixes the following issue(s)

APK testing

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

Due diligence

@opusforlife2
Copy link
Collaborator

Yay! Much needed usability fix.

@AudricV AudricV added bug Issue is related to a bug player Issues related to any player (main, popup and background) labels Mar 27, 2021
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.

Thank you!
Almost there ;-)

@Douile
Copy link
Contributor Author

Douile commented Mar 28, 2021

These changes seem to make
https://github.com/Douile/NewPipe/blob/963a7609f1681ec4942a2a3e560db82f280fc0e3/app/src/main/java/org/schabi/newpipe/player/Player.java#L709-L713

redundant, should I replace this snippit with playPause? I'm not sure why getPlayWhenReady was being used here but not in playPause

@Stypox
Copy link
Member

Stypox commented Mar 28, 2021

@Douile nice catch. Replace every instance of if (getPlayWhenReady()) play(); else pause(); with playPause(), thanks

@Douile
Copy link
Contributor Author

Douile commented Mar 29, 2021

I think that's the only one.

@Stypox Stypox merged commit 84de865 into TeamNewPipe:dev Mar 29, 2021
@Stypox
Copy link
Member

Stypox commented Mar 29, 2021

@Douile thank you :-D

This was referenced Apr 11, 2021
@triallax triallax mentioned this pull request Apr 21, 2021
3 tasks
tossj pushed a commit to tossj/NewPipe-legacy that referenced this pull request Apr 23, 2021
…pe#5929)

Fix pause while buffering
Use getPlayWhenReady wrapper everywhere playWhenReady is checked
Remove duplicate `playPause()` code
@Redirion Redirion mentioned this pull request Apr 28, 2021
4 tasks
@litetex
Copy link
Member

litetex commented May 10, 2021

ℹ This PR very likely caused #6179, see also #6179 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Issue is related to a bug player Issues related to any player (main, popup and background)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Can't pause video while buffering

5 participants