Merge lp:~hui.wang/unity-settings-daemon/unity-settings-daemon into lp:unity-settings-daemon

Proposed by Hui Wang
Status: Merged
Approved by: Sebastien Bacher
Approved revision: 4061
Merged at revision: 4065
Proposed branch: lp:~hui.wang/unity-settings-daemon/unity-settings-daemon
Merge into: lp:unity-settings-daemon
Diff against target: 84 lines (+24/-19)
1 file modified
plugins/media-keys/gsd-media-keys-manager.c (+24/-19)
To merge this branch: bzr merge lp:~hui.wang/unity-settings-daemon/unity-settings-daemon
Reviewer Review Type Date Requested Status
Sebastien Bacher Approve
David Henningsson (community) Approve
Review via email: mp+240807@code.launchpad.net

Commit message

show correct microphone mute icon

Description of the change

We found a problem on the ubuntu 14.04 (trusty), and we think this problem exists on 14.10 and 15.04 as well.

The problem is when we press microphone mute hotkey, the OSD shows a speaker mute icon instead of microphone mute.

the tracking bug for this problem is: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1381856

To post a comment you must log in.
Revision history for this message
Hui Wang (hui.wang) wrote :

Forgot to mention, the microphone icons I used in the patch was installed by this package: gnome-icon-theme-symbolic.

Those icons will be installed by default and already be used in the gsd-media-keys-manager.c, so it is safe to use it again in this file.

Revision history for this message
Sebastien Bacher (seb128) wrote :

Thanks for the work, Ccing David who knows that code a bit better than me

David, could you comment on whether "GVC_IS_MIXER_SINK (stream)" is the right way to check if the right way to tell a mic appart from an output?

Revision history for this message
David Henningsson (diwic) wrote :

> David, could you comment on whether "GVC_IS_MIXER_SINK (stream)" is the right way to check if the right way to tell a mic appart from an output?

I suppose so, since it is already used that way a few lines down.

In short, assuming Hui has properly tested his patch, I see no problems with it.

Nitpick: perhaps get_icon_name_for_volume can now be refactored to use the global definitions of "icon_names" and "mic_icon_names" instead of copy-pasted local ones.

review: Approve
Revision history for this message
Hui Wang (hui.wang) wrote :

I have tested my patch on the machine whith the trusty installed. It worked very well.

And about the Nitpick mentioned by David, if you want me to do the change, just let me know. Or probably you can do the change by yourself when you merge it.

Revision history for this message
Sebastien Bacher (seb128) wrote :

Hui, could you do the nitpick change? we can as well fix that before merging in. thanks!

4061. By Hui Wang

change to use global variables in the get_icon_name_for_volume

Revision history for this message
Hui Wang (hui.wang) wrote :

Done, the commit 4061 is for nitpick.

Revision history for this message
Anthony Wong (anthonywong) wrote :

Seb, we need this fix for oem projects, could you review this patch at your earliest convenience?

Revision history for this message
Sebastien Bacher (seb128) wrote :

Sorry for the delay due to holidays, the change looks fine indeed

review: Approve
Revision history for this message
Hui Wang (hui.wang) wrote :

Hi Sebastien,

Is it possible to merge this fix to Trusty, we have an OEM bug #1389099
still waiting this fix to be merged to Trusty, if you need me to do
something, please let me know.

Regards,
Hui.

On 01/08/2015 12:54 AM, Sebastien Bacher wrote:
> The proposal to merge lp:~hui.wang/unity-settings-daemon/unity-settings-daemon into lp:unity-settings-daemon has been updated.
>
> Commit Message changed to:
>
> show correct microphone mute icon
>
> For more details, see:
> https://code.launchpad.net/~hui.wang/unity-settings-daemon/unity-settings-daemon/+merge/240807

Revision history for this message
Sebastien Bacher (seb128) wrote :

Le 13/01/2015 03:16, Hui Wang a écrit :
> Hi Sebastien,
>
> Is it possible to merge this fix to Trusty,

Hey Hui,

Yes, I was waiting to get some feedback from the upload to vivid, I'm
going to SRU it next

Cheers,
Sebastien Bacher

Revision history for this message
Hui Wang (hui.wang) wrote :

Got it, thanks.

On 01/14/2015 10:08 PM, Sebastien Bacher wrote:
> Le 13/01/2015 03:16, Hui Wang a écrit :
>> Hi Sebastien,
>>
>> Is it possible to merge this fix to Trusty,
> Hey Hui,
>
> Yes, I was waiting to get some feedback from the upload to vivid, I'm
> going to SRU it next
>
> Cheers,
> Sebastien Bacher
>

Revision history for this message
Hui Wang (hui.wang) wrote :

Hello Sebastien,

Our OEM project has been waiting this fix to be merged to Trusty (14.04)
, is it possible we do that now? If you need me to do something, just
let me know.

Thanks,
Hui.

On 01/14/2015 10:09 PM, Sebastien Bacher wrote:
> Le 13/01/2015 03:16, Hui Wang a écrit :
>> Hi Sebastien,
>>
>> Is it possible to merge this fix to Trusty,
> Hey Hui,
>
> Yes, I was waiting to get some feedback from the upload to vivid, I'm
> going to SRU it next
>
> Cheers,
> Sebastien Bacher
>

Revision history for this message
Sebastien Bacher (seb128) wrote :

Hey Hui,

