Merge lp:~diwic/unity-control-center/lp1367693 into lp:unity-control-center

Proposed by David Henningsson on 2014-09-16
Status: Merged
Approved by: Sebastien Bacher on 2014-09-22
Approved revision: 12788
Merged at revision: 12789
Proposed branch: lp:~diwic/unity-control-center/lp1367693
Merge into: lp:unity-control-center
Diff against target: 60 lines (+12/-9)
2 files modified
panels/sound/gvc-mixer-control.c (+3/-3)
panels/sound/gvc-mixer-dialog.c (+9/-6)
To merge this branch: bzr merge lp:~diwic/unity-control-center/lp1367693
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve on 2014-09-19
Robert Ancell Approve on 2014-09-19
Sebastien Bacher 2014-09-16 Pending
Review via email: mp+234816@code.launchpad.net

Commit message

Make sure selecting a bluetooth input works even when the headset is in A2DP mode, and make sure the correct levels are shown in the level bar (Bug 1367693)

Description of the change

Hi Sebastian!

I've been asked (by oem-priority) to fix bug 1367693 and SRU it into Trusty. I've come up with fixes for the bug (which turned out to be two bugs in one), but can you review my fixes and help me with the process, i e what else do I need to do (in form of testing etc) to make sure this gets SRUed into 14.04?

So far I've just been running testing my fixes with lp:unity-control-center's trunk on 14.04.

Thanks!

To post a comment you must log in.
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:12788
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~diwic/unity-control-center/lp1367693/+merge/234816/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/unity-control-center-ci/112/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-control-center-utopic-amd64-ci/14
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-control-center-utopic-armhf-ci/14
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-control-center-utopic-i386-ci/14

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-control-center-ci/112/rebuild

review: Needs Fixing (continuous-integration)
Robert Ancell (robert-ancell) wrote :

I don't know the code in enough detail to know the implications of the change but they make sense to me. You should file a bug against libgnome-volume-control as their code appears to contain the same error. They're also more likely to give a better review.

Please update the commit message to be more informative of the problem and solution.

To propose this into trusty make a merge proposal to lp:unity-control-center/14.04

review: Approve
David Henningsson (diwic) wrote :

Hi Robert and thanks for the review,

> I don't know the code in enough detail to know the implications of the change
> but they make sense to me. You should file a bug against libgnome-volume-
> control as their code appears to contain the same error. They're also more
> likely to give a better review.

The first bug has been upstreamed to:

 https://bugzilla.gnome.org/show_bug.cgi?id=736943

The second bug is in gvc-mixer-dialog and their gvc-mixer-dialog looks slightly different, so the fix could also be slightly different. And after all it's mostly a band-aid for a PulseAudio oddity, so they may argue that things should be fixed in PA instead (although such a behaviour change would be more difficult and nothing I would want to SRU into trusty).

> Please update the commit message to be more informative of the problem and
> solution.

Done.

> To propose this into trusty make a merge proposal to lp:unity-control-
> center/14.04

Done: https://code.launchpad.net/~diwic/unity-control-center/lp1367693-14.04/+merge/235243

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'panels/sound/gvc-mixer-control.c'
2--- panels/sound/gvc-mixer-control.c 2013-12-03 02:12:11 +0000
3+++ panels/sound/gvc-mixer-control.c 2014-09-16 13:54:26 +0000
4@@ -1550,9 +1550,9 @@
5 if (gvc_mixer_ui_device_get_stream_id (dev) == gvc_mixer_stream_get_id (stream)) {
6 g_debug ("Looks like we profile swapped on a non server default sink");
7 gvc_mixer_control_set_default_sink (control, stream);
8+ control->priv->profile_swapping_device_id = GVC_MIXER_UI_DEVICE_INVALID;
9 }
10 }
11- control->priv->profile_swapping_device_id = GVC_MIXER_UI_DEVICE_INVALID;
12 }
13
14 if (control->priv->default_sink_name != NULL
15@@ -1662,11 +1662,11 @@
16 if (dev != NULL) {
17 /* now check to make sure this new stream is the same stream just matched and set on the device object */
18 if (gvc_mixer_ui_device_get_stream_id (dev) == gvc_mixer_stream_get_id (stream)) {
19- g_debug ("Looks like we profile swapped on a non server default sink");
20+ g_debug ("Looks like we profile swapped on a non server default source");
21 gvc_mixer_control_set_default_source (control, stream);
22+ control->priv->profile_swapping_device_id = GVC_MIXER_UI_DEVICE_INVALID;
23 }
24 }
25- control->priv->profile_swapping_device_id = GVC_MIXER_UI_DEVICE_INVALID;
26 }
27 if (control->priv->default_source_name != NULL
28 && info->name != NULL
29
30=== modified file 'panels/sound/gvc-mixer-dialog.c'
31--- panels/sound/gvc-mixer-dialog.c 2014-03-18 08:35:09 +0000
32+++ panels/sound/gvc-mixer-dialog.c 2014-09-16 13:54:26 +0000
33@@ -397,8 +397,13 @@
34 res = pa_stream_disconnect (s);
35 if (res == 0) {
36 g_debug("stream has been disconnected");
37- pa_stream_unref (s);
38- }
39+ }
40+ else {
41+ g_warning ("pa_stream_disconnect failed, res = %d", res);
42+ /* If disconnect failed, at least make sure we don't show the data */
43+ pa_stream_set_read_callback (s, NULL, NULL);
44+ }
45+ pa_stream_unref (s);
46 g_object_set_data (G_OBJECT (dialog->priv->input_level_bar), "pa_stream", NULL);
47 }
48
49@@ -407,10 +412,8 @@
50 if (pa_context_get_server_protocol_version (context) < 13) {
51 return;
52 }
53- if (res == 0) {
54- g_object_set_data (G_OBJECT (stream), "has-monitor", GINT_TO_POINTER (FALSE));
55- }
56- g_debug ("Stopping monitor for %u", pa_stream_get_index (s));
57+ g_object_set_data (G_OBJECT (stream), "has-monitor", GINT_TO_POINTER (FALSE));
58+ g_debug ("Stopping monitor");
59 g_object_set_data (G_OBJECT (dialog->priv->input_level_bar), "stream", NULL);
60 }
61

Subscribers

People subscribed via source and target branches