firefox: enable pulseaudio by default

[lheckemann]

Motivation for this change

This fixes choppy audio in WebRTC. Firefox's closure already includes
libpulseaudio anyway, so this shouldn't affect closure size either.

Notes:

• This does not disable ALSA support. Firefox will still use ALSA if pulseaudio is unavailable.
• Previously suggested in WIP: firefox: enable pulseaudio by default in wrapper #24012. This was to resolve a "no sound" issue which was caused by upstream disabling ALSA by default (!). This is to resolve a "choppy sound" issue which is caused by an upstream bug in the (unmaintained, ref https://bugzilla.mozilla.org/show_bug.cgi?id=1345661) ALSA code.

I think backporting to 17.09 is also appropriate?

Things done

• Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
• Built on platform(s)
  • NixOS
  • macOS
  • other Linux distributions
• Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
• Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
• Tested execution of all binary files (usually in ./result/bin/)
• Fits CONTRIBUTING.md.

---

[Mic92]

I am surprised. I thought firefox dropped support for ALSA before.

[Mic92]

@GrahamcOfBorg build firefox

[GrahamcOfBorg]

Failure on x86_64-darwin (full log)

Partial log (click to expand)

Package ‘firefox-unwrapped-58.0.2’ in /Users/graham/nix-borg/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-ndnd/pkgs/applications/networking/browsers/firefox/packages.nix:23 is not supported on ‘x86_64-darwin’, refusing to evaluate.

a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowBroken = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowBroken = true; }
to ~/.config/nixpkgs/config.nix.

[GrahamcOfBorg]

Success on x86_64-linux (full log)

Partial log (click to expand)

  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed

  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
  8 37.0M    8 3162k    0     0  3188k      0  0:00:11 --:--:--  0:00:11 3185k
 78 6781k   78 5300k    0     0  5359k      0  0:00:01 --:--:--  0:00:01 5354k
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
100 6781k  100 6781k    0     0  5902k      0  0:00:01  0:00:01 --:--:-- 5902k


 84  879k   84  747k    0     0   674k      0  0:00:01  0:00:01 --:--:--  674k
 40 37.0M   40 15.1M    0     0  7522k      0  0:00:05  0:00:02  0:00:03 7522k
100  879k  100  879k    0     0   733k      0  0:00:01  0:00:01 --:--:--  732k


 78 37.0M   78 28.9M    0     0  9588k      0  0:00:03  0:00:03 --:--:-- 9588k
100 37.0M  100 37.0M    0     0   9.7M      0  0:00:03  0:00:03 --:--:--  9.7M

building path(s) ‘/nix/store/jk52gpjm3pnji51x7nzvs555h4c8qwc0-firefox-58.0.2’
/nix/store/jk52gpjm3pnji51x7nzvs555h4c8qwc0-firefox-58.0.2

[Mic92]

This flag does not trigger a firefox rebuild, so what is the effect of changing it from false to true?

[GrahamcOfBorg]

Failure on aarch64-linux (full log)

Partial log (click to expand)

   Compiling std v0.0.0 (file:///build/rustc-1.22.1-src/src/libstd)
building of '/nix/store/1vbyjba76gw7jcs9g5sqyb2bxsb0xpln-rustc-1.22.1.drv' timed out after 3600 seconds
cannot build derivation '/nix/store/wjjfbzq8gfxgbd3f426j8syljcqjdsk1-cargo-0.23.0.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/bvngc1qvckx0cm4lbcvjalnzi4bdy4dz-librsvg-2.42.2.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/mhzclr9bnpg5zgvx7whibbz9zl00hjkg-hook.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/pg03jgn06gghqq2m8wdwbi06fpkjakpw-icon-naming-utils-0.8.90.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/jwq5d7splqfkb625jgd6fqvfdvpbxn22-adwaita-icon-theme-3.26.1.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/d25py6zgbj8gwylrm2d6xxdkvdb7vzq3-firefox-unwrapped-58.0.2.drv': 3 dependencies couldn't be built
cannot build derivation '/nix/store/wp4s2k5rqdg5mrgp82bn59mivq35yyk8-firefox-58.0.2.drv': 2 dependencies couldn't be built
error: build of '/nix/store/wp4s2k5rqdg5mrgp82bn59mivq35yyk8-firefox-58.0.2.drv' failed

[lheckemann]

I am surprised. I thought firefox dropped support for ALSA before.

It still includes it, although according to irc.mozilla.org#media nobody's actually maintaining it.

This flag does not trigger a firefox rebuild, so what is the effect of changing it from false to true?

It makes the wrapper include libpulseaudio so firefox can actually find it at runtime. Compare the "audio backend" entry in about:support to see the difference (or try using jitsi meet!) :)

[oxij]

Does the "choppy audio in WebRTC" has a bug number on the bugtracker? I disagree with the direction. I think we should _remove_ pulseaudio from the closure and fix firefox like #34822 does for output. The fix is simple: replace all `config.pulseaudio or true` with `configu.pulseaudio or false` in nixpkgs. Please don't push broken software onto everyone!

[lheckemann]

