Match device IDs rather than name substrings for autoloading

[andersk]

• Fixes #3180 more robustly

[wwmm]

Using id numbers is not a good idea. I used to do this in the past in other places and sooner or later the logic breaks because PipeWire reuses id numbers.

[wwmm]

The solution has to be based only on node/device name. It sucks but it is what it is. I talked about this a long time ago to PipeWire's developers and they do not recommend using the id numbers as some form of unique identifier.

[andersk]

I understand that ID numbers can be reused across different times, but surely they can’t be reused for multiple devices at the same time?

[wwmm]

I understand that ID numbers can be reused across different times, but surely they can’t be reused for multiple devices at the same time?

I used the id numbers in the past. The issue is that PipeWire does not look at id numbers as unique identifiers. One day some things randomly stopped working in "mysterious" ways and after spending hours trying to understanding why the hell using id numbers were breaking things I made contact with them. And the conclusion was that I was wrong about assuming that id numbers could be used as an unique identifier. Only the name could be used this way. Since then I stopped relying on id numbers.

[wwmm]

Sometimes things will look just fine with id numbers. But then when we start to plug/unplug devices unexpected things happen. It is not worth the headache they bring.

[wwmm]

At the time I thought "maybe if I use serial numbers instead of id numbers things will be ok". But eventually they also caused some bugs I do not remember very well now. That is why the whole autoloading logic is now based on names.

One of the reasons why autoloading is tricky is because the server's behavior changes with the hardware. For example on some soundcards the node is recreated when a headphone is plugged. On others the node is preserved and just the routing changes. The same annoyance happened on Pulseaudio. It is probably related to how the respective driver works.

[wwmm]

In any case I wonder why the previous patch did not work on your hardware. The list of possible matches did not match in the final comparison?

[andersk]

This patch does not assume ID numbers remain unique across time. The only way I’m using the device ID is to correlate nodes with the corresponding devices at any particular instant. So, the only invariant I rely on is that if a device is removed, all of the corresponding nodes are removed too. Then it doesn’t matter if the ID is reused later because there’s nothing leftover to refer to it.

If you are worried that even that invariant is violated, we could enforce it ourselves by walking through node_map in on_destroy_device_proxy; would that satisfy your concern?

[wwmm]

This patch does not assume ID numbers remain unique across time. The only way I’m using the device ID is to correlate nodes with the corresponding devices at any particular instant. So, the only invariant I rely on is that if a device is removed, all of the corresponding nodes are removed too. Then it doesn’t matter if the ID is reused later because there’s nothing leftover to refer to it.

If you are worried that even that invariant is violated, we could enforce it ourselves by walking through node_map in on_destroy_device_proxy; would that satisfy your concern?

As you can see in one of our legacy branches https://github.com/wwmm/easyeffects/blob/57cd192f7a09ed03daaa34c8493d13c1275777e7/src/application.cpp#L191 I used the node.device_id == device.id approach in the past. This failed in some situations.

[wwmm]

I still think it is not worth to go back to the past and spending energy again trying a solution based on the id numbers. It is better to figure out how to make the approach based on names to work.

[andersk]

In any case I wonder why the previous patch did not work on your hardware. The list of possible matches did not match in the final comparison?

To be clear, #3181 does work on my hardware—but I’m trying to find a way to improve the solution for everyone, because substring matches always have unexpected consequences (like, usb-0:8:1.1 is a substring of usb-0:8:1.12).

I used the node.device_id == device.id approach in the past. This failed in some situations.

Looking back at that legacy branch, it took the first node associated with the device and ignored all the others. We know exactly why that could fail.

[wwmm]

In any case I wonder why the previous patch did not work on your hardware. The list of possible matches did not match in the final comparison?

To be clear, #3181 does work on my hardware—but I’m trying to find a way to improve the solution for everyone, because substring matches always have unexpected consequences (like, usb-0:8:1.1 is a substring of usb-0:8:1.12).

I used the node.device_id == device.id approach in the past. This failed in some situations.

I understand that you are trying to help to build a more robust solution. But at the same time I do not want to take the risk of reintroducing the subtle bugs of the past that only manifest themselves in specific hardware...

Looking back at that legacy branch, it took the first node associated with the device and ignored all the others. We know exactly why that could fail.

Sure we can see why the old approach would fail in a hardware like yours. But not all cases where I've seen the old approach failing had devices with almost identical names where the wrong one was chosen. For example I have 3 different sound cards on my computer with completely different names. And the issue I talked about could still happen. While at the same time the particular issue you've seen just never happened here... Like I said before things can change in surprising ways when changing the hardware. So far the most reliable approach across different kind of hardware has been the one based on names. I do not see an advantage in investigating the whole thing again...