| Summary: | pa_namereg_get_default_* has unwanted side-effects | ||
|---|---|---|---|
| Product: | PulseAudio | Reporter: | Arun Raghavan <arun> |
| Component: | core | Assignee: | Arun Raghavan <arun> |
| Status: | RESOLVED FIXED | QA Contact: | pulseaudio-bugs |
| Severity: | normal | ||
| Priority: | medium | CC: | lennart |
| Version: | unspecified | ||
| Hardware: | Other | ||
| OS: | All | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
| Attachments: |
namereg: Don't set default sink/source on get()
namereg: Don't set default sink/source on get() |
||
I'll push a fix for this in a bit. Careful there. I already removed the side effect in <http://cgit.freedesktop.org/pulseaudio/pulseaudio/diff/src/pulsecore/namereg.c?id=9347e90fed732dac619bb88f6518c344e7436447>. It caused some trouble, and I reverted it in <http://cgit.freedesktop.org/pulseaudio/pulseaudio/commit/?id=f48684e4dbc00d102ed17700fb693726a2676566>. Unfortunately, the revert commit message sucks (it doesn't say why that was done), and I don't exactly remember what the problem was. I think it was related to events where one sink disappears and immediately after that another appears. I remember discussing this with Lennart in IRC. If someone has IRC logs from 2009 August, that might be useful... Created attachment 51733 [details] [review] namereg: Don't set default sink/source on get() This removes the nasty side-effect that a call to pa_namereg_get_default_{source,sink}() will also *set* the default source/sink. This is a more complete fix for commit 766dbc68 ("conf: Make sure module-dbus-protocol is loaded after module-default-device-restore") I've attached the patch I mean to commit. As far as I can tell, it shouldn't cause any fallout like the commits you mentioned. Sorry, actually the revert seems to mostly happen already in this merge commit: <http://cgit.freedesktop.org/pulseaudio/pulseaudio/commit/src/pulsecore/namereg.c?id=bcaba0b1b43d6a1b32aadfa98860f40b2c93e136>. (In reply to comment #4) > I've attached the patch I mean to commit. As far as I can tell, it shouldn't > cause any fallout like the commits you mentioned. I think it does cause problems. See the comment and code at the end of pa_namereg_register() - if there is no default device set when a new device is registered, the default device doesn't get set. (In reply to comment #6) > I think it does cause problems. See the comment and code at the end of > pa_namereg_register() - if there is no default device set when a new device is > registered, the default device doesn't get set. As far as I can tell, that's just documenting the (IMO erroneous) behaviour -- everything access the default device via the pa_namereg_get_*() calls, so it's fine that the default device is not set -- it's a slightly more expensive operation, but it does mean that we're not forcing a setting that was not explicitly requested. The following fix has been pushed: 5c28acb namereg: Don't set default sink/source on get() Created attachment 52169 [details] [review] namereg: Don't set default sink/source on get() This removes the nasty side-effect that a call to pa_namereg_get_default_{source,sink}() will also *set* the default source/sink. This is a more complete fix for commit 766dbc68 ("conf: Make sure module-dbus-protocol is loaded after module-default-device-restore") |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.
As David reports, pa_namereg_get_default_{source,sink}() end up *setting* the default source and sink as well. This appears to be premature optimisation and probably should not be done.