Created attachment 230300 [details] [review]
1/3

Created attachment 230301 [details] [review]
2/3

Created attachment 230302 [details] [review]
3/3

I believe I cleaned up a little and fixed a few memory leaks in the process, hopefully without causing new ones :-)

(And hopefully there aren't too many coding style problems either, despite the fact that I almost forgot them while working on PulseAudio & kernel the latest months)

Hmm, I discovered some problems with the existing version of the patches. Please wait a little with committing them.

Created attachment 231457 [details] [review]
1/3

Created attachment 231458 [details] [review]
2/3

Created attachment 231459 [details] [review]
3/3

So it turns out that the patch that was to dynamically add ports was not merged for 3.0, but planned for 4.0 instead. 

Anyway, posting updated patches here but no rush in merging them. I should probably test them with something real too before merging.

Review of attachment 231457 [details] [review]:

::: gvc-mixer-card.c
@@ +357,3 @@
 
+static gint
+find_port_by_name (const GvcMixerCardPort *p, const char *n)

That's not a great name. I would expect the function that ran g_list_find_custom() to be called that.

port_compare()?

@@ +375,3 @@
+ * Adds or updates a port.
+ * @port: (transfer full): Template for port to add or update.
+ * Return value: (transfer none) The port added or updated, or NULL if the function

Missing ":" after the parenthesis

@@ +379,3 @@
+ * one passed.
+ */
+const GvcMixerCardPort * gvc_mixer_card_ensure_port       (GvcMixerCard     *card,

Line feed after the return type.

@@ +403,3 @@
+
+        memcpy (p, port, sizeof (GvcMixerCardPort));
+        g_free (port);

free_port (link->data);
link->data = g_memdup (port, sizeof(GvcMixerCardPort));
return link->data;

@@ +412,3 @@
+ * @port_name: Delete the port with this name from the list.
+ */
+gboolean                 gvc_mixer_card_free_port         (GvcMixerCard     *card,

Missing line feed too.

_free_port_by_name() (to not confuse with free_port)

@@ +425,3 @@
+        free_port (link->data);
+        card->priv->ports = g_list_delete_link (card->priv->ports, link);
+        return TRUE;

Line feed before the return.

Review of attachment 231458 [details] [review]:

::: gvc-mixer-control.c
@@ +1231,3 @@
+        data.port_name = port_name;
+
+        dlist = g_hash_table_get_values (control->priv->ui_outputs);

Would adding new hashtables be useful to look this up? Instead of the current lookup mechanism.

If not, using g_hash_table_find() looks like it would be cleaner.

Review of attachment 231459 [details] [review]:

::: gvc-mixer-control.c
@@ +1867,3 @@
         gint i;
         GList *supported_profiles = NULL;
+        const GList *p;

Looks like a separate bug fix. Can you please split it out?

@@ +1989,3 @@
+        g_debug ("Card removal remove device %s",
+                 gvc_mixer_ui_device_get_description (device));
+        g_hash_table_remove (gvc_mixer_ui_device_is_output (device) ? control->priv->ui_outputs : control->priv->ui_inputs,

Run _device_is_output() once in the function please.

@@ +2002,3 @@
+{
+        /* Currently we only update the availability, since that's
+           the only thing that can change (as of PulseAudio 3.0). */

Put the comment above the code that exits out lower down in the code.

@@ +2006,3 @@
+        guint    signal;
+
+        new_avail = port->available != PA_PORT_AVAILABLE_NO;

brackets around the comparison.

@@ +2029,3 @@
+
+/**
+ * If should_exist == TRUE, this function adds or updates a UI device.

Given the code in the function, a separate "remove_ui_device" would be cleaner.

@@ +2129,3 @@
+
+        for (i = 0; i < info->n_ports; i++) {
+                GvcMixerCardPort *port;

line feed here

@@ +2173,3 @@
+        }
+
+        while (ports_to_delete) {

I prefer for() loops for iterating over lists.

(In reply to comment #0)
> With PulseAudio 3.0, Bluetooth ports can sometimes appear after a card has been
> created. Therefore catch new ports as they appear.

Is that the functionality you told me was going to be removed from PA 4.0?

Looks like bug 697545 is the same problem.

The functionality (dynamic port creation/deletion) is not present in either 3.0, nor will it be in 4.0.