Does the "choppy audio in WebRTC" has a bug number on the bugtracker?

Not that I know of.

Please don't push broken software onto everyone!

If with "broken software" you are referring to pulseaudio, this isn't nixpkgs pushing it, this is Mozilla pushing it. If you want to fix Firefox's ALSA support and make this change (along with pulseaudio's presence in firefox's closure) unnecessary be my guest! :) in the meantime pulseaudio is the audio backend Firefox actively supports so it's the one we should use by default. SLNOS can change that default of course!

padenot linus, about:support, can you tell me what the audio backend section says ?
linus   alsa
padenot ok
padenot then this is your issue
padenot nobody wants to support it and we don't have time to do it
padenot so after a couple years we kind of stopped

[oxij]

Not that I know of.
the IRC quotes

Well, all the firefox bugs I've ever encountered were either fixed by firefox devs or by somebody else in the community and the patch available from the bug tracker (even when mozilla says its a wontfix (usually for ridiculous reasons) like, e.g. https://bugzilla.mozilla.org/show_bug.cgi?id=178506). If it doesn't have a bug reference on the bugzilla I think it should be reported first.

I agree that we might need to do something special for firefox (because mozilla devs gone crazy), but I don't like your solution because it assumes that the only thing that can be done is us forcing users to run global pulseaudio. The problem with the latter is that it will make sound output choppy for all the apps on older hardware (pulseaudio tends to do that for me) and will significantly increase overall sound latency. (Note that I did not use the suckless argument, nor the security argument, I'm trying really hard to be "purely pragmatic" here. But both of those arguments are valid for this too.)

To make my argument clear.

Surely, it would be nice if we could use pulseaudio just for firefox, but we can't. To use it for firefox we have to force users to use it for everything.
"But users that don't want it can still run firefox with pure ALSA!"
With choppy WebRTC sound? And if they don't like choppy WebRTC sound? They then will be forced to use pulseaudio on the whole system. This is not a solution.

In short, either it is a bug that should be reported to bugzilla and if they wontfix and nobody else provides a patch, then we should attempt to fix it with tools like apulse, and only if they can't handle it, then we should force users to use global pulseaudio.

In shorter short, I think we should go bugzilla -> apulse -> pressueaudio -> pulseaudio (when everything else fails) route.

Btw, in the IRC log above I find in hilarious that one guy can do in a weekend what the whole mozilla can't in "a couple of years". See http://git.r-36.net/pressureaudio/tree/README.md

Please start by reporting the WebRTC bug, not by merging this.

Also see #35041.

[lheckemann]

"But users that don't want it can still run firefox with pure ALSA!"
With choppy WebRTC sound? And if they don't like choppy WebRTC sound? They then will be forced to use pulseaudio on the whole system. This is not a solution.

It's not a great solution, but it is a solution. It's a solution that's satisfactory by my standards and I'm too lazy to report a bug upstream (register an account on the bugtracker, gather all the appropriate information, etc) when this is upstream's first suggestion and it does solve my problem. Sorry!

This also does not force use of pulseaudio on anybody. libpulseaudio was already in the closure, and firefox will still use ALSA if pulseaudio is not enabled (tested this just now).

[7c6f434c]

Surely, it would be nice if we could use pulseaudio just for firefox, but we can't.

Hm, I think I actually have it. Maybe it can even be separated from all the reset of Firefox sandboxing. Unfortunately, I suspect the trick is scary enough for people to complain if I try to add it to the wrapper.

[oxij]

and it does solve my problem

nixpkgs.config.pulseaudio = true will solve your problem too without increasing the attack surface for everyone else.

[lheckemann]

It does not, due to the rebuild issue. Since the whole pulseaudio debate tends to be more acrimonious than I care for, I'll now withdraw myself from the discussion and leave it to the powers that be to decide whether to merge this change or not.

[oxij]

Just wondering, can you also test whenever #35047 fixes the choppy audio issue for you?

[oxij]

@7c6f434c You mean you run pulseaudiod from the wrapper? Does it work with other ALSA apps at the same time?

[7c6f434c]

PulseAudio does not make any active effort to break ALSA software mixing. The PA socket exists only inside the sandbox, so the other applications use ALSA just fine.

[lheckemann]

Will try #35047 (building for now), but I doubt it — I've had this issue since April or so and I never had the issue of not having sound at all which is what that patch is supposed to fix (ref https://bugzilla.mozilla.org/show_bug.cgi?id=1430274 )

[lheckemann]

Can confirm that the issue persists with #35047 applied.

[oxij]

Linus Heckemann <notifications@github.com> writes:

Can confirm that the issue persists with #35047 applied.

