Ubuntu
pulseaudio package

Short-circuit in pm-utils hook can cause incorrect mixer state to be (re)stored

Bug #569395 reported by Chow Loong Jin
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
pulseaudio (Ubuntu)
Fix Released
Low
Daniel T Chen
Ubuntu lucid-updates
Lucid
Won't Fix
Low
Daniel T Chen
Ubuntu lucid-updates

Bug Description

-- Lucid SRU report follows --

Impact: A short-circuit in PulseAudio's pm-utils hook to check for the ConsoleKit active seat when (re)storing sink and source states can result in the wrong states being stored. (This change was introduced for suspend-to-* and resume optimization.)

Bug resolution: Prior to suspend-to-*, save all PA users' sink & source states and then mute only the active ConsoleKit seat's sink & source. After resume, restore all PA users' pre-suspend sink & source states. This change has been made in revision 292: http://bazaar.launchpad.net/~ubuntu-core-dev/pulseaudio/ubuntu.2a/revision/292?remember=284&compare_revid=284 .

Minimal patch: http://bazaar.launchpad.net/%7Eubuntu-core-dev/pulseaudio/ubuntu.2a/diff/292/284?remember=284

TEST CASE: Detailed below in the original defect report

Regression potential: Suspend-to-* and resume will require [negligibly] more time. Otherwise, there are no identified regressions.

-- Original defect report follows --

Binary package hint: pulseaudio

I noticed this the other day after accidentally going into "Switch User" mode: I had my own pulseaudio instance, and gdm had its own. I then suspending my machine, and upon resuming, sound was muted.

The reason for this happening is due to a bug in the 01Pulseaudio pm-utils hook.
1. First, it checks the status of gdm's pulseaudio, stores it, then mutes it.
2. Then, it checks the status of my pulseaudio, stores it, and also mutes it. Due to #1, the sink is already muted, as both sinks were pointing to the same physical device. Hence, the stored states are as follows: gdm's sink is not muted, but mine is.

Upon resuming:
1. It reads gdm's state, and unmutes the sink.
2. Then it reads my pulseaudio's saved state, and mutes the sink again.

Result: The sink is muted after resuming from suspend.

N.B.:
This seems to be a corner case that only occurs when multiple pulseaudio instances are running, and both contain sinks pointing to the same underlying device. It can probably be fixed by changing the pm-utils hook order to read and store all the states of the sinks prior to muting them.

See original description
Tags: testcase
Revision history for this message
Daniel T Chen (crimsun) wrote : Re: [Bug 569395] [NEW] Having multiple pulseaudio instances open causes physical sinks to be muted upon resume

Actually, the proper fix is to rip out the pm-utils hook altogether. It's merely a workaround for broken HDA drivers (sadly, nearly all of them) anyhow.

Revision history for this message
Chow Loong Jin (hyperair) wrote :

On Saturday 24,April,2010 08:26 PM, Daniel T Chen wrote:
> Actually, the proper fix is to rip out the pm-utils hook altogether.
> It's merely a workaround for broken HDA drivers (sadly, nearly all of
> them) anyhow.
>

But are all the drivers fixed yet? From my memory, the HDA drivers had issues
with pulseaudio holding onto the sound card while suspending, causing them to
have issues resuming later on, and resulting in a hung pulseaudio. At least,
that is what prompted me to write that script. On my system, currently, (custom
2.6.34-rc5 + sound-2.6.git) it works fine without the script.

--
Kind regards,
Chow Loong Jin

Revision history for this message
Daniel T Chen (crimsun) wrote : Re: [Bug 569395] [NEW] Having multiple pulseaudio instances open causes physical sinks to be muted upon resume

As discussed on irc, the way forward is twofold:

On the driver side, we need to fix the remaining controller/codec combos. This is fairly limited to Cirrus and some Conexant (on NVidia).

On the userspace side, the suspend and resume states need to be exposed over dbus.

Revision history for this message
Daniel T Chen (crimsun) wrote : Re: Having multiple pulseaudio instances open causes physical sinks to be muted upon resume

