The Sound Menu

Merge lp:~ted/indicator-sound/lp1436021-high-volume-warning into lp:indicator-sound/15.04

  1. lp1436021-high-volume-warning
  2. Merge into trunk.15.04
Proposed by Ted Gould
Status: Merged
Approved by: Ricardo Salveti
Approved revision: 489
Merged at revision: 491
Proposed branch: lp:~ted/indicator-sound/lp1436021-high-volume-warning
Merge into: lp:indicator-sound/15.04
Diff against target: 26 lines (+12/-4)
1 file modified
src/volume-control-pulse.vala (+12/-4)
To merge this branch: bzr merge lp:~ted/indicator-sound/lp1436021-high-volume-warning
Reviewer Review Type Date Requested Status
Ricardo Salveti (community) Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+255892@code.launchpad.net

Commit message

Fix stream property lookup to reliably signal multimedia streams

Description of the change

So we were using an index as an index into the stream type array that wasn't really an index of that array. Which means, in a nutshell, it gave really bad results. So most of the time it was infact getting an error condition and returning the default 'alert' instead of which stream was actually being used. So now we use the index on the hash table that it is meant for and reverse out the stream name from there. It makes the high volume warning reliable and actually work. One of those "how did this work at all" type bugs.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:489
http://jenkins.qa.ubuntu.com/job/indicator-sound-ci/222/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-sound-vivid-amd64-ci/49
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-sound-vivid-armhf-ci/49
        deb: http://jenkins.qa.ubuntu.com/job/indicator-sound-vivid-armhf-ci/49/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/indicator-sound-ci/222/rebuild

review: Approve (continuous-integration)
Revision history for this message
Ricardo Salveti (rsalveti) wrote :

Looks good, works as expected.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/volume-control-pulse.vala'
2--- src/volume-control-pulse.vala 2015-02-27 22:34:23 +0000
3+++ src/volume-control-pulse.vala 2015-04-10 20:53:39 +0000
4@@ -56,10 +56,18 @@
5 private string[] _valid_roles = {"multimedia", "alert", "alarm", "phone"};
6 public override string stream {
7 get {
8- if (_active_sink_input < 0 || _active_sink_input >= _valid_roles.length)
9- return "alert";
10- else
11- return _valid_roles[_active_sink_input];
12+ if (_active_sink_input == -1)
13+ return "alert";
14+ var path = _sink_input_hash[_active_sink_input];
15+ if (path == _objp_role_multimedia)
16+ return "multimedia";
17+ if (path == _objp_role_alert)
18+ return "alert";
19+ if (path == _objp_role_alarm)
20+ return "alarm";
21+ if (path == _objp_role_phone)
22+ return "phone";
23+ return "alert";
24 }
25 }
26 private string? _objp_role_multimedia = null;

Subscribers

People subscribed via source and target branches