Can you describe the issue in more detail? Anything in the stderr/syslog/dmesg? Does it persist when you run `firefox` under `apulse`? (If it does, I'll try to package pressureaudio.)

[lheckemann]

Audio gets distorted: https://sphalerite.org/dump/firefox-webrtc-alsa.wav this is me saying "hello world" on a jitsi meet call, recorded externally through my laptop's headphone port.

Nothing relevant in any of the logs. Running firefox under apulse does indeed fix it.

[oxij]

That's pretty awesome news! Thanks for checking! Will experiment with packaging `pressueaudio` proper or making a similar thing with `apulse` and pulseaudio headers with nix later.

[lheckemann]

Anyway… I daresay pulseaudio is still the path of least resistance for someone to get audio working the way they're most likely to want it:

• Bluetooth audio working with no additional config
• Mixing for multiple applications supported by default
• Application volumes easily controlled individually (pavucontrol)

So I'm still in favour of having this merged, to provide firefox with a real and complete implementation of the API it targets and supports exclusively by default. This:

• can still be disabled by people who think pulseaudio is the devil and wish to banish every trace of it from their system
• will still use a fake pulseaudio implementation like apulse if given such
• will still use firefox's buggy disabled-by-default-upstream ALSA support if pulseaudio is unavailable (this is probably undesirable though, in favour of falling back to apulse or pressureaudio?)
• fixes the bug which was the actual reason I opened this PR in the first place

This seems to be an opinion shared by most others who have expressed any opinion here (see "thumbs up"s on the initial post; kind of a naff way of gathering opinions since it's only accessible via the web interface but hey).

_{One Day™ I'll probably switch to pure-alsa myself…}

[7c6f434c]

I think that the fact that Mozilla supports only PA, and we switched on official branding (with a promise not to patch Firefox beyond recognition) is unfortunately a reason enough to have PA available to Firefox by default, if enabled in the system.

(At the same time I hope that keeping ALSA enabled in the build is not against any implied promises, because I use it)

[lheckemann]

keeping ALSA enabled in the build

do you mean specifically Firefox's ALSA code, or would an alternative that supports running without pulseaudio but using apulse or similar (which fixes the bug) within the package meet this criterion for you too?

I certainly agree that our firefox package should still support audio without pulseaudio. I just don't think that firefox's buggy and unsupported ALSA code is a good way to do so.

[oxij]

:/ #29250 again? To recap it.

- Bluetooth audio working with no additional config

It needs a couple of lines in `asoundrc` to make it work with ALSA. I would put them into nixos ALSA module, but I don't have a headset to test. See #29250.

- Mixing for multiple applications supported by default

Supported by default in ALSA since a decade ago. See #29250.

- Application volumes easily controlled individually (pavucontrol)

Supported by most applications using ALSA, can be emulated with softvol sink for applications that don't. See #29250.

This: - can still be disabled by people who think pulseaudio is the devil and wish to banish every trace of it from their system

Normal playback works flawlessly on all versions of firefox in master (after #35047). This PR helps one specific use case at the cost of linking with a huge insecure lib.

- will still use a fake pulseaudio implementation like apulse if given such - will still use firefox's buggy disabled-by-default-upstream ALSA support if pulseaudio is unavailable (this is probably undesirable though, in favour of falling back to apulse or pressureaudio?)

Right, but this PR doesn't integrate any of that. One buggy WebRTC with a simple solution is better that a whole 150K lines of security issues what is `libpulseaudio`.

This seems to be an opinion shared by most others who have expressed any opinion here (see "thumbs up"s on the initial post; kind of a naff way of gathering opinions since it's only accessible via the web interface but hey).

"Likes" are not votes and definitely not peer-review. I hate that feature and wish github never added it. I think we should seriously discuss going in the other direction: #35041 (read the discussion! don't simply complain about the patches, they are there to make a point), this I will fix with `apulse`/`pressureaudio` (currently, its number 3 on my NixOS TODO).

[7c6f434c]

Hm. Looks like I currently use per-instance isolated PA instances for input. I.e. most instances do not have any sound access (as in: no PA and no permissions to open ALSA devices), some can play sound (PA), and the launchers which open Firefox with URLs where I want microphone to work start an isolated PA instance. For playback Firefox ALSA code seems OK; I tried apulse and there are some missing interfaces to implement; if pressureaudio works translates everything Firefox needs, it is a fine solution, but I am not sure if it is a moving target.

[oxij]

Status report: pressureaudio packaged, rebuilding the firefoxes. @7c6f434c writes:

I think that the fact that Mozilla supports _only_ PA, and we switched on official branding (with a promise not to patch Firefox beyond recognition) is unfortunately a reason enough to have PA available to Firefox by default, if enabled in the system.

With pressureaudio the default firefox expression will compile the vanilla firefox with the default options. I fail to see how Mozilla can reason against such a thing. "You have to link against the PA implementation by Poettering in officially branded Firefox, nonbackdoored code is not allowed." ? Ahaha.

[Mic92]

I was able to confirm choppy audio recording on https://jitsi.org/jitsi-meet/
when the alsa backend was enabled, but not in pulseaudio's case.
Until those bugs in the alsa backend are not fixed, we should set pulseaudio as default in firefox.
Users, who prefer the alsa backend, can set pulseaudio = false; in ~/.config/nixpkgs/config.nix.
This does not require to recompile firefox.

[oxij]

Locked-unlocked conversation, heh. "The suckless rage is the mother of progress."

See #35076.