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

Bug #1834138 reported by Hui Wang on 2019-06-25
20
This bug affects 1 person
Affects Status Importance Assigned to Milestone
HWE Next
Undecided
Unassigned
OEM Priority Project
Critical
Unassigned
pulseaudio (Ubuntu)
Status tracked in Eoan
Bionic
High
Hui Wang
Disco
High
Unassigned
Eoan
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

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
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) on 2019-06-25
tags: added: originate-from-1833676 sutton
Changed in pulseaudio (Ubuntu):
importance: Critical → High
Sebastien Bacher (seb128) wrote :

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

Hui Wang (hui.wang) on 2019-06-25
Changed in pulseaudio (Ubuntu):
status: New → Incomplete
Hui Wang (hui.wang) on 2019-06-25
description: updated
Hui Wang (hui.wang) on 2019-06-25
description: updated
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
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.

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
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.

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.

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
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.

Sebastien Bacher (seb128) wrote :

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

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
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
Hui Wang (hui.wang) wrote :

This is the debdiff for Cosmic.

Thanks.

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

Hui Wang (hui.wang) wrote :

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

Thanks.

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

Hui Wang (hui.wang) wrote :

OK, got it.

Hui Wang (hui.wang) wrote :

Sporsor-team, this is the debdiff for bionic.

thx.

Alex Tu (alextu) on 2019-07-23
Changed in oem-priority:
importance: Undecided → High
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
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.

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)
Daniel van Vugt (vanvugt) wrote :

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

Daniel van Vugt (vanvugt) wrote :

Please use this debdiff for the bionic SRU instead.

Daniel van Vugt (vanvugt) wrote :

I think this probably should be reworded:

[Regression Potential]

No, ...

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
Hui Wang (hui.wang) wrote :

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

description: updated
Daniel van Vugt (vanvugt) wrote :

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

Hui Wang (hui.wang) wrote :

addressed the #29, change No to Low.

description: updated
description: updated
Changed in pulseaudio (Ubuntu Bionic):
status: Incomplete → Triaged
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
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"?

Sebastien Bacher (seb128) wrote :

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

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers