Comment 13 for bug 1985057

Revision history for this message
Robie Basak (racb) wrote :

This looks like a memory management bug in fairly central code in pipewire upstream that has been fixed upstream with some significant refactoring. It is only two months old so can’t have had very wide testing in the ecosystem. In this time pipewire upstream have made a significant number of releases. And there is a subsequent upstream commit that fixes a race that I’m not sure is related or not. Possibly more than one. See for example: https://gitlab.freedesktop.org/pipewire/pipewire/-/commits/master/src/gst/gstpipewiresrc.c

The above assessment comes from a quick glance, not a detailed analysis. If this analysis is wrong I welcome being corrected.

For now, it does really feel like upstream are iterating here in a way that we really want to avoid doing in an LTS.

It’s clearly necessary to fix this bug, and cherry-picking from upstream does seem appropriate. However, for the above reasons I consider the risk of regression in this case to be particularly high. In particular I think the risk is significant that this sort of change will fix one piece of hardware at the cost of regressing another.

So to mitigate the regression potential, I think a substantial test plan is required. For example, I don’t think it’s sufficient to verify the fix only on the affected hardware and only with one application.

I’m not the expert here so I don’t want to dictate the specifics here, but for example I think you should plan to test:

A variety of different hardware, where the variety tries to cover the different factors that affect why this bug affects only this hardware in the first place (for example, also include different video capture resolutions)
A variety of different apps where the variety tries to cover different configurations, behaviours or code paths, as well as key use cases. Example: Google Meet with Chromium and Firefox, Zoom, screen sharing vs. webcam etc.

Please could you develop a Test Plan accordingly and update the bug? Then we can look again.

Thanks!