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

improve audio level display #197

Merged
merged 14 commits into from
Apr 29, 2018
Merged

Conversation

fightling
Copy link
Contributor

Improved CPU usage and look of the audio level display

After a first profiling session with voctogui I realized a CPU usage
peak at audioleveldisplay.py:22(on_draw). This drawing routine uses
move-to and line-to to create a gradient for displaying the audio levels
which I found quite expensive. I replaced it by code which draws four
rectangles for every channel by using cairo gradients which works much
faster.

Sorry for the temporary mess on the master branch where I accidentally committed eight changes which I have already reverted.

After a first profiling session with voctogui I realized a CPU usage
peak at audioleveldisplay.py:22(on_draw). This drawing routine uses
move-to and line-to to create a gradient for displaying the audio levels
which I found quite expensive. I replaced it by code which draws four
rectangles for every channel by using cairo gradients which works much
faster.
By doing this I accidentally changed the look quite a bit, because the
gradient calculation is now a little different.
I don't know the original goal for the look but one can now easily
change the gradient look by changing the values of XXX_fade in lines
50..53 and the gradient stops themselves in lines 57..59, 62..64, 67..69
and 72..74.
So for example if you like to have a larger green zone we could add
another green stop and so start the yellow a bit higher.
Suggestions welcome.
also reduced code amount

gradient stops now could be changed in gradient() in line 24 and
brightnesses in line 64..67
text is now rendered over the most right channel bar and inverted if
this bar gets bright
@MaZderMind
Copy link
Contributor

@fightling Would mind if I enable branch restrictions on the master-branch so that it can no longer be directly pushed to? This might help to avoid such accidents which I also have done more then once.

@CarlFK
Copy link
Collaborator

CarlFK commented Apr 18, 2018 via email

@fightling
Copy link
Contributor Author

fightling commented Apr 18, 2018 via email

@MaZderMind
Copy link
Contributor

@fightling no worries, happened to everyone. I enabled branch protection for master ans also force-push-removed your commits from there, because they still failed the travis build.

Please ensure to merge your PRs into the git@c3voc.de:voctomix.git repo, as the github-repo is only a mirror (which is overwritten by the state of the above mentioned repo). Anyone of the VOC core team can add your ssh-key there, you cann mail/chat it to me if you want.

@fightling
Copy link
Contributor Author

The travis warnings are not originated in the files I have changed. I already have a patch to fix all these warnings lying in my local repro. But at least in one case I'm not sure that my change is neutral and correct. Should I push it into a new branch review/fixing-pep8-warnings or something similar and then we can discuss my changes?

@MaZderMind
Copy link
Contributor

@fightling absolutely! I'll take a closer look at this PR later this day.

Merged to get travis fixed from pull request:Review/fix travis issues #198
@fightling
Copy link
Contributor Author

fightling commented Apr 23, 2018

Merged master to get travis fixes

@MaZderMind
Copy link
Contributor

@fightling I guess you decided to not center the text for better readability? I kind'a liked them better centered, but I guess that's a case of personal opinion :D

@voc-mng voc-mng merged commit 8cb55d6 into master Apr 29, 2018
@voc-mng voc-mng deleted the feature/improve-audioleveldisplay branch May 23, 2018 21:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants