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

[David Henningsson]

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)

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!

[PS Jenkins bot (ps-jenkins)]

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)]

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

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:12788
http://jenkins.qa.ubuntu.com/job/unity-control-center-ci/113/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-control-center-utopic-amd64-ci/15
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-control-center-utopic-armhf-ci/15
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-control-center-utopic-i386-ci/15

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

review: Approve (continuous-integration)

[David Henningsson (diwic)]

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

=== modified file 'panels/sound/gvc-mixer-control.c'
--- panels/sound/gvc-mixer-control.c	2013-12-03 02:12:11 +0000
+++ panels/sound/gvc-mixer-control.c	2014-09-16 13:54:26 +0000
@@ -1550,9 +1550,9 @@
                         if (gvc_mixer_ui_device_get_stream_id (dev) == gvc_mixer_stream_get_id (stream)) {
                                 g_debug ("Looks like we profile swapped on a non server default sink");
                                 gvc_mixer_control_set_default_sink (control, stream);
+                                control->priv->profile_swapping_device_id = GVC_MIXER_UI_DEVICE_INVALID;
                         }
                 }
-                control->priv->profile_swapping_device_id = GVC_MIXER_UI_DEVICE_INVALID;
         }
 
         if (control->priv->default_sink_name != NULL
@@ -1662,11 +1662,11 @@
                 if (dev != NULL) {
                         /* now check to make sure this new stream is the same stream just matched and set on the device object */
                         if (gvc_mixer_ui_device_get_stream_id (dev) == gvc_mixer_stream_get_id (stream)) {
-                                g_debug ("Looks like we profile swapped on a non server default sink");
+                                g_debug ("Looks like we profile swapped on a non server default source");
                                 gvc_mixer_control_set_default_source (control, stream);
+                                control->priv->profile_swapping_device_id = GVC_MIXER_UI_DEVICE_INVALID;
                         }
                 }
-                control->priv->profile_swapping_device_id = GVC_MIXER_UI_DEVICE_INVALID;
         }
         if (control->priv->default_source_name != NULL
             && info->name != NULL

=== modified file 'panels/sound/gvc-mixer-dialog.c'
--- panels/sound/gvc-mixer-dialog.c	2014-03-18 08:35:09 +0000
+++ panels/sound/gvc-mixer-dialog.c	2014-09-16 13:54:26 +0000
@@ -397,8 +397,13 @@
                 res = pa_stream_disconnect (s);
                 if (res == 0) {
                         g_debug("stream has been disconnected");
-                        pa_stream_unref (s);
-                }
+                }
+                else {
+                        g_warning ("pa_stream_disconnect failed, res = %d", res);
+                        /* If disconnect failed, at least make sure we don't show the data */       
+                        pa_stream_set_read_callback (s, NULL, NULL);
+                }
+                pa_stream_unref (s);
                 g_object_set_data (G_OBJECT (dialog->priv->input_level_bar), "pa_stream", NULL);        
         }
                 
@@ -407,10 +412,8 @@
         if (pa_context_get_server_protocol_version (context) < 13) {
                 return;
         }
-        if (res == 0) {
-                g_object_set_data (G_OBJECT (stream), "has-monitor", GINT_TO_POINTER (FALSE));
-        }
-        g_debug ("Stopping monitor for %u", pa_stream_get_index (s));
+        g_object_set_data (G_OBJECT (stream), "has-monitor", GINT_TO_POINTER (FALSE));
+        g_debug ("Stopping monitor");
         g_object_set_data (G_OBJECT (dialog->priv->input_level_bar), "stream", NULL);
 }