We could probably SRU it now, there are some issues though like [1] the
fact that indicator menus are misplaced on the greeter when hidpi
scaling on. We could have a first SRU to try to activate it only for the
installer/oem config (we just need to find e.g an env variable specific
to those sessions) or wait to have a fix for the unity-greeter issue and
SRU that as well.
Do you have a preference for one of those options?

Cheers,
Sebastien Bacher

[1] https://bugs.launchpad.net/ubuntu/+source/unity-greeter/+bug/1434094

Le 25/03/2015 07:37, Hui Wang a écrit :
> Hello Sebastien,
>
> Our OEM project has been waiting this fix to be merged to Trusty
> (14.04) , is it possible we do that now? If you need me to do
> something, just let me know.
>
>
> Thanks,
> Hui.

Revision history for this message
Hui Wang (hui.wang) wrote :

Hello Seb,

The option 2 is fine, it is best that two fixes to be SRUed together. :-)

Thanks,
Hui.

On 03/25/2015 04:07 PM, Sebastien Bacher wrote:
> Hey Hui,
>
> We could probably SRU it now, there are some issues though like [1] the
> fact that indicator menus are misplaced on the greeter when hidpi
> scaling on. We could have a first SRU to try to activate it only for the
> installer/oem config (we just need to find e.g an env variable specific
> to those sessions) or wait to have a fix for the unity-greeter issue and
> SRU that as well.
> Do you have a preference for one of those options?
>
> Cheers,
> Sebastien Bacher
>
> [1] https://bugs.launchpad.net/ubuntu/+source/unity-greeter/+bug/1434094
>
> Le 25/03/2015 07:37, Hui Wang a écrit :
>> Hello Sebastien,
>>
>> Our OEM project has been waiting this fix to be merged to Trusty
>> (14.04) , is it possible we do that now? If you need me to do
>> something, just let me know.
>>
>>
>> Thanks,
>> Hui.
>
>

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'plugins/media-keys/gsd-media-keys-manager.c'
--- plugins/media-keys/gsd-media-keys-manager.c 2014-03-04 10:49:56 +0000
+++ plugins/media-keys/gsd-media-keys-manager.c 2014-12-03 02:43:22 +0000
@@ -257,6 +257,22 @@
257 NULL257 NULL
258};258};
259259
260static const char *audio_volume_icons[] = {
261 "audio-volume-muted-symbolic",
262 "audio-volume-low-symbolic",
263 "audio-volume-medium-symbolic",
264 "audio-volume-high-symbolic",
265 NULL
266};
267
268static const char *mic_volume_icons[] = {
269 "microphone-sensitivity-muted-symbolic",
270 "microphone-sensitivity-low-symbolic",
271 "microphone-sensitivity-medium-symbolic",
272 "microphone-sensitivity-high-symbolic",
273 NULL
274};
275
260static const char *brightness_icons[] = {276static const char *brightness_icons[] = {
261 "notification-display-brightness-off",277 "notification-display-brightness-off",
262 "notification-display-brightness-low",278 "notification-display-brightness-low",
@@ -343,10 +359,13 @@
343static gboolean359static gboolean
344ubuntu_osd_notification_show_volume (GsdMediaKeysManager *manager,360ubuntu_osd_notification_show_volume (GsdMediaKeysManager *manager,
345 gint value,361 gint value,
346 gboolean muted)362 gboolean muted,
363 gboolean is_mic)
347{364{
365 const char **icons_name = is_mic ? mic_volume_icons : volume_icons;
366
348 return ubuntu_osd_do_notification (&manager->priv->volume_notification,367 return ubuntu_osd_do_notification (&manager->priv->volume_notification,
349 "volume", value, muted, volume_icons);368 "volume", value, muted, icons_name);
350}369}
351370
352static gboolean371static gboolean
@@ -606,20 +625,6 @@
606 gboolean muted,625 gboolean muted,
607 int volume)626 int volume)
608{627{
609 static const char *icon_names[] = {
610 "audio-volume-muted-symbolic",
611 "audio-volume-low-symbolic",
612 "audio-volume-medium-symbolic",
613 "audio-volume-high-symbolic",
614 NULL
615 };
616 static const char *mic_icon_names[] = {
617 "microphone-sensitivity-muted-symbolic",
618 "microphone-sensitivity-low-symbolic",
619 "microphone-sensitivity-medium-symbolic",
620 "microphone-sensitivity-high-symbolic",
621 NULL
622 };
623 int n;628 int n;
624629
625 if (muted) {630 if (muted) {
@@ -635,9 +640,9 @@
635 }640 }
636641
637 if (is_mic)642 if (is_mic)
638 return mic_icon_names[n];643 return mic_volume_icons[n];
639 else644 else
640 return icon_names[n];645 return audio_volume_icons[n];
641}646}
642647
643static gboolean648static gboolean
@@ -1337,7 +1342,7 @@
1337 const GvcMixerStreamPort *port;1342 const GvcMixerStreamPort *port;
1338 const char *icon;1343 const char *icon;
13391344
1340 if (ubuntu_osd_notification_show_volume (manager, vol, muted))1345 if (ubuntu_osd_notification_show_volume (manager, vol, muted, !GVC_IS_MIXER_SINK (stream)))
1341 goto done;1346 goto done;
13421347
1343 vol = CLAMP (vol, 0, 100);1348 vol = CLAMP (vol, 0, 100);

Subscribers

People subscribed via source and target branches