Closed Bug 1247056 Opened 8 years ago Closed 8 years ago

Require PulseAudio to play sound on Linux

Categories

(Core :: Audio/Video: cubeb, defect, P2)

46 Branch
All
Linux
defect

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
relnote-firefox --- 52+
firefox52 --- fixed

People

(Reporter: ajones, Assigned: mozbugz)

References

Details

Attachments

(13 files, 1 obsolete file)

58 bytes, text/x-review-board-request
ajones
: review+
Details
58 bytes, text/x-review-board-request
jwwang
: review+
Details
58 bytes, text/x-review-board-request
jwwang
: review+
Details
58 bytes, text/x-review-board-request
jwwang
: review+
Details
58 bytes, text/x-review-board-request
jwwang
: review+
Details
58 bytes, text/x-review-board-request
jwwang
: review+
Details
58 bytes, text/x-review-board-request
jwwang
: review+
Details
58 bytes, text/x-review-board-request
smaug
: review+
Details
58 bytes, text/x-review-board-request
jwwang
: review+
Details
58 bytes, text/x-review-board-request
jwwang
: review+
Details
58 bytes, text/x-review-board-request
Gijs
: review+
Details
58 bytes, text/x-review-board-request
Gijs
: review+
Details
58 bytes, text/x-review-board-request
Gijs
: review+
Details
Make Pulse Audio a hard dependency on Linux so that we reduce the problems and maintenance associated with maintaining multiple audio backends.
Component: Audio/Video: Playback → Audio/Video: cubeb
Is there a discussion forum for this change? I'm sure I'm not the only one would rather not suffer PulseAudio on their systems, given that Firefox would be the only software (including other browsers) that would have it as a hard requirement (and given that PulseAudio still does not run well on some of the systems where I use Firefox as my main browser).
The reasonable thing to do IMHO is to disable the fallbacks, but still have a dlopen for libpulse.
(In reply to Thomas from comment #1)
> Is there a discussion forum for this change? I'm sure I'm not the only one
> would rather not suffer PulseAudio on their systems, given that Firefox
> would be the only software (including other browsers) that would have it as
> a hard requirement (and given that PulseAudio still does not run well on
> some of the systems where I use Firefox as my main browser).

There is project to emulate PulseAudio support for ALSA (zomg) - https://github.com/i-rinat/apulse you can try this in worst case.
(With sincere apologies for the bugspam).

@V.Korn, if it's that easy then why wouldn't Mozilla just support only ALSA, instead of only PA? Besides, the site in question says the project is dead, as it has achieved the author's modest goal. At this juncture it seems more practical to just use another browser on those systems when I need in-browser audio, or to hope that PA will finally improve for those cases where it still has major issues.
Anthony -- How quickly do you want to do this?  (I'm trying to figure out relative priority.)  Thanks.
Flags: needinfo?(ajones)
(In reply to Mike Hommey [:glandium] from comment #2)
> The reasonable thing to do IMHO is to disable the fallbacks, but still have
> a dlopen for libpulse.

The plan is to make it a hard dependency otherwise audio wouldn't work on systems that lack PulseAudio support.

(In reply to Thomas from comment #1)
> Is there a discussion forum for this change? I'm sure I'm not the only one
> would rather not suffer PulseAudio on their systems, given that Firefox
> would be the only software (including other browsers) that would have it as
> a hard requirement (and given that PulseAudio still does not run well on
> some of the systems where I use Firefox as my main browser).

Please give references to the PulseAudio bug numbers so we can follow up with their developers?

(In reply to V. Korn from comment #3)
> There is project to emulate PulseAudio support for ALSA (zomg) -
> https://github.com/i-rinat/apulse you can try this in worst case.

PulseAudio maps PulseAudio to ALSA already. We're not intending to remove ALSA support in cubeb or at compile time.

(In reply to Maire Reavy [:mreavy] (Plz ni?:mreavy) from comment #5)
> Anthony -- How quickly do you want to do this?  (I'm trying to figure out
> relative priority.)  Thanks.

I'm not in a hurry. It depends on how many ALSA related problems we get.
Flags: needinfo?(ajones)
(In reply to Anthony Jones (:kentuckyfriedtakahe, :k17e) from comment #6)
> (In reply to Mike Hommey [:glandium] from comment #2)
> > The reasonable thing to do IMHO is to disable the fallbacks, but still have
> > a dlopen for libpulse.
> 
> The plan is to make it a hard dependency otherwise audio wouldn't work on
> systems that lack PulseAudio support.

The point is, however irrational that is, some people don't want libpulse on their system. Making it a hard dependency makes the choice "install libpulse or don't use Firefox", rather than "install libpulse otherwise you won't have sound in Firefox".
> Please give references to the PulseAudio bug numbers so we can follow up
> with their developers?

Sorry, I don't currently have the time to currently hunt any down. I guess we'll find out soon enough if/when reports start coming in on Bugzilla.

Besides, I'm the only ALSA Firefox user who seems to have noticed this report anyway. It seems silly for me to rock the boat on my own, so I'll deal with it if I'm in such a tiny minority. Especially if ALSA has been causing you guys serious levels of technical debt.
(In reply to Mike Hommey [:glandium] from comment #7)
> The point is, however irrational that is, some people don't want libpulse on
> their system. Making it a hard dependency makes the choice "install libpulse
> or don't use Firefox", rather than "install libpulse otherwise you won't
> have sound in Firefox".

The point is to avoid people accidentally not installing Pulse Audio and having a broken/poor playback experience. They can always rebuild the package differently if they feel strongly about it. Linux developers are well placed to (a) feel strongly about the things, and (b) compile their own build.
They wouldn't have a broken/poor playback experience, they would have no playback at all. We could even show a popup/dropdown/whatever telling them "hey this page would normally have sound but you don't have libpulse. Please install it if you want sound".
The purpose of requiring Pulse Audio is to avoid the situation you're describing.
Can't they have libpulse and not the pulseaudio daemon? In which case they still won't have sound, right?
So the best path is |Recommends: libpulse0, pulseaudio| then?
Rank: 25
Priority: -- → P2
Summary: Require Pulse Audio on Linux → Require PulseAudio on Linux
Assignee: nobody → gsquelart
JW, could you please let me know if a bit of design in the MDSM is ok? -- As it may be controversial!

A bit of context: I need to catch cubeb initialization errors (when we're trying to play something with audio), and then display a message on the relevant document.
I think the best location to do that is in the MDSM: It starts the audio sink and therefore can catch any failure. (In some cases it drops the error and keeps playing the video alone.)

The main issue I have is to then find the associated document, to display a message.
To do that, I propose that we store the given MediaDecoderOwner (i.e., the HTMLMediaElement) in MDSM::Init, and remove it in Shutdown.
Then when an audio-sink-initialization happens, from the main thread we can retrieve the document from our stored MediaDecoderOwner in order to display a message (using Decoder Doctor to do the heavy lifting).

Here's my proposed patch to store the MediaDecoderOwner in the MDSM:
  https://hg.mozilla.org/try/rev/aa1f79aa4492
And the following patch where it's used:
  https://hg.mozilla.org/try/rev/323befa921e1
(Full context in this try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=dcaff8ecde3d56d1d02d329362e64ce6f30dfe44 )

I'd like to know if you're ok with this general approach (If yes, no need to fully review these patches yet, unless you see something very wrong).
And if not acceptable, how else would you propose to solve my conundrum?
Thanks in advance.
Flags: needinfo?(jwwang)
http://searchfox.org/mozilla-central/rev/8eb4fd2c7be150b0aa1b05c0f3707e82dc8f2dc8/dom/media/MediaDecoderStateMachine.cpp#2727

We have MediaDecoderStateMachine::DecodeError() to propagate errors to MediaDecoder. All we need is to pass an additional error code to indicate the type of the error.

I don't want to store a MediaDecoderOwner in MDSM to increase the coupling. Please use mOnPlaybackEvent (or something like that if you have additional data to pass) to publish events to the listener.

http://searchfox.org/mozilla-central/rev/8eb4fd2c7be150b0aa1b05c0f3707e82dc8f2dc8/dom/media/MediaDecoderStateMachine.h#907
Flags: needinfo?(jwwang)
Comment on attachment 8792393 [details]
Bug 1247056 - Expose MOZ_PULSEAUDIO #define -

https://reviewboard.mozilla.org/r/79468/#review78012
Attachment #8792393 - Flags: review?(jwwang) → review+
Comment on attachment 8792394 [details]
Bug 1247056 - NS_ERROR_DOM_MEDIA_CUBEB_INITIALIZATION_ERR -

https://reviewboard.mozilla.org/r/79470/#review78014
Attachment #8792394 - Flags: review?(jwwang) → review+
Comment on attachment 8792395 [details]
Bug 1247056 - Report specific cubeb-init error code -

https://reviewboard.mozilla.org/r/79472/#review78016
Attachment #8792395 - Flags: review?(jwwang) → review+
Comment on attachment 8792396 [details]
Bug 1247056 - Fix DecoderDoctor logging -

https://reviewboard.mozilla.org/r/79474/#review78020
Attachment #8792396 - Flags: review?(jwwang) → review+
Comment on attachment 8792397 [details]
Bug 1247056 - Refactor ReportToConsole and ReportAnalysis to be static -

https://reviewboard.mozilla.org/r/79476/#review78022
Attachment #8792397 - Flags: review?(jwwang) → review+
Comment on attachment 8792398 [details]
Bug 1247056 - DecDoc::StoreEvent for singular playback issues/solutions -

https://reviewboard.mozilla.org/r/79478/#review78024

::: dom/media/DecoderDoctorDiagnostics.cpp:821
(Diff revision 1)
> +    case eEvent:
> +      s = nsPrintfCString("event domain %s result=%u",
> +                          EventDomainString(mEvent.mDomain), mEvent.mResult);
> +      break;
>      default:
> +      MOZ_ASSERT(false);

Use MOZ_ASSERT_UNREACHABLE.
Attachment #8792398 - Flags: review?(jwwang) → review+
Comment on attachment 8792400 [details]
Bug 1247056 - Handle PulseAudio init error in DecDoc -

https://reviewboard.mozilla.org/r/79482/#review78026
Attachment #8792400 - Flags: review?(jwwang) → review+
Comment on attachment 8792401 [details]
Bug 1247056 - Notify MediaDecoder about Decoder Doctor events from MDSM -

https://reviewboard.mozilla.org/r/79484/#review78028

::: dom/media/MediaDecoder.cpp:705
(Diff revision 1)
> +      return;
> +    }
> +    DecoderDoctorDiagnostics diags;
> +    diags.StoreEvent(doc, aEvent, __func__);
> +  });
> +  AbstractThread::MainThread()->Dispatch(r.forget());

Can you do |diags.StoreEvent| synchronously here?

::: dom/media/MediaDecoderStateMachine.cpp:2950
(Diff revision 1)
>  
>    mMediaSinkAudioPromise.Complete();
>    mAudioCompleted = true;
>  
> +  // Report any non-OK result to Decoder Doctor.
> +  if (aResult != NS_OK) {

It doesn't make sense to reject the promsie with NS_OK, right?
Comment on attachment 8792401 [details]
Bug 1247056 - Notify MediaDecoder about Decoder Doctor events from MDSM -

https://reviewboard.mozilla.org/r/79484/#review78028

Thank you for the prompt reviews.

> Can you do |diags.StoreEvent| synchronously here?

Synchronous events are objectively evil, so I'd prefer to avoid them unless absolutely necessary!

In this situation, in the worst case the media element or document could disappear by the time the event arrives on the main thread, it just means we won't display anything for an object that was just about to die anyway. So I don't think there would be much to gain from a synchronous dispatch.

Please let me know if you still disagree.

> It doesn't make sense to reject the promsie with NS_OK, right?

It seems very unlikely indeed, but I didn't want to take the risk. And it's a cheap test compared to the whole work that will (always?) follow.
Do you think I should just remove the test? Maybe add |MOZ_ASSERT(aResult != NS_OK)| to make a clear distinction from the other call with NS_OK at line 2937?
Comment on attachment 8792401 [details]
Bug 1247056 - Notify MediaDecoder about Decoder Doctor events from MDSM -

https://reviewboard.mozilla.org/r/79484/#review78040

::: dom/media/MediaDecoder.cpp:705
(Diff revision 1)
> +      return;
> +    }
> +    DecoderDoctorDiagnostics diags;
> +    diags.StoreEvent(doc, aEvent, __func__);
> +  });
> +  AbstractThread::MainThread()->Dispatch(r.forget());

I don't see any synchronous events here.

OnDecoderDoctorEvent() already runs on the main thread. I can't see the point in dispatching a new task to run |diags.StoreEvent| on the main thread.

::: dom/media/MediaDecoderStateMachine.cpp:2950
(Diff revision 1)
>  
>    mMediaSinkAudioPromise.Complete();
>    mAudioCompleted = true;
>  
> +  // Report any non-OK result to Decoder Doctor.
> +  if (aResult != NS_OK) {

I prefer to put an assertion.
Comment on attachment 8792401 [details]
Bug 1247056 - Notify MediaDecoder about Decoder Doctor events from MDSM -

https://reviewboard.mozilla.org/r/79484/#review78040

> I don't see any synchronous events here.
> 
> OnDecoderDoctorEvent() already runs on the main thread. I can't see the point in dispatching a new task to run |diags.StoreEvent| on the main thread.

Oh now I see what you meant (a direct call, not a synchronous dispatch). Ok will do, thank you for letting me know.
Comment on attachment 8792401 [details]
Bug 1247056 - Notify MediaDecoder about Decoder Doctor events from MDSM -

https://reviewboard.mozilla.org/r/79484/#review78050

::: dom/media/MediaDecoder.cpp:685
(Diff revision 2)
>  {
>    DecodeError(aError);
>  }
>  
>  void
> +MediaDecoder::OnDecoderDoctorEvent(DecoderDoctorEvent aEvent)

Since we disconnect the listener in Shutdown(), this function will always happen before Shutdown() and mOwner is non-null.

Please assert !IsShutdown() and remove the check for !mOwner.

::: dom/media/MediaDecoderStateMachine.cpp:2950
(Diff revision 2)
>  
>    mMediaSinkAudioPromise.Complete();
>    mAudioCompleted = true;
>  
> +  // Result should never be NS_OK in this *error* handler. Report to Dec-Doc.
> +  MOZ_ASSERT(aResult != NS_OK);

NS_FAILED(rv) is used more often than rv != NS_OK.
Attachment #8792401 - Flags: review?(jwwang) → review+
Comment on attachment 8792402 [details]
Bug 1247056 - Fix collecting of formats-at-issues -

https://reviewboard.mozilla.org/r/79486/#review78060

D'oh. This can land independently, I think?

Also, are there no tests that are catching these issues? Can we add some (can be a followup) so we realize if we break things? :-)
Attachment #8792402 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8792403 [details]
Bug 1247056 - More flexible SUMO link -

https://reviewboard.mozilla.org/r/79488/#review78062

::: browser/base/content/browser-media.js:238
(Diff revision 2)
>      }
>      return "";
>    },
>  
> +  getSumoForLearnHowButton(type) {
> +    if (AppConstants.isPlatformAndVersionAtLeast("win", "6")) {

Previously we showed this for all Windows users, so we also included XP. Feels like we shouldn't change that here? (In particular, AIUI the CDM not activated thing can happen on XP, right?)
Attachment #8792403 - Flags: review?(gijskruitbosch+bugs) → review-
Comment on attachment 8792404 [details]
Bug 1247056 - Show notification when PulseAudio cannot be used -

https://reviewboard.mozilla.org/r/79490/#review78064

r=me with the lonely if / checking for Linux sorted out. :-)

::: browser/base/content/browser-media.js:234
(Diff revision 2)
> +    if (type == "cannot-initialize-pulseaudio") {
> +      if (AppConstants.platform == "linux") {

No lonely ifs! I'd be surprised if eslint didn't pick this up. Anyway, please && these two conditions. That said... is the second one really necessary? We don't check for platform == linux in the next block, so maybe we can just omit it here, too? Not sure if this can also happen on BSD in which case checking for Linux is probably wrong. Either way, there should be no lonely ifs, and we should make the conditions match the one in the if block a bit lower down. :-)
Attachment #8792404 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8792399 [details]
Bug 1247056 - New DecoderDoctorNotificationType -

https://reviewboard.mozilla.org/r/79480/#review78082

This is not exposed to the web, so r+.
Attachment #8792399 - Flags: review?(bugs) → review+
Comment on attachment 8792403 [details]
Bug 1247056 - More flexible SUMO link -

https://reviewboard.mozilla.org/r/79488/#review78062

> Previously we showed this for all Windows users, so we also included XP. Feels like we shouldn't change that here? (In particular, AIUI the CDM not activated thing can happen on XP, right?)

Well spotted! I actually did it on purpose, I should have explained my reasoning in the commit description.

The first thing appearing on the SUMO page is: "This article applies to Windows Vista/7/8/8.1/10 users only."
https://support.mozilla.org/en-US/kb/fix-video-audio-problems-firefox-windows#firefox:winxp
So I thought it would be confusing to XP users.

Also, I think the "Learn How" should only appear on XP in the "adobe-cdm-not-activated" case, for which the fix-video-etc SUMO page doesn't say anything!

So from here I see the following options:
1. Re-enable the Learn-How button for XP, and maybe work on the side to add some XP-specific help to the SUMO page.
2. Point to another page in the XP case -- though after a quick search I couldn't find a good one.
3. Other?
What do you think?
(In reply to Gerald Squelart [:gerald] from comment #57)
> Comment on attachment 8792403 [details]
> Bug 1247056 - More flexible SUMO link -
> 
> https://reviewboard.mozilla.org/r/79488/#review78062
> 
> > Previously we showed this for all Windows users, so we also included XP. Feels like we shouldn't change that here? (In particular, AIUI the CDM not activated thing can happen on XP, right?)
> 
> Well spotted! I actually did it on purpose, I should have explained my
> reasoning in the commit description.
> 
> The first thing appearing on the SUMO page is: "This article applies to
> Windows Vista/7/8/8.1/10 users only."
> https://support.mozilla.org/en-US/kb/fix-video-audio-problems-firefox-
> windows#firefox:winxp
> So I thought it would be confusing to XP users.

You're right, that would be confusing.

> Also, I think the "Learn How" should only appear on XP in the
> "adobe-cdm-not-activated" case, for which the fix-video-etc SUMO page
> doesn't say anything!

Yeah, the page seems unrelated to the CDM stuff. In fact, https://support.mozilla.org/en-US/kb/enable-drm says that "Adobe Primetime is available on Windows Vista/7/8/10 for both 32-bit and 64-bit versions of Firefox.", which makes me think that we shouldn't be showing this on XP at all, and the patch is correct as-is. I'll just mark r+ though it would be good to clarify this in the commit message.
Comment on attachment 8792403 [details]
Bug 1247056 - More flexible SUMO link -

https://reviewboard.mozilla.org/r/79488/#review78102

r=me with an explanation in the commit message that the not-activated CDM and missing media features can only happen on vista and later, so the checks have been tightened up.
Attachment #8792403 - Flags: review- → review+
Comment on attachment 8792404 [details]
Bug 1247056 - Show notification when PulseAudio cannot be used -

https://reviewboard.mozilla.org/r/79490/#review78064

> No lonely ifs! I'd be surprised if eslint didn't pick this up. Anyway, please && these two conditions. That said... is the second one really necessary? We don't check for platform == linux in the next block, so maybe we can just omit it here, too? Not sure if this can also happen on BSD in which case checking for Linux is probably wrong. Either way, there should be no lonely ifs, and we should make the conditions match the one in the if block a bit lower down. :-)

I was hoping the double-if would help with future changes if needed.
And PulseAudio actually exists for Windows and Mac as well. But it's true we only use it on Linux, so we don't really need to test for it now.
So I'll remove the linux test.
Attachment #8792402 - Attachment is obsolete: true
Comment on attachment 8792392 [details]
Bug 1247056 - Don't default-enable alsa on Linux -

https://reviewboard.mozilla.org/r/79466/#review78254
Attachment #8792392 - Flags: review?(ajones) → review+
Pushed by gsquelart@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/01aa7f5bde71
Don't default-enable alsa on Linux - r=kentuckyfriedtakahe
https://hg.mozilla.org/integration/mozilla-inbound/rev/0ce3c5cb6e6a
Expose MOZ_PULSEAUDIO #define - r=jwwang
https://hg.mozilla.org/integration/mozilla-inbound/rev/344a41f87a23
NS_ERROR_DOM_MEDIA_CUBEB_INITIALIZATION_ERR - r=jwwang
https://hg.mozilla.org/integration/mozilla-inbound/rev/be036a725d3a
Report specific cubeb-init error code - r=jwwang
https://hg.mozilla.org/integration/mozilla-inbound/rev/2b3dce1c98f6
Fix DecoderDoctor logging - r=jwwang
https://hg.mozilla.org/integration/mozilla-inbound/rev/49558f78782a
Refactor ReportToConsole and ReportAnalysis to be static - r=jwwang
https://hg.mozilla.org/integration/mozilla-inbound/rev/32fc8121ca20
DecDoc::StoreEvent for singular playback issues/solutions - r=jwwang
https://hg.mozilla.org/integration/mozilla-inbound/rev/024d7295b6f4
New DecoderDoctorNotificationType - r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/8e164f6d6c5c
Handle PulseAudio init error in DecDoc - r=jwwang
https://hg.mozilla.org/integration/mozilla-inbound/rev/f48fd90bf839
Notify MediaDecoder about Decoder Doctor events from MDSM - r=jwwang
https://hg.mozilla.org/integration/mozilla-inbound/rev/e1e5be2fc586
More flexible SUMO link - r=gijs
https://hg.mozilla.org/integration/mozilla-inbound/rev/13434fa439d1
Show notification when PulseAudio cannot be used - r=gijs
Joni, it looks like this bug wants to link to a sumo article. Is there a help-topic link set up for what there are using?
Flags: needinfo?(jsavage)
sorry had to back this out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=36364091&repo=mozilla-inbound
Flags: needinfo?(gsquelart)
Backout by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2ae91c5e5354
Backed out 12 changesets for windows bc7 test failures
Thanks for need info'ing me on this, Chris. We don't have a SUMO link yet, but we can create one. 

I'll need help writing the article because I don't understand the bug well though. Is there anyone who can help us write the article (an outline would be okay and we can turn it into an article)?
Flags: needinfo?(jsavage)
(In reply to Joni Savage ("need info" me) from comment #102)
> Thanks for need info'ing me on this, Chris. We don't have a SUMO link yet,
> but we can create one. 
> 
> I'll need help writing the article because I don't understand the bug well
> though. Is there anyone who can help us write the article (an outline would
> be okay and we can turn it into an article)?

As I understand it, they want to point to <https://support.mozilla.org/kb/fix-common-audio-and-video-issues>.
 
> As I understand it, they want to point to
> <https://support.mozilla.org/kb/fix-common-audio-and-video-issues>.

We have an in-product link for this article in this case: https://support.mozilla.org/1/firefox/%VERSION%/%OS%/%LOCALE%/fix-video-audio-problems-firefox-windows
Thanks. :)
(In reply to Joni Savage ("need info" me) from comment #102)
> Thanks for need info'ing me on this, Chris. We don't have a SUMO link yet,
> but we can create one. 

Joni, I've already written a KB section about this in https://support.mozilla.org/en-US/kb/fix-common-audio-and-video-issues#w_pulseaudio-required-for-linux (which is where the last patch links to), but it's only visible from FF>=51 on Linux.
I see that Chris has just made an improvement to it, thank you Chris.
(In reply to Carsten Book [:Tomcat] from comment #100)
> sorry had to back this out for test failures like
> https://treeherder.mozilla.org/logviewer.html#?job_id=36364091&repo=mozilla-
> inbound

Thank you Carsten, sorry for the trouble.
I see now that the tests for Decoder Doctor are in a different area than what I usually Try; I'll make sure I run them from now on.
Flags: needinfo?(gsquelart)
(In reply to Gerald Squelart [:gerald] from comment #106)
> (In reply to Joni Savage ("need info" me) from comment #102)
> > Thanks for need info'ing me on this, Chris. We don't have a SUMO link yet,
> > but we can create one. 
> 
> Joni, I've already written a KB section about this in
> https://support.mozilla.org/en-US/kb/fix-common-audio-and-video-
> issues#w_pulseaudio-required-for-linux (which is where the last patch links
> to), but it's only visible from FF>=51 on Linux.
> I see that Chris has just made an improvement to it, thank you Chris.

We were just making sure the help-topic was set up in sumo, as per <https://support.mozilla.org/en-US/kb/a-guide-to-linking-to-support-articles>, that's all. :)
Gijs, I had forgotten one message in the "More flexible SUMO link" patch, see:
https://reviewboard.mozilla.org/r/79488/diff/5-6/
Good that the test was there!

And now I've added the appropriate test for the new message:
https://reviewboard.mozilla.org/r/80972/diff/#index_header
Comment on attachment 8794630 [details]
Bug 1247056 - Added test for cannot-initialize-pulseaudio -

https://reviewboard.mozilla.org/r/80972/#review79650

::: browser/base/content/test/general/browser_decoderDoctor.js:40
(Diff revision 1)
> -    let url = baseURL + "fix-video-audio-problems-firefox-windows";
> +    let url = baseURL + ((options && options.sumo)
> +                         ? options.sumo
> +                         : "fix-video-audio-problems-firefox-windows");

Due to JS being dark magic and duct tape, you can just use:

```
let url = baseURL + (options.sumo || "fix-video-audio-problems-firefox-windows");
```
Attachment #8794630 - Flags: review?(gijskruitbosch+bugs) → review+
(In reply to :Gijs Kruitbosch from comment #123)
> let url = baseURL + (options.sumo || "fix-video-audio-problems-firefox-windows");

Thank you Gijs for the extra review.

However regarding your dark magic and duct tape: If no 'options' is given, 'options.sumo' will throw an error instead of giving us a falsy value.

How about: 'baseURL + (options && options.sumo || "fix-...")', that seems to work.
But is it worth the obfuscation? (I admit I'm not a regular JSer!)
(In reply to Gerald Squelart [:gerald] from comment #124)
> (In reply to :Gijs Kruitbosch from comment #123)
> > let url = baseURL + (options.sumo || "fix-video-audio-problems-firefox-windows");
> 
> Thank you Gijs for the extra review.
> 
> However regarding your dark magic and duct tape: If no 'options' is given,
> 'options.sumo' will throw an error instead of giving us a falsy value.

Oh, oops, I missed out the and clause - good point.

> How about: 'baseURL + (options && options.sumo || "fix-...")', that seems to
> work.
> But is it worth the obfuscation? (I admit I'm not a regular JSer!)

Using "foo || bar" instead of "foo ? foo : bar" in JS is a pretty commonly accepted idiom, but I can see why you might want to stick with the ternary. If you do use the || version, I would include () around the && expression ((foo && foo.something) || bar). Up to you.
Pushed by gsquelart@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/511ed53eee9a
Don't default-enable alsa on Linux - r=kentuckyfriedtakahe
https://hg.mozilla.org/integration/mozilla-inbound/rev/63094a80bee7
Expose MOZ_PULSEAUDIO #define - r=jwwang
https://hg.mozilla.org/integration/mozilla-inbound/rev/934d7c9628f1
NS_ERROR_DOM_MEDIA_CUBEB_INITIALIZATION_ERR - r=jwwang
https://hg.mozilla.org/integration/mozilla-inbound/rev/2bfe3aeb9f59
Report specific cubeb-init error code - r=jwwang
https://hg.mozilla.org/integration/mozilla-inbound/rev/c5cd9597ec02
Fix DecoderDoctor logging - r=jwwang
https://hg.mozilla.org/integration/mozilla-inbound/rev/1f3b037bfd1f
Refactor ReportToConsole and ReportAnalysis to be static - r=jwwang
https://hg.mozilla.org/integration/mozilla-inbound/rev/b8857adc4003
DecDoc::StoreEvent for singular playback issues/solutions - r=jwwang
https://hg.mozilla.org/integration/mozilla-inbound/rev/cd40559e25d7
New DecoderDoctorNotificationType - r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/d0d71a80a029
Handle PulseAudio init error in DecDoc - r=jwwang
https://hg.mozilla.org/integration/mozilla-inbound/rev/001b42631702
Notify MediaDecoder about Decoder Doctor events from MDSM - r=jwwang
https://hg.mozilla.org/integration/mozilla-inbound/rev/03553484a062
More flexible SUMO link - r=gijs
https://hg.mozilla.org/integration/mozilla-inbound/rev/a444d7cb1091
Show notification when PulseAudio cannot be used - r=gijs
https://hg.mozilla.org/integration/mozilla-inbound/rev/ff3538fd40fe
Added test for cannot-initialize-pulseaudio - r=gijs
Thank you Wes. I *did* try before pushing, but somehow the failing Windows XP tests just did not run there. :-(
Flags: needinfo?(gsquelart)
Pushed by gsquelart@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/541ae26e38e5
Don't default-enable alsa on Linux - r=kentuckyfriedtakahe
https://hg.mozilla.org/integration/autoland/rev/287d078fc307
Expose MOZ_PULSEAUDIO #define - r=jwwang
https://hg.mozilla.org/integration/autoland/rev/95aa4314a12c
NS_ERROR_DOM_MEDIA_CUBEB_INITIALIZATION_ERR - r=jwwang
https://hg.mozilla.org/integration/autoland/rev/40270f33ff47
Report specific cubeb-init error code - r=jwwang
https://hg.mozilla.org/integration/autoland/rev/d2efccd7743c
Fix DecoderDoctor logging - r=jwwang
https://hg.mozilla.org/integration/autoland/rev/61b24fd02149
Refactor ReportToConsole and ReportAnalysis to be static - r=jwwang
https://hg.mozilla.org/integration/autoland/rev/da7b4f4ff12f
DecDoc::StoreEvent for singular playback issues/solutions - r=jwwang
https://hg.mozilla.org/integration/autoland/rev/b99273f770d8
New DecoderDoctorNotificationType - r=smaug
https://hg.mozilla.org/integration/autoland/rev/289bdf541e58
Handle PulseAudio init error in DecDoc - r=jwwang
https://hg.mozilla.org/integration/autoland/rev/a7b60f7a950b
Notify MediaDecoder about Decoder Doctor events from MDSM - r=jwwang
https://hg.mozilla.org/integration/autoland/rev/93c70da4ed72
More flexible SUMO link - r=Gijs
https://hg.mozilla.org/integration/autoland/rev/25be34cfa5ad
Show notification when PulseAudio cannot be used - r=Gijs
https://hg.mozilla.org/integration/autoland/rev/a540a043466d
Added test for cannot-initialize-pulseaudio - r=Gijs
So my sound is not working now.

Do i open a new bug or do we handle it here since it's related ?
(In reply to ligrec from comment #163)
> So my sound is not working now.

Did you see a notification saying "To play audio, you may need to install the required PulseAudio software."?
If not, it might be something else.

If yes, it was a conscious decision, so it's unlikely to get "fixed".
You could try and install PulseAudio, or build your own Firefox with ALSA enabled (the code is still there).

> Do i open a new bug or do we handle it here since it's related ?

If you're still not happy, open a new bug please, as patches have landed and nothing more can happen in this bug here.
>Did you see a notification saying "To play audio, you may need to install the required PulseAudio software."?
If not, it might be something else.

Yes, but i will not install PulseAudio as it 1) doesn't work properly with my sound card and 2) is really bad.

>... or build your own Firefox with ALSA enabled (the code is still there).

I will try, but i had trouble before since i have only 4GB or RAM.

>If yes, it was a conscious decision, so it's unlikely to get "fixed".

>If you're still not happy, open a new bug please, as patches have landed and nothing more can happen in this bug here.

I was thinking of opening a new bug, but i'm in doubt that it would change anything.

Sorry to sound negative but after being a FF user (and fan) for... as long as i remember, in spite the crashes of old versions and many glitches of new version, this is the first time i seriously considered chromium.

Then again i will need another 4GB RAM stick to even use chromium :) so i could just as well build FF.
Hello, 

Today I build firefox from https://hg.mozilla.org/mozilla-central/ with --enable-alsa and --disable-pulseaudio, but there is no audio. 

Couple of days ago --enable-alsa works, but no any more.

So is alsa code still there? 

---
about:buildconfig
Build platform
target
x86_64-pc-linux-gnu
Build tools
Compiler 	Version 	Compiler flags
/usr/bin/x86_64-pc-linux-gnu-gcc -std=gnu99 	5.4.0 	-Wall -Wempty-body -Wignored-qualifiers -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wno-error=maybe-uninitialized -Wno-error=deprecated-declarations -Wno-error=array-bounds -march=native -pipe -fno-strict-aliasing -fno-math-errno -pthread -pipe
/usr/bin/x86_64-pc-linux-gnu-g++ -std=gnu++11 	5.4.0 	-Wall -Wc++11-compat -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wwrite-strings -Wno-invalid-offsetof -Wc++14-compat -Wno-error=maybe-uninitialized -Wno-error=deprecated-declarations -Wno-error=array-bounds -march=native -pipe -fno-exceptions -fno-strict-aliasing -fno-rtti -fno-exceptions -fno-math-errno -pthread -pipe -O2 -fomit-frame-pointer
Configure options

--host=x86_64-pc-linux-gnu --target=x86_64-pc-linux-gnu --enable-application=browser --disable-tests PKG_CONFIG=x86_64-pc-linux-gnu-pkg-config CC=x86_64-pc-linux-gnu-gcc CXX=x86_64-pc-linux-gnu-g++ HOST_CC=x86_64-pc-linux-gnu-gcc HOST_CXX=x86_64-pc-linux-gnu-g++ --disable-debug-symbols LD=x86_64-pc-linux-gnu-ld MOZ_JEMALLOC4=1 --enable-replace-malloc --enable-default-toolkit=cairo-gtk2 --with-google-api-keyfile=/build/portage/www-client/firefox-52.0/work/firefox-52.0/google-api-key MAKE=make XARGS=/usr/bin/xargs --enable-system-hunspell --enable-alsa --disable-crashreporter --enable-dbus --enable-extensions=default --disable-gconf --enable-gio --disable-gnomeui --disable-gold --disable-install-strip --enable-ion --disable-necko-wifi --enable-official-branding --enable-optimize=-O2 --disable-pulseaudio --enable-startup-notification --disable-strip --disable-system-cairo --disable-system-sqlite --disable-updater --libdir=/usr/lib64 --prefix=/usr --with-default-mozilla-five-home=/usr/lib64/firefox --with-intl-api --with-system-bz2 --without-system-graphite2 --without-system-harfbuzz --without-system-icu --without-system-jpeg --without-system-libvpx --with-system-png --with-system-zlib --x-includes=/usr/include --x-libraries=/usr/lib64
(In reply to drJeckyll from comment #166)
> Today I build firefox from https://hg.mozilla.org/mozilla-central/ with
> --enable-alsa and --disable-pulseaudio, but there is no audio. 
> Couple of days ago --enable-alsa works, but no any more.
> So is alsa code still there? 

Yes the ALSA code is still there.

I've just tried, and I see that the content sandbox recent tightening [1] is denying access to some files when cubeb is trying to start:
> Sandbox: SandboxBroker: denied op=0 rflags=2000002 perms=3 path=/dev/snd/controlC0 for pid=37387 error="No such file or directory"
> Sandbox: SandboxBroker: denied op=0 rflags=2000002 perms=3 path=/dev/snd/controlC0 for pid=37387 error="No such file or directory"
> [Child 37387] WARNING: AudioStream::OpenCubeb() 7f8044a08700 failed to init cubeb: file dom/media/AudioStream.cpp, line 377

To confirm this:
- Go to about:support and in the "Graphics" section, the "Audio Backend" should show "alsa" (this shows that we at least tried to use ALSA.)
- In about:config, "security.sandbox.content.level" is at 2 by default; change it to 1, restart Firefox, and you should have sound working again.
If that does the trick, this new issue has nothing to do with this bug here.
Please confirm the above, and I'll open a new bug for that.

[1] https://groups.google.com/d/msg/mozilla.dev.platform/V-5G9dWJEXo/S9qxkg9EAgAJ
Flags: needinfo?(drJeckyll)
I can confirm that with "security.sandbox.content.level" set to 1 I have sound again.
Thank you dr. I've opened bug 1309098 to deal with this.
Flags: needinfo?(drJeckyll)
(In reply to ligrec from comment #165)
> Yes, but i will not install PulseAudio as it 1) doesn't work properly with
> my sound card and 2) is really bad.
Neither would I go for pulseaudio...

> >... or build your own Firefox with ALSA enabled (the code is still there).
...
> Sorry to sound negative but after being a FF user (and fan) for... as long
> as i remember, in spite the crashes of old versions and many glitches of new
> version, this is the first time i seriously considered chromium.
And Mozilla got lots of people either going for the bad (the one further above [puls], or just above [chromium]), lots of people, or for going through hardship of building their own, a minority... 

...like below:
> Then again i will need another 4GB RAM stick to even use chromium :) so i
> could just as well build FF.
That is what I was considering. But...

Hey, the distros did it for us (or I should hope that they are doing it, information needed from users of other distros here)!

Archlinux does not require Pulseaudio:
http://www.mail-archive.com/alsa-user@lists.sourceforge.net/msg31929.html

Gentoo does not require Pulseaudio:
https://marc.info/?l=gentoo-user&m=147962947600929&w=2

And please, developers, make is all more understandable for us not-very-advanced users... The title, as far as I figured out, if you browse the discussion from the links on the alsa-user ML and gentoo-user ML, is very misleading! Have a look at how users understand it:
Firefox nightly requires Pulse Audio
http://forums.debian.net/viewtopic.php?f=20&t=130028

Pulseaudio is *not* required on Linux (at least not on some very fundamental Linux distros, like the above)!

Regards!
Thank you Miroslav, I've updated the bug title to clarify that PulseAudio is required *to play sound* on Linux.
Summary: Require PulseAudio on Linux → Require PulseAudio to play sound on Linux
(In reply to Gerald Squelart [:gerald] from comment #172)
> Thank you Miroslav, I've updated the bug title to clarify that PulseAudio is
> required *to play sound* on Linux.
But it's not required, on at least two very noteworthy (at least), or even leading I would say, Linux distributions: Archlinux and Gentoo.
And I'm using my Gentoo machine, without Pulsaudio, with pure ALSA, with HTML5 video (I just right now re-tested in on a random Youtube video, it just works!)...
However, I won't dispute any more. You're a senior here, and I'm just a user.
Regards!
(In reply to Miroslav Rovis from comment #173)
For clarity:
> And I'm using my Gentoo machine, without Pulsaudio, with pure ALSA, with
> HTML5 video (I just right now re-tested in on a random Youtube video, it
> just works!)...
The audio component of any moving pictures clip just works, along the video component, just tested it on a random Youtube that I choose from a bunch that opened when I typed in www.youtube.com in a new window.
(In reply to Miroslav Rovis from comment #174)
> (In reply to Miroslav Rovis from comment #173)
> For clarity:
[[ audio ]]
> > just works!
For clarity, I'm on ~amd64, and I'm using:

$ firefox --version
Mozilla Firefox 50.0
$

Any more info on my system needed, do tell. But of course, it's Gentoo devs that can tell how come it works, when it was (don't know what expression to use...) scheduled not to work anymore...
(In reply to Miroslav Rovis from comment #175)
> $ firefox --version
> Mozilla Firefox 50.0
> $

ALSA support is dropped from Firefox 52 onward.
(In reply to Miroslav Rovis from comment #175)
> (In reply to Miroslav Rovis from comment #174)
> > (In reply to Miroslav Rovis from comment #173)
> > For clarity:
> [[ audio ]]
> > > just works!
> For clarity, I'm on ~amd64, and I'm using:
> 
> $ firefox --version
> Mozilla Firefox 50.0
> $
> 
> Any more info on my system needed, do tell. But of course, it's Gentoo devs
> that can tell how come it works, when it was (don't know what expression to
> use...) scheduled not to work anymore...

I believe the point Miroslav Rovis was trying to make is that 'PulseAudio is NOT required *to play sound* on Linux.', rather "Mozilla's official binaries require PulseAudio is to play sound on Linux."

A stubborn person who doesn't want to install pulse just has to go through the process of compiling firefox themselves. Granted, installing pulse is the easier and probably the easier and more logical solution, but the option still stands. I am one of those stubborn people, just compiled Firefox 53.0a1 (latest development version) with alsa enabled and pulse disabled. Even spent an hour writing an gentoo ebuild for it... and 6hrs recompiling things until it worked.... (I should just install pulse)

FWIW, I don't think we really need to make the distinction though. While pulse isn't required for sound, it is now preferred. And chances are support for alsa will slowly die out, and the option eventually removed.
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0
User notices no sound from YouTube.
User checks everything and finally notices a tiny
link to https://support.mozilla.org/1/firefox/52.0/Linux/en-US/fix-common-audio-and-video-issues
on their screen which says "No se encuentra la página".
And something about PulseAudio (which I finally eradicated last week.)
Not one Depends or Recommends or even Suggests here on Debian,
$ aptitude -v show firefox|grep -c pulse
0
I already have
$ aptitude search ~i|grep -i pulse
i  libpulse0 - PulseAudio client libraries
what more do I need short of the full born pulseaudio package?
This looks like something that could have a big impact on our Linux users, in terms of audio playback.

Gerald, is there anything manual QA could do to help test this?
Flags: qe-verify?
Flags: needinfo?(gsquelart)
Will ESR 52 also depend on PulseAudio? Or is there an exception for the ESR version, similar to the NPAPI exception for ESR 52 ( https://bugzilla.mozilla.org/show_bug.cgi?id=1269807 )?
Restrict Comments: true
Depends on: 1345661
Depends on: 1345439
Release Note Request (optional, but appreciated)
> [Why is this notable]:

The set of system libraries that Firefox requires on Linux changed in Firefox 52, but this was not mentioned in the release notes. Maintainers of Firefox packages in many distros didn't catch this, and as a result, the distro packages for Firefox didn't update their dependencies. For users who download Firefox from Mozilla and who use the (rare according to telemetry) affected configuration, it may be unclear whether the situation is a bug or an intentional change requiring user action. Furthermore, not documenting this in the release notes makes it look like the change was made somehow in secret, which annoys people even more. Additionally, the in-product messaging and SUMO got out of sync, so the in-product messaging goes to a 404 SUMO URL (bug 1345439).

It's too late to address the distro issue via rel notes, but at least the directly user-facing documentation concern could still be fixed by editing the 52 rel notes.

> [Affects Firefox for Android]:

No.

[Suggested wording]:
On Linux, Firefox now requires PulseAudio to play sound and no longer plays sound directly with ALSA.

[Links (documentation, blog post, etc)]:
https://support.mozilla.org/t5/Videos-sound-pictures-and/Fix-common-audio-and-video-issues/ta-p/401#w_you-may-need-to-install-the-required-pulseaudio-software
relnote-firefox: --- → ?
Depends on: 1352780
Flags: needinfo?(gsquelart)
You need to log in before you can comment on or make changes to this bug.