PA: Don't restore the streams to sinks/sources with only unavailable ports

Bug #1834138 reported by Hui Wang
18
This bug affects 1 person
Affects Status Importance Assigned to Milestone
HWE Next
Fix Released
Undecided
Unassigned
OEM Priority Project
Fix Released
High
Unassigned
pulseaudio (Ubuntu)
Fix Released
High
Hui Wang
Bionic
Fix Released
High
Hui Wang
Disco
Fix Released
High
Unassigned
Eoan
Fix Released
High
Hui Wang

Bug Description

SRU Document:

[Impact]

The Lenovo P520 machine has dual analogue codecs, so there are two sinks and two sources in the PA, one has the front headphone and front microphone, the other has the rear lineout, linein and rear microphone, and the rear microphone always shows up in the gnome-sound-setting, When we plug a microphone to front audio jack, there are two input devices: rear mic and front mic in the gnome-sound-setting, and suppose users select the the front mic to record sound via audio app like arecord, the front mic will be bond the arecord, after the front mic is unplugged, there is only one rear mic left in the gnome-sound-setting, but the binding will not be changed, the arecrod still bind to front mic, under this situation if users record sound via arecord, they will find they can't record any sound from any other input devices even they are listed in the gnome-sound-setting. This problem also happens to output devices too.

[Test Case]

After applying this patch, I did the same test: unplug the front mic, then use the arecord to record sound, the app can record sound from rear mic now. After I plug the front mic back, the arecord still record from front mic. Also did the similar test for output devices, it worked as expected too.

[Regression Potential]

Low, Just make a simple check when creating new streams (sink_input/source_output), If the restored device (sink/source) has ports and all ports are unavialble, it will not restore the binding, otherwise it will work as before.

For the Bionic, This SRU also includes the fix of LP: #1556439, this fix is safe and is very low possible to introduce any regression too, because it just adds a sink-input/source-output state checking, if the sink-input/source-output is unlinking or unlinked, it is useless to move it to a new sink/source, furthermore it will trigger an assertion that make the pulseaudio crash, adding this check can fix this problem (LP: #1556439).

[Other Info]

No more info here

Revision history for this message
Hui Wang (hui.wang) wrote :

Probably we need to backport this patch:

commit 30a551bbc45f2d213e8b2889c8bede8a9c16c9d2
Author: João Paulo Rechi Vita <email address hidden>
Date: Mon Dec 10 16:16:46 2018 -0800

    switch-on-port-available: Check if we need to change the active profile

    When a port becomes unavailble its profile may also become unavailable.
    If that profile is the card's active profile, we need to switch the
    card's active profile to a different one.

    If we don't do that a card may get stuck on a profile without available
    ports, but its sink and source will still exist, preventing
    module-rescue-streams to move the streams to a different card with
    available ports.

    The relation between port availability and profile availability is
    defined by the driver, and for the ALSA driver a profile is considered
    available if there is at least one (available || unknown) port for each
    direction implemented by the profile. Because of that we can only check
    the profile's availability and priority when looking for the best
    profile and don't need to look at port's priorities.

    https://phabricator.endlessm.com/T24904

Changed in pulseaudio (Ubuntu):
importance: Undecided → Critical
Revision history for this message
Hui Wang (hui.wang) wrote :

And this patch:

commit cbaeea4af7669003ae97064fe12fa75fd4870611
Author: Hui Wang <email address hidden>
Date: Wed May 15 14:39:27 2019 +0800

    stream-restore: Don't restore if the active_port is PA_AVAILABLE_NO

    We met two problems recently, one happened on a Lenovo machine with
    dual analogue codecs, the other happened on a Dell machine with
    a digital mic directly connected to PCH. The two problems are
    basically same, there is an internal mic and an external mic, the
    internal mic always shows up in the gnome-control-center, the external
    mic only shows up when it is plugged. After the external mic is
    plugged and users select it from gnome-control-center, the
    gnome-control-center will read all saved streams through extension_cb,
    and bind the source of external mic to all streams, after that the
    apps only record sound via the source of external mic, after the
    external mic is unplugged, the internal mic will automatically be
    selected since it is the only left input device in the
    gnome-control-center, since users don't select it, all streams are
    still bond the source of external mic. When users record sound via
    apps, they can't record any sound even the default_source is the
    source of internal mic and the internal mic is selected in the UI.

    It is very common that a machine has internal mic and external mic,
    but this problem didn't expose before, that is because both internal
    mic and external mic belong to one source, but for those two
    machines, the internal mic belongs to one source, while the external
    mic belongs to another source (they are in differnt codecs or one is
    in the codec and the other is from PCH),

    To fix it with a mininal change, we just check if the active_port is
    PA_AVAILABLE_NO or not when building a new stream, if it is, don't
    restore the device to the new built stream, let pa_source_output_new()
    decide the source device for this stream.

    And we also do the same change to sink_input.

    This change only affects the new built streams, it will not change
    the database, so the users' preference is still saved in the database,
    after the active_port is not PA_AVAILABLE_NO, the new streams will
    still restore to the preferred device.

    Signed-off-by: Hui Wang <email address hidden>

