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

Change player progress bar update from 500 ms to 1 s #7071

Merged
merged 1 commit into from Sep 9, 2021
Merged

Change player progress bar update from 500 ms to 1 s #7071

merged 1 commit into from Sep 9, 2021

Conversation

ghost
Copy link

@ghost ghost commented Sep 6, 2021

Just like in the issue #7062, this doesn't affect UI as it updates every one second anyway, but reduces very heavy android widget progress bar high cpu usage. With every 500 ms there is 6% cpu usage and with 1 s only 4%. However further changes will have to be made to disable updating of player progress bar when screen is off to further reduce power consumption. With this, total power savings would be 20% in mAh consumption in background playback. This pull request does approx 10% longer screen-off playback.

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

Change frequency of player progress bar update to 1s

Fixes the following issue(s)

Due diligence

Just like in the issue 7062, #7062, this doesn't affect UI as it updates every one second anyway, but reduces very heavy android widget progress bar high cpu usage. With every 500s there is 6% cpu usage and with 1s only 4%. However further changes will have to be made to disable updating of player progress bar when screen is off to further reduce power consumption. With this, total power savings would be 20% in mAh consumption.
@TobiGr TobiGr added the player Issues related to any player (main, popup and background) label Sep 7, 2021
@ghost
Copy link
Author

ghost commented Sep 7, 2021

Testing results are 0.0395 mAh / s versus 0.046 mAh / s without patch, so therefore with 500 ms the power consumption on Moto G4 with android 7 is 16% higher. Note this is done in controlled way the way that the player is started from scratch. After some time garbage collection usage due to progress bar widget string operations grows 2x but when changing playlist it reduces a bit but not to the minimum as with this test. Hence stopping progress update on screen off would really increase playback time a lot.

@TobiGr TobiGr mentioned this pull request Sep 7, 2021
7 tasks
@litetex
Copy link
Member

litetex commented Sep 7, 2021

Fun fact:
This was originally 1s. However it got changed to 500ms in
4553850#diff-7a10d4f22d171ff412c4fd220a9c6eeffe68ba93c4a88fe9b8088c3017795c85L334
4553850#diff-2ed842dce430debfa98eadea4336fe9e0ed85295c56e8959d361cc88bcedd07bR148

And never got changed back...

Copy link
Member

@Redirion Redirion left a comment

Choose a reason for hiding this comment

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

good catch!

@TobiGr
Copy link
Member

TobiGr commented Sep 7, 2021

Do we want to inlcude this into 0.21.10? If yes, we need to change the target branch.

@opusforlife2
Copy link
Collaborator

@TobiGr I vote yes.

@Redirion
Copy link
Member

Redirion commented Sep 9, 2021

can this be done with the "Rebase and merge" option? I didn't use it yet and don't want to break anything.

@TobiGr TobiGr changed the base branch from dev to release/0.21.10 September 9, 2021 11:36
@TobiGr
Copy link
Member

TobiGr commented Sep 9, 2021

@Redirion It's done via the Edit button next to the PR title.

@TobiGr TobiGr merged commit e4a2d2f into TeamNewPipe:release/0.21.10 Sep 9, 2021
@ghost
Copy link

ghost commented Sep 9, 2021

@TobiGr don't PRs get merged into TeamNewPipe:dev not TeamNewPipe:release/0.21.10?

@TobiGr
Copy link
Member

TobiGr commented Sep 9, 2021

generally: yes
But this case is different. We already started the release process for 0.21.10, i.e. new features and code chages are merged to dev while the release process continues in a separate branch. This is a significant improvement which should be included in the next release and is therefore merged into the release branch. The release branch is later merged back into dev.

@litetex litetex mentioned this pull request Sep 10, 2021
1 task
@opusforlife2
Copy link
Collaborator

opusforlife2 commented Sep 10, 2021

However further changes will have to be made to disable updating of player progress bar when screen is off to further reduce power consumption.

@thefalsedev Have you documented this anywhere? If not, please open a new issue with whatever you think is required (or reopen your linked issue if it already contains the relevant info).

Edit: also, do you see the same power consumption stats with the pre-unified version? #4918

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.

High CPU usage
4 participants