This is an ideal lucid-proposed candidate. We just need to remove the ck check and conditional (and break) from suspend_pulse() and resume_pulse(). If someone can prepare the description of this bug and generate a debdiff, that would be great.

Changed in pulseaudio (Ubuntu):
status: New → Triaged
importance: Undecided → Low
Revision history for this message
Daniel T Chen (crimsun) wrote :

I've made this change in revision 290.

summary: - Having multiple pulseaudio instances open causes physical sinks to be
- muted upon resume
+ Short-circuit in pm-utils hook can cause incorrect mixer state to be
+ (re)stored
Daniel T Chen (crimsun)
description: updated
Changed in pulseaudio (Ubuntu Lucid):
milestone: none → lucid-updates
assignee: nobody → Daniel T Chen (crimsun)
status: Triaged → Fix Committed
Revision history for this message
Chow Loong Jin (hyperair) wrote : Re: [Bug 569395] Re: Short-circuit in pm-utils hook can cause incorrect mixer state to be (re)stored

Looking at the minimal patch linked in the description, I actually think it
would still cause issues, as the issue was the order of muting/saving the
sessions. Basically:

1. Save user A's state
2. Mute user A's sink+source
3. Save user B's state
4. Mute user B's sink+source
---suspend and resume---
5. Read and restore user A's sink+source state (not muted)
6. Read and restore user B's sink+state (muted)

But step #2 caused step #3 to be saved as muted rather than not muted, since the
physical sinks are the same.

I think the solution would be to loop twice through the pulse users, saving the
states the first round, and muting+suspending the sinks and sources the second
round.

Also, I am still using Ubuntu Karmic, and the 01Pulseaudio hook in Karmic does
not have the ConsoleKit bits.

--
Kind regards,
Chow Loong Jin

Daniel T Chen (crimsun)
description: updated
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package pulseaudio - 1:0.9.22~0.9.21+stable-queue-32-g8478-0ubuntu15

---------------
pulseaudio (1:0.9.22~0.9.21+stable-queue-32-g8478-0ubuntu15) maverick; urgency=low

  * 0093-backport-fixes-stable-queue-head.patch: Backport the following
    changesets from the stable-queue branch:
    + bc7314f (name all threads for /proc/$PID/task/$TID/comm)
    + d519ca4 (prevent crash on jack server shutdown) (LP: #538815)
    + 65f89dc (fix bracketing in pa_rtp_recv())
  * debian/01PulseAudio: modify the ConsoleKit active seat checks in the
    pm-utils hook so that:
    - pre-suspend: all users' sink & source states are stored, then only
      the active seat's sinks and sources are muted
    - post-resume: all users' sink & source pre-suspend states are
      restored
    This resolves the issue where users' post-resume sink & source
    states don't correspond with their pre-suspend ones.
    (LP: #569395)
  * debian/01PulseAudio: fix broken quoting and misuse of su -l. Merged
    from lp:~hyperair/pulseaudio/fix-01Pulseaudio-quoting. Thanks,
    Chow Loong Jin! (LP: #572391)
  * debian/control: Update bzr branch for maverick.
 -- Daniel T Chen <email address hidden> Sat, 08 May 2010 11:08:56 -0400

Changed in pulseaudio (Ubuntu):
status: Fix Committed → Fix Released
Revision history for this message
Luke Yelavich (themuso) wrote :

The code for this is sitting in bzr, but it doesn't seem this was uploaded to lucid-proposed. Should we push this to lucid-proposed for woder testing?

Revision history for this message
Luke Yelavich (themuso) wrote :

Is there anybody still using lucid, who is affected by this bug?

Ubuntu Foundations Team Bug Bot (crichton)
tags: added: testcase
Revision history for this message
Rolf Leggewie (r0lf) wrote :

lucid has seen the end of its life and is no longer receiving any updates. Marking the lucid task for this ticket as "Won't Fix".

Changed in pulseaudio (Ubuntu Lucid):
status: Fix Committed → Won't Fix
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.