tags: added: eoan
Hui Wang (hui.wang)
tags: added: originate-from-1833676 sutton
Changed in pulseaudio (Ubuntu):
importance: Critical → High
Revision history for this message
Sebastien Bacher (seb128) wrote :

@Hui, could you describe what's the usecase/user story so we understand exactly the impact?

Hui Wang (hui.wang)
Changed in pulseaudio (Ubuntu):
status: New → Incomplete
Hui Wang (hui.wang)
description: updated
Hui Wang (hui.wang)
description: updated
Revision history for this message
Hui Wang (hui.wang) wrote :

Since the P520 machine uses ucm in the PA, the patch in the #1 doesn't fix any problem for this machine, so only need to backport the patch of #2.

summary: - PA: can't auto switch streams between different sinks
+ PA: Don't restore the streams to sinks/sources with only unavailable
+ ports
description: updated
Revision history for this message
Hui Wang (hui.wang) wrote :

This is the debdiff for disco version, after it is merged any verified, I will continue uploading the debdiff for cosmic and bionic.

So please put this change into the disco queue first. thanks.

Revision history for this message
Sebastien Bacher (seb128) wrote :

@Hui, thanks for the work. Is that bug fixed in eoan? It needs to be resolved there before being SRUed

Changed in pulseaudio (Ubuntu):
status: Incomplete → New
Revision history for this message
Hui Wang (hui.wang) wrote :

Reply #6:

The ubuntu 19.10 (Eaon) also has this problem, need to apply this patch as well. And since both eaon and disco share the same pulseaudio version, the debdiff in the #5 also can apply to eaon, let us apply it to eaon first.

Revision history for this message
Hui Wang (hui.wang) wrote :

@Sebastien,

I checked the ubuntu eaon also uses pulseaudio_12.2-2ubuntu3 which is same as the disco uses, so the debdiff in the #5 can be applied to eaon too, could we apply the #5 to eaon first? or I need to re-generate a debdiff for eaon?

thx.

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package pulseaudio - 1:12.2-2ubuntu4

---------------
pulseaudio (1:12.2-2ubuntu4) eoan; urgency=medium

  * debian/patches/0006-load-module-x11-bell.patch:
    - remove that old distro patch, it's undocumented, not needed since
      GNOME handles the login sound by itself and create issues in some
      cases (duplicate sound, not respecting the UI choice)
      (lp: #1827842)

  [ Hui Wang ]
  * d/p/0800-stream-restore-Don-t-restore-if-the-active_port-is-P.patch:
    - Don't restore the streams to sinks/sources with only unavailable ports
      (LP: #1834138)

 -- Sebastien Bacher <email address hidden> Fri, 28 Jun 2019 11:45:10 +0200

Changed in pulseaudio (Ubuntu):
status: New → Fix Released
Revision history for this message
Hui Wang (hui.wang) wrote :

Installed the latest eaon image of http://cdimage.ubuntu.com/ubuntu/daily-live/current/ to the lenovo p520, and upgrade the pulseaudio to 1:12.2-2ubuntu4, Then I tested the recording and palyback, it worked as expected:

recording: plug a mic into the front audio jack, and select it from sound-setting, use arecord to record sound, after recording, unplug the front mic and plug it to rear mic, use arecord to record sound again, it still can record sound.

playback: plug a headphone to rear lineout and select lineout as output device, then play sound via totem, after a while, close the totem and unplug the rear lineout and plug the headphone to front headphone jack, use totem to play sound, I can hear the sound from front headphone.

It proves after applying the patch, both playback and recording works as expected.

Please push the debdiff to disco.

thx.

Revision history for this message
Sebastien Bacher (seb128) wrote :

The SRU has been uploaded to Disco as well, it's waiting for SRU team review

Revision history for this message
Brian Murray (brian-murray) wrote : Please test proposed package

Hello Hui, or anyone else affected,

Accepted pulseaudio into disco-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/pulseaudio/1:12.2-2ubuntu3.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-disco to verification-done-disco. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-disco. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Changed in pulseaudio (Ubuntu Disco):
status: New → Fix Committed
tags: added: verification-needed verification-needed-disco
Revision history for this message
Hui Wang (hui.wang) wrote :

Verification done on the Disco, it fixed the problem.

On the lenovo P520, I installed the disco (19.04), edit the /etc/apt/sources.list to add disco-proposed repository, then run apt-get update abd sudo apt install pulseaudio, now the pulseaudio is:
ii pulseaudio 1:12.2-2ubuntu3.1 amd64 PulseAudio sound server
ii pulseaudio-utils 1:12.2-2ubuntu3.1 amd64 Command line tools for the PulseAudio sound server

recording: plug a mic into the front audio jack, and select it from sound-setting, use arecord to record sound, after recording, unplug the front mic and plug it to rear mic, use arecord to record sound again, it still can record sound.

playback: plug a headphone to rear lineout and select lineout as output device, then play sound via totem, after a while, close the totem and unplug the rear lineout and plug the headphone to front headphone jack, use totem to play sound, I can hear the sound from front headphone.

tags: added: verification-done-disco
removed: verification-needed-disco
Revision history for this message
Hui Wang (hui.wang) wrote :

This is the debdiff for Cosmic.

Thanks.

Revision history for this message
Sebastien Bacher (seb128) wrote :

Thanks for the new debdiff but I don't think it makes sense to do a Cosmic SRU at this point since it's a deprecated serie

Revision history for this message
Hui Wang (hui.wang) wrote :

OK, let us skip the cosmic, I will upload a debdiff for bionic.

Thanks.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Please don't upload anything for bionic till this is done:
https://bugs.launchpad.net/ubuntu/+source/pulseaudio/+bug/1556439/comments/32

Revision history for this message
Hui Wang (hui.wang) wrote :

OK, got it.

Revision history for this message
Hui Wang (hui.wang) wrote :

Sporsor-team, this is the debdiff for bionic.

thx.

Alex Tu (alextu)
Changed in oem-priority:
importance: Undecided → High
Revision history for this message
Hui Wang (hui.wang) wrote :

@sebastien or anyone else in the sponsor-team,

Please review my debdiff for bionic in the #19 and if possible, please enqueue it.

thx.

tags: added: oem-priority
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

For PulseAudio changes, feel free to commit them to git immediately, just don't tag them:

  https://git.launchpad.net/~ubuntu-audio-dev/pulseaudio/log/?h=ubuntu-bionic

They will then be included in whatever version we manage to successfully release next.

Mathew Hodson (mhodson)
Changed in pulseaudio (Ubuntu Disco):
importance: Undecided → High
Changed in oem-priority:
status: New → Confirmed
tags: added: originate-from-1835155
Changed in oem-priority:
importance: High → Critical
Changed in pulseaudio (Ubuntu Bionic):
status: New → In Progress
importance: Undecided → High
assignee: nobody → Hui Wang (hui.wang)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Ignore comment #22, and please remove comment #19. I will make a new version today.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Please use this debdiff for the bionic SRU instead.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

I think this probably should be reworded:

[Regression Potential]

No, ...

Revision history for this message
Daniel van Vugt (vanvugt) wrote :
Revision history for this message
Brian Murray (brian-murray) wrote :

Please update the "Regression Potential" of the bug description. Additionally, I think this should be reuploaded to bionic-proposed queue in such a fashion that 1556439 will appear in Launchpad-Bugs-Fixed so that we reverify the fix for that bug when verifying the new pulseaudio upload.

Changed in pulseaudio (Ubuntu Bionic):
status: In Progress → Incomplete
Revision history for this message
Hui Wang (hui.wang) wrote :

"Regression Potential" is updated with adding the 1556439 part in it.

description: updated
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

"Regression Potential" also shouldn't start with "No".

Revision history for this message
Hui Wang (hui.wang) wrote :

addressed the #29, change No to Low.

description: updated
description: updated
Mathew Hodson (mhodson)
Changed in pulseaudio (Ubuntu Bionic):
status: Incomplete → Triaged
Revision history for this message
Robie Basak (racb) wrote :

> Additionally, I think this should be reuploaded to bionic-proposed queue in such a fashion that 1556439 will appear in Launchpad-Bugs-Fixed so that we reverify the fix for that bug when verifying the new pulseaudio upload.

I don't see any change in the upload. Please could you address Brian's comment? If it is no longer relevant, an explanation would be helpful.

Changed in pulseaudio (Ubuntu Bionic):
status: Triaged → Incomplete
Revision history for this message
Hui Wang (hui.wang) wrote :

@Daniel van,

Do you know how to "reupload to bionic-proposed queue in such a fashion that 1556439 will appear in Launchpad-Bugs-Fixed"?

Revision history for this message
Sebastien Bacher (seb128) wrote :

@Brian, it has been reuploaded with the previous changelog entry included

Revision history for this message
Łukasz Zemczak (sil2100) wrote :

@Daniel @Sebastien the upload generally looks good, but I'm slightly worried about the Launchpad-Bugs-Fixed: including the bug number of 1663528 - which has been reverted as part of the upload. We don't really have good ways to tag a bug as 'fix reverted' via the LP changelog parsing, and when we accept the SRU right now, the bug will be marked for verification and we'd have to make sure to switch it to 'Won't Fix' again after release. I guess the easiest way would be to re-upload 1:11.1-1ubuntu7.4 without the changelog entry of 1:11.1-1ubuntu7.3, but with an additional mention of a fix for 1556439 in one version. I would normally re-upload myself to save you the trouble, but I guess that would break your workflow (I got yelled at for doing that once before).
Could you consider re-uploading with that changelog modification? It would save much trouble and possible future confusion. Thanks.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

I understand the concern but would personally prefer keeping it as-is. It's better to have a paper trail than to hide any history of what came before.

I'm happy to manually set bug 1663528 to Won't Fix again for bionic.

Revision history for this message
Chris Halse Rogers (raof) wrote : Proposed package upload rejected

An upload of pulseaudio to bionic-proposed has been rejected from the upload queue for the following reason: "The changes file claims to fix #1663528, but this won't. We don't need to preserve the changelog of -proposed packages, so drop it from the 1ubuntu7.3 entry".

Revision history for this message
Chris Halse Rogers (raof) wrote : Please test proposed package

Hello Hui, or anyone else affected,

Accepted pulseaudio into bionic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/pulseaudio/1:11.1-1ubuntu7.4 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-bionic to verification-done-bionic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-bionic. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Changed in pulseaudio (Ubuntu Bionic):
status: Incomplete → Fix Committed
tags: added: verification-needed-bionic
Revision history for this message
Hui Wang (hui.wang) wrote :

Verification done on the Bionic, it fixed the problem.

On the lenovo P520, I installed OEM image (18.04), edit the /etc/apt/sources.list to add bionic-proposed repository, then run apt-get update abd sudo apt install pulseaudio, now the pulseaudio is:
ii pulseaudio 1:11.1-1ubuntu7.4 amd64 PulseAudio sound server
ii pulseaudio-utils 1:11.1-1ubuntu7.4 amd64 Command line tools for the PulseAudio sound server

recording: plug a mic into the front audio jack, and select it from sound-setting, use arecord to record sound, after recording, unplug the front mic and plug it to rear mic, use arecord to record sound again, it still can record sound.

playback: plug a headphone to rear lineout and select lineout as output device, then play sound via totem, after a while, close the totem and unplug the rear lineout and plug the headphone to front headphone jack, use totem to play sound, I can hear the sound from front headphone.

tags: added: verification-done verification-done-bionic
removed: verification-needed verification-needed-bionic
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package pulseaudio - 1:12.2-2ubuntu3.1

---------------
pulseaudio (1:12.2-2ubuntu3.1) disco; urgency=medium

  * debian/patches/0006-load-module-x11-bell.patch:
    - remove that old distro patch, it's undocumented, not needed since
      GNOME handles the login sound by itself and create issues in some
      cases (duplicate sound, not respecting the UI choice)
      (lp: #1827842)

  [ Hui Wang ]
  * d/p/0800-stream-restore-Don-t-restore-if-the-active_port-is-P.patch:
    - Don't restore the streams to sinks/sources with only unavailable ports
      (LP: #1834138)

 -- Sebastien Bacher <email address hidden> Fri, 28 Jun 2019 11:45:10 +0200

Changed in pulseaudio (Ubuntu Disco):
status: Fix Committed → Fix Released
Revision history for this message
Łukasz Zemczak (sil2100) wrote : Update Released

The verification of the Stable Release Update for pulseaudio has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

Changed in oem-priority:
importance: Critical → High
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package pulseaudio - 1:11.1-1ubuntu7.5

---------------
pulseaudio (1:11.1-1ubuntu7.5) bionic; urgency=medium

  * Update snap policy to make access to audio recording conditional on
    plugging the "pulseaudio" or "audio-record" interfaces (LP: #1781428):
    - 0700-modules-add-snappy-policy-module.patch: rewrite to query
      snapd for the client's plugged interfaces.
    - 0701-enable-snap-policy-module.patch: enable the module in the
      default configuration.
    - Build depend on libsnapd-glib-dev.
  * Remove module-trust-store patch set:
    - 0409-Trust-store-patch.patch: trimmed down to pulsecore changes.
    - 0410-Add-thread-to-activate-trust-store-interface.patch: removed.
    - 0417-increase-timeout-check-apparmor.patch: removed.

 -- James Henstridge <email address hidden> Wed, 05 Nov 2019 17:16:25 +0800

Changed in pulseaudio (Ubuntu Bionic):
status: Fix Committed → Fix Released
Changed in hwe-next:
status: New → Fix Released
Rex Tsai (chihchun)
Changed in oem-priority:
status: Confirmed → Fix Committed
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

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