JACK audio output support [initial version] [cubeb]

RESOLVED FIXED in Firefox 50

Status

()

Core
Audio/Video: cubeb
P5
enhancement
Rank:
55
RESOLVED FIXED
5 years ago
13 days ago

People

(Reporter: David Richards, Assigned: David Richards)

Tracking

(Blocks: 2 bugs)

Trunk
mozilla50
All
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox50 fixed)

Details

Attachments

(2 attachments, 20 obsolete attachments)

1.89 KB, patch
glandium
: review+
Details | Diff | Splinter Review
188.85 KB, patch
Details | Diff | Splinter Review
(Assignee)

Description

5 years ago
Created attachment 653015 [details]
A cubeb jack implementation

JACK audio connection kit is the audio routing software used for professional audio production, primarily on Linux, but also on legacy operating systems. Attached is an initial cubeb implementation for JACK output. I've been using it for a few days without issue, but a few enhancements and fixes are needed.

1) Optionally connect mono output streams to two output ports
2) Move #define options to preferences
3) Resampling < 44100 to >= 44100 is broken.
4) Better handle of jack server stop/restart
5) draining vs. underrunning
6) unknown uknowns

Comment: This patch was made for my own purposes of being able to better debug firefox A/V handling, but I'm quite certain I am not the only person in the world that would enjoy having JACK output from firefox. When I first got it working, I was watching webm videos on youtube, some old techno documentaries recorded from TV in the 90's, one of them had quite an annoying hiss in the audio, so I routed the output through an EQ and removed the hiss. As an A/V geek, this was quite a moving experience, Firefox felt like a pro tool.
Component: General → Video/Audio
Product: Firefox → Core
Attachment #653015 - Attachment mime type: text/x-csrc → text/plain
I've also wished firefox had jack support. For example, when watching video tutorials with music software running.
(Assignee)

Updated

5 years ago
Assignee: nobody → drichards
Duplicate of this bug: 812900
Depends on: 837564
Blocks: 812900

Comment 3

4 years ago
Hi David (Richards) and al,
Your patch is a very good starting point. 
It's out of date - relative to the architecture improvements of Cubeb (e.g dynamic backend selection) - and some bugs/limitation remain, but I'm currently working on updating and improving it.
I'll send an updated version very soon.

Comment 4

4 years ago
OK, I've got a working version of JACK audio output on github (user Ace17, branch jack_support, forked from cubeb).
(Assignee)

Comment 5

4 years ago
Created attachment 798195 [details] [diff] [review]
firefox_jack_build.patch

new option for .mozconfig:

ac_add_options --enable-jack

Comment 6

4 years ago
Comment on attachment 798195 [details] [diff] [review]
firefox_jack_build.patch

Review of attachment 798195 [details] [diff] [review]:
-----------------------------------------------------------------

Wouldn't it rather be:

CPP_SOURCES += [
         'cubeb_jack.cpp'
     ]

Instead of "CSRCS" ?
(Assignee)

Comment 7

4 years ago
Created attachment 798211 [details] [diff] [review]
firefox_jack_build.patch
Attachment #798195 - Attachment is obsolete: true
(Assignee)

Comment 8

4 years ago
Created attachment 798213 [details]
cubeb_jack.c

Attempt to cleanup Sebastien Alaiwan's patch, but I can't figure out the build system today..

Withe the combination of C++, build system, mutexes, linking, 32bits and all the rest I can't really say I have a clue whats going on here anymore. I do wish Fx to have JACK support even if it needs to be compile-it-yourself only, so I gave it a go for far to many hours and hopefully this can be a team effort.
(Assignee)

Comment 9

4 years ago
I was somehow under the impression that the cubeb side of things needed to be C only, so my whole effort there might just be one big thinko. Oops. The main challenge to me is how to get the link to link to jack without doing the usual Fx nom nom. Probably one of the other folks watching this bug can advise on how to get it to just simply build. If other interested parties can figure out how to make this build in the right way, and be OK with it being C based, I would be happy to maintain this and help it reach ostensible perfection. When a mutex combines with a usleep and C++ autopointers, my brainwaves go into an anti-pattern.

Comment 10

4 years ago
Hi David,

It's nice to have some feedback from you. I put you in copy when I sent by email the first JACK patch to Matthew Gregan. However, your email address seems not to exist anymore.

Before starting, I asked Matthew for advice about which language I should use for this new backend, and he answered me that there was a slight preference for C, but that C++ also was fine (some cubeb backends are in C++). I did not want to allow C99 specificities to creep in.

I also had to add a mutex to prevent concurrent access to the streams, between cubeb exported functions (create_stream) and the refill thread. And I thought that scope-based unlocking ("AutoLock") was cleaner and safer than explicit unlocking. This has saved me from the headache of missing one "unlock" before one "continue" in stream_refill_thread ;-)

If you absolutely want the backend to be C, that's fine, I can live with that.
But please don't use the term "cleanup" when all you mean is "translating from C++ to C"! Especially if it is followed by my complete name.

I also spent (a lot of) time yesterday trying to get the link right under Firefox. Didn't succeed. I'm not comfortable enough with the build system yet, and it's painfully slow.

Did you succeed with your C version which explicit loads libjack?

Comment 11

4 years ago
OK, i found what the link error was: cubeb_jack.cpp gets compiled with gcc-hidden.h pre-included. This file only contains a #pragma GCC visibility push(hidden).
This ends up marking all libjack functions as "hidden". When libxul is created, the linker excludes external libraries when searching for libjack functions.

One workaround, in cubeb_jack.cpp source code, is to wrap jack header inclusions this way:

#pragma GCC visibility push(default)
#include <jack/jack.h>
#pragma GCC visibility pop

It feels clumsy though ; does anyone know the right way to overcome this? And why such a hack isn't needed with the ALSA backend?

Comment 12

4 years ago
I managed to build it, and it works!
Here's the full firefox patch attached.

Comment 13

4 years ago
Created attachment 798262 [details] [diff] [review]
firefox_cubeb_jack_output.diff
(Assignee)

Comment 14

4 years ago
Sebastien: Jolly good! Let it hearby be known to all that I did not clean up your patch at all, I mangled it.

Regarding linking, IMO its not very clumsy considering the complexity of the Fx build process. Seems like it might even be the most correct way. The reason it is not needed for ALSA is because ALSA is part of the linux platform, and is used by other components.

Regarding C, its all about if its me maintaining this code, nothing else, my mind is inept at reasoning about C++ code, I spilt much blood finding that out. I will be a very happy user of your version otherwise :D And I thank you for your efforts here!
Thanks!  I've merged the changes upstream.  To import it into the Firefox source, please prepare a patch using media/libcubeb/update.sh.  Include the necessary Firefox build changes in a separate patch, as that will need to be reviewed by a build system peer.

Comment 16

3 years ago
So, what's a status on that, people ? Is it possible to use JACK with FF directly ? Is this code merged ? Right now i'm stuck with libflashsupport-jack for flash & PA-JACK sink for FF which works nicely but is pretty dumb construct since FF has GStreamer support build-in with native jack sink which it refuses to use.

Anyone cares to write up some wiki page or something about this "cubeb" thing ? There are thousands of "help" pages about how one should nullify cache, cookies and entire profile on every bother but this and bug 812900 are the only sources of info on the real deal.

Comment 17

3 years ago
AFAIK, there are two blocking points:
- the cubeb-jack code should use explicit library loading (dlopen, as the cubeb-portaudio code does). Currently, the generated Firefox binary won't load unless the JACK libraries are present on the system. Many GNU/Linux distributions would ship Firefox with JACK disabled.
- I did not finish the work of modifying the Firefox build process to enable JACK support. The last time I worked on this, the build would take ages to complete/fail even on a fast machine, I got fed up with trial-and-error and gave up. That's a pity because the work is nearly complete here (see the patch in my previous post in this thread).

However, I've been running the same JACK-enabled Firefox binary (branded as "Nightly") since then, and I've had no issue with audio.

Comment 18

3 years ago
Created attachment 8505278 [details] [diff] [review]
#1: update media/libcubeb to a JACK-enabled version.

Here's a patch updating media/libcubeb to the most recent version, which is JACK-enabled.
Attachment #798262 - Attachment is obsolete: true

Comment 19

3 years ago
Created attachment 8505279 [details]
#2: update configure.in to allow enabling JACK output.

And here's a second patch to the build system which allows enabling JACK output.

Comment 20

3 years ago
(In reply to Sebastien Alaiwan from comment #18)
> Created attachment 8505278 [details] [diff] [review]
> #1: update media/libcubeb to a JACK-enabled version.
> 
> Here's a patch updating media/libcubeb to the most recent version, which is
> JACK-enabled.

Patching Firefox 33 from OpenSUSE (https://build.opensuse.org/package/show/home:X0F:HSF/MozillaFirefox) fails with: 
[  130s] patching file media/libcubeb/README
[  130s] patching file media/libcubeb/README_MOZILLA
[  130s] patching file media/libcubeb/include/cubeb.h
[  130s] Hunk #2 succeeded at 150 (offset -6 lines).
[  130s] Hunk #3 succeeded at 194 (offset -11 lines).
[  130s] Hunk #4 succeeded at 218 (offset -11 lines).
[  130s] Hunk #5 FAILED at 279.
[  130s] 1 out of 5 hunks FAILED -- saving rejects to file media/libcubeb/include/cubeb.h.rej
<bunch of similar stuff further in https://build.opensuse.org/build/home:X0F:HSF/openSUSE_13.1/x86_64/MozillaFirefox/_log>

Comment 21

3 years ago
Hi, the patch was meant to be applied to the current head of: https://hg.mozilla.org/mozilla-central

By the way, about this "cubeb thing":
- AFAIU it's the audio I/O library firefox uses to play HTML5 audio
- It's hosted in its own separate repository on github, but the cubeb code version currently used by firefox is copied into the firefox repository (see media/libcubeb/update.sh).
- it abstracts firefox from OS-specific audio I/O APIs (JACK, ALSA, WASAPI, etc.).

Please note that flash-based audio doesn't go through cubeb, you need to use libflashsupport-jack to send it to JACK.

Comment 22

3 years ago
(In reply to Sebastien Alaiwan from comment #21)
> Hi, the patch was meant to be applied to the current head of:
> https://hg.mozilla.org/mozilla-central

I've wanted to make a build in my OpenSUSE repo and to include it in the live image (a spin that uses JACK by default everywhere) too. But with bleeding edge code that wouldn't be practical, unless it's that way as a merge submission for inclusion into a near release. Is Mozilla going to do it already ?
Is there a straightforward way to make use of it in a release now ?

> Please note that flash-based audio doesn't go through cubeb, you need to use
> libflashsupport-jack to send it to JACK.

So i've noticed and am trying to get rid of flash altogether by using HTML5 & Shumway wherever possible. But without native Firefox JACK upstream support, using damned flash with its aforementioned shim is more reliable solution now.

Comment 23

3 years ago
Created attachment 8508910 [details] [diff] [review]
Quick-and-dirty patch to be applied to firefox version 33

Comment 24

3 years ago
Hi Sergey, I've made a patch based on firefox version 33 so you can test it.

Comment 25

3 years ago
Created attachment 8509222 [details] [diff] [review]
firefox-v33-jack-output.patch_remove-bad-chunks.patch

Patch failed with errors:
[  166s] + /usr/bin/cat /home/abuild/rpmbuild/SOURCES/firefox-v33-jack-output.patch
[  166s] + /usr/bin/patch -p1 --fuzz=0
[  168s] patching file configure.in
[  168s] Hunk #1 succeeded at 5485 (offset 5 lines).
...
[  168s] patching file media/libcubeb/src/cubeb_opensl.c
[  168s] Hunk #11 FAILED at 706.
[  168s] Hunk #12 succeeded at 739 (offset -1 lines).
[  168s] Hunk #13 succeeded at 790 (offset -1 lines).
[  168s] 1 out of 13 hunks FAILED -- saving rejects to file media/libcubeb/src/cubeb_opensl.c.rej
...
[  168s] patching file media/libcubeb/tests/test_sanity.cpp
[  168s] Hunk #16 FAILED at 567.
[  168s] Hunk #17 FAILED at 579.
[  168s] 2 out of 17 hunks FAILED -- saving rejects to file media/libcubeb/tests/test_sanity.cpp.rej
[  168s] patching file media/libcubeb/update.sh
[  168s] error: Bad exit status from /var/tmp/rpm-tmp.J06OmQ (%prep)

I removed bad chunks from but then build failed with "missing cubeb_jack.cpp".
Then i have put cubeb_jack.c from here as cubeb_jack.cpp but it didn't like it either.

Comment 26

3 years ago
Can you share the exact url+tag of firefox that are you using?

Comment 27

3 years ago
Created attachment 8509806 [details] [diff] [review]
20141022 git cubeb for Firefox 33

As i mentioned earlier, it a slightly modified build: https://build.opensuse.org/package/show/home:X0F:HSF/MozillaFirefox
of what seems to be the official Mozilla one for OpenSUSE: https://build.opensuse.org/package/show/mozilla:Factory/MozillaFirefox
In spec-file it says: "33.0 2014101000".

I also tried to just
1) put current git (20141022/43aeb40397254022020912885ff6af9fa8574585) of cubeb in Firefox
2) applying "--enable-jack" and "SOURCES += cubeb_jack.cpp" parts from these patches to configure.in and moz.build
3) building with "export MOZ_JACK=1" and "ac_add_options --enable-jack"
4) making "media.use_cubeb=true" parameter for resulting build
But nothing seems to change.

Comment 28

3 years ago
Ahhh, success !
Maybe i just missed it the first time, but then i tested a build with that patch again, this time with --disable-[alsa,pulse], Firefox has created a jack client and stated pumping sound in it. I will try again with enabled pulseaudio, just to check a viable fallback.

But, i also noticed a problem: client name was "AudioStream" and so were port names instead of "cubeb" or, better yet, "Mozilla Firefox", even though libcubeb code has the line "const char* jack_client_name = "cubeb";".

Comment 29

3 years ago
I wasn't mistaken, it only works if pulseaudio is disabled in build.

Moreover, there are some more problems in addition to wrong names:
1) Sometimes (not always) it creates a lot of ports (recently i opened 1 video and it spawned 7 pairs) for no good visible reason. Maybe it's just figured to create them for some not yet completely loaded Youtube pages, it likes to create a pair for each video.
2) Rarely it creates several clients, additional one with names ending with "-<2 digits>"
3) Rarely it weirdly **** itself and infinitely throws jack errors like:
Thu Oct 23 19:15:49 2014: ERROR: JackEngine::XRun: client = jamin was not finished, state = Triggered
Thu Oct 23 19:15:49 2014: ERROR: JackEngine::XRun: client = AudioStream was not finished, state = Triggered
Thu Oct 23 19:15:49 2014: ERROR: JackAudioDriver::ProcessGraphAsyncMaster: Process error
<constant stream of those>
Thu Oct 23 19:15:55 2014: ERROR: JackAudioDriver::ProcessGraphAsyncMaster: Process error
<here Firefox was closed>
Thu Oct 23 19:15:55 2014: ERROR: JackAudioDriver::ProcessGraphAsyncMaster: Process error
Thu Oct 23 19:15:55 2014: Client 'AudioStream' with PID 18838 is out
Thu Oct 23 19:15:55 2014: ERROR: Cannot write socket fd = 23 err = Broken pipe
Thu Oct 23 19:15:55 2014: ERROR: CheckRes error
Thu Oct 23 19:15:55 2014: ERROR: Could not write notification
Thu Oct 23 19:15:55 2014: ERROR: ClientNotify fails name = AudioStream notification = 1 val1 = 0 val2 = 0
Thu Oct 23 19:15:55 2014: Client 'AudioStream-01' with PID 18945 is out
4) Rarely Firefox crashes on loading a page (but it may be due to something else, it's not uncommon for it to crash on Youtube for me)

Note, that i'm redirecting all clients, including it, to jamin for good mixing in realtime.

Comment 30

2 years ago
Created attachment 8565668 [details] [diff] [review]
firefox-jack.patch

Hi all,

I am also interested in seeing support for JACK audio in Firefox. Attaching a slimmed down version of Sergey's last patch, which only includes changes related to the build in update.sh, moz.build in libcubeb and configure.in.

The second file has a small change in cubeb.c, which moves the initialization attempt of JACK higher up, so it gets picked up before PulseAudio if the JACK server is running.

Both patches should apply cleanly to revision 09f496. (You will have to apply the second one after running the "updated" update.sh, as any changes in cubeb.c would be overwritten by that script.)

Comment 31

2 years ago
Created attachment 8565670 [details] [diff] [review]
cubeb-init-order.patch

Comment 32

2 years ago
These patches really should go upstream: 1st to FF and 2nd to cubeb. Recently I've almost gave up on straight JACK support and were ready to leave it with PA shim since many FF's actually critical features hold on snots and crutches anyway. Please, push them for inclussion.
Component: Audio/Video → Audio/Video: MSG/cubeb/GMP
Anyone still working on this?

Comment 34

2 years ago
Comment on attachment 8565670 [details] [diff] [review]
cubeb-init-order.patch

Review of attachment 8565670 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me.

Comment 35

2 years ago
I've been using a JACK-patched Firefox for two years now.
It definitely works well ; and I'd be very glad to see these patches upstreamed, because having to build my own Firefox on a regular basis is a real pain.

David, are you still there? We've got the JACK backend upstreamed in cubeb, including the JACK library dynamic loading, and the integration with FF build system. Is there something we're still missing?
It's actually been requested to me a number of times during conference when talking about Web Audio, and I think the cost would not be high.

This would mean:
- move jack higher in cubeb.c (has to be done upstream)
- include the file for jack in the distribution of cubeb we use in Firefox, and pick up the change to cubeb.c
- Wire it up in configure.in and in the cubeb build system
- Make sure it does not break at compile time nor at run time for users that don't have jack headers or libraries. We already have some infrastructure for late binding in the jack impl, so it should not be very problematic

Sebastien, maybe you have done some or all of the above, do you mind sharing your setup here so we can upstream? Let me know if you need any kind of help, I'll make sure to get you answer and reviews.
Flags: needinfo?(ace17)

Comment 37

2 years ago
Hi Paul, thanks for your support.
- About the JACK/PA initialization order, I've just submitted a pull request on github to Matthew.
I've indeed done everything else you mention, it was one year ago. I'll re-do a recent build of FF and share my settings!
Flags: needinfo?(ace17)

Comment 38

2 years ago
Created attachment 8672926 [details] [diff] [review]
Wiring of JACK in the FF build system (configure.in, moz.build, update.sh)
Attachment #8505278 - Attachment is obsolete: true
Attachment #8505279 - Attachment is obsolete: true
Attachment #8508910 - Attachment is obsolete: true

Comment 39

2 years ago
OK, Matthew has merged my pull request.

So now, one can:
- do a fresh checkout from https://hg.mozilla.org/mozilla-central
- apply the above patch
- run: cd media/libcubeb && ./update.sh /path/to/HEAD/cubeb
- build a JACK-enabled firefox with: ./configure --enable-jack && make

Please let me know if I should include a separate patch corresponding to the modifications resulting from running "media/libcubeb/update.sh" (basically, overwriting embedded cubeb files with updated ones).

Updated

2 years ago
Flags: needinfo?(padenot)
(In reply to Sebastien Alaiwan from comment #39)
> OK, Matthew has merged my pull request.
> 
> So now, one can:
> - do a fresh checkout from https://hg.mozilla.org/mozilla-central
> - apply the above patch
> - run: cd media/libcubeb && ./update.sh /path/to/HEAD/cubeb
> - build a JACK-enabled firefox with: ./configure --enable-jack && make
> 
> Please let me know if I should include a separate patch corresponding to the
> modifications resulting from running "media/libcubeb/update.sh" (basically,
> overwriting embedded cubeb files with updated ones).

Yes, that would be cool, thanks.
Flags: needinfo?(padenot)

Comment 41

2 years ago
Created attachment 8673790 [details] [diff] [review]
mozilla-cubeb-jack.diff

Here it is, a patch corresponding to the modifications resulting from running "media/libcubeb/update.sh"

Updated

2 years ago
Flags: needinfo?(padenot)
(In reply to Sebastien Alaiwan from comment #41)
> Created attachment 8673790 [details] [diff] [review]
> mozilla-cubeb-jack.diff
> 
> Here it is, a patch corresponding to the modifications resulting from
> running "media/libcubeb/update.sh"

Sebastien, I'm in Japan for the TPAC this week, and pretty busy with W3C things, I'll look at this when I'm back next week.
Component: Audio/Video: MediaStreamGraph → Audio/Video: cubeb
We're updating cubeb to a more recent version in another bug, so I think the jack changes will go in as well.
Flags: needinfo?(padenot)

Comment 44

2 years ago
Excellent, thanks!
Cubeb has been updated - should this bug be marked resolved?
Flags: needinfo?(padenot)

Updated

2 years ago
Rank: 55
Priority: -- → P5

Comment 46

2 years ago
No ;
The update doesn't seem to include the required files (e.g cubeb_jack.cpp)
As a reminder, these files are copied by the cubeb-updater script ( update-cubeb.sh ).
"update-cubeb.sh" needs to be patched with the patch provided in this ticket before running it.

Comment 47

2 years ago
Guys, please let me know if you need any help with this.

I understand that JACK compatibility might not exactly be a top priority ; but this ticket has been opened 3 years ago, and a complete working patch was posted two years ago. Let's patch the damn thing and put this ticket behind us!

The JACK code was integrated to the "github" cubeb two years ago.
However, it's NOT enough to update the cubeb version on which Firefox points.

1)
Firefox embeds a _partial_ copy of cubeb, which only include a subset of the audio backends.
Have a look at media/libcubeb/update.sh to see what I mean.
At the moment, this subset doesn't include "cubeb_jack.cpp".

2)
The Firefox build system needs to be modified to add the "--enable-jack" option to the build configure scripts.

The above patch "8672926" do both.

Please do not let this ticket go into limbo again ... because when it gets some attention again, the patch will be obsolete (and I don't specially look forward modifying the Firefox build system again)
Comment on attachment 8672926 [details] [diff] [review]
Wiring of JACK in the FF build system (configure.in, moz.build, update.sh)

Putting up for review, since it wasn't actually submitted for review (needinfo is more of a generic "hey, look at this bug if you could").  Needs build-peer review, plus review by a cubeb peer (kinetik or padenot), though most changes here are build-system). 

When running the update script, check that the jack file gets added by the script (not just copied over).
Attachment #8672926 - Flags: review?(mh+mozilla)
Attachment #8672926 - Flags: review?(kinetik)
Comment on attachment 8672926 [details] [diff] [review]
Wiring of JACK in the FF build system (configure.in, moz.build, update.sh)

Review of attachment 8672926 [details] [diff] [review]:
-----------------------------------------------------------------

::: configure.in
@@ +5601,5 @@
> +         [echo "$MOZ_JACK_PKG_ERRORS"
> +          AC_MSG_ERROR([JACK audio backend requires jack libraries])])
> +fi
> +
> +AC_SUBST(MOZ_JACK)

An alternative to this would be to import the jack headers (assuming there are no license issues preventing that; I haven't looked what jack's license is), and build against them unconditionally. Since cubeb turns jack support at runtime with dlopen, that would make official builds support jack out of the box. Whether this is desired or not is up to the media team, though.

::: media/libcubeb/src/moz.build
@@ +29,5 @@
> +    SOURCES += [
> +        'cubeb_jack.cpp',
> +    ]
> +    USE_LIBS += [
> +        'speex',

This is not needed, because speex is already in gkmedias.
Attachment #8672926 - Flags: review?(mh+mozilla) → review+

Comment 50

2 years ago
Having official builds support JACK out of the box seems very attractive. JACK2 is LGPL so there shouldn't be any licensing issue.
Is the explicitely loading of 'libjack.so.0' supposed to protect us from ABI changes?
(this is taken care of by kinetik and glandium, clearing NI).
Flags: needinfo?(padenot)
Comment on attachment 8672926 [details] [diff] [review]
Wiring of JACK in the FF build system (configure.in, moz.build, update.sh)

Review of attachment 8672926 [details] [diff] [review]:
-----------------------------------------------------------------

> An alternative to this would be to import the jack headers (assuming there
> are no license issues preventing that; I haven't looked what jack's license
> is), and build against them unconditionally. Since cubeb turns jack support
> at runtime with dlopen, that would make official builds support jack out of
> the box. Whether this is desired or not is up to the media team, though.

I'd be okay with this provided:

- PulseAudio remains the default and enabling JACK at runtime required setting a preference
- The in-use audio backend is reported in about:support
Attachment #8672926 - Flags: review?(kinetik) → review+

Comment 53

2 years ago
(In reply to Matthew Gregan [:kinetik] from comment #52)
> I'd be okay with this provided:
> 
> - PulseAudio remains the default and enabling JACK at runtime required
> setting a preference
> - The in-use audio backend is reported in about:support

Guess what ? I would like for ALSA to have remained a default while PulseAudio to require setting a preference and for in-use audio backend be reported in about:support. But it didn't stop Mozilla from silently updating cubeb when it implemented its current auto-selection for backends and made PA the default. And nothing in this logic has changed so why the hell this is suddenly a requirement now ?

This is just an update of cubeb, what you ask needs reimplementation of backend selection code. This looks like an artificial barrier, which doesn't even pertain to the subject and should be considered a separate issue, for JACK support, which is long overdue as it is.

Comment 54

2 years ago
Sergey,
although I'd also prefer JACK to be the default, we have to admit Matthew might have a point here.
(And a Firefox allowing to enable JACK at runtime (via a config variable) would still be a huge improvement!).

Matthew, would you mind sharing your concerns about having, in Firefox, a 'default-to-JACK' cubeb?

Why not, at this point, stick to the --enable-jack option at configure stage?
The decision would then belong to the package maintainers.

Comment 55

2 years ago
(In reply to Sebastien Alaiwan from comment #54)
> Sergey,
> although I'd also prefer JACK to be the default, we have to admit Matthew
> might have a point here.
> (And a Firefox allowing to enable JACK at runtime (via a config variable)
> would still be a huge improvement!).

Of course he is ! And it should have been done years ago regardless of JACK. But it's no reason to postpone JACK support yet again.
Also, one can check what Firefox uses by looking at client status in pavucontrol and qjackctl. It falls back to PA if JACK server is not running, AFAIK, so no one will even notice the change.
(In reply to Sebastien Alaiwan from comment #54)
> Why not, at this point, stick to the --enable-jack option at configure stage?
> The decision would then belong to the package maintainers.

Yeah, that's fine, the patch has r+ and can land.  Comment 52 is specifically in response to the suggestion of supporting JACK via dynamic loading in official release builds.

(In reply to Sebastien Alaiwan from comment #54)
> Matthew, would you mind sharing your concerns about having, in Firefox, a
> 'default-to-JACK' cubeb?

It comes down to concerns about support and long term code maintenance.  The fewer combinations of platforms and audio backends we have enabled by default, the less testing and debugging is required and the lower the risk of problems for users.

Comment 57

a year ago
Sorry for the noise, but I'm long awaiting this feature. If I see it correct the patch is ready and can land. Is there some kind of schedule for this?
There is a new backend that is being written, and that is better (event-driven). I've done the review on github, and I'm waiting for a new version of the patch.

Comment 59

a year ago
You mean cubeb is replaced with another audiobackend? What is necessary for getting JACK in it?
No, I meant a new backend for cubeb: cubeb can already use Pulse and Alsa on Linux, and it's growing the capability to use Jack directly. Note that the new Jack backend also appears to work on Windows and OSX.

Comment 61

a year ago
Created attachment 8761179 [details] [diff] [review]
New JACK backend gecko glue (zamaudio)

My new duplex, event-based JACK backend has been reviewed and merged in cubeb pending the next uplift, this patch is to glue it into gecko.

Comment 62

a year ago
Created attachment 8761206 [details] [diff] [review]
New JACK backend gecko glue (zamaudio) (2)

This adds extra check for --enable-jack so JACK system includes are not necessary unless building with JACK.
Attachment #8761179 - Attachment is obsolete: true

Comment 63

a year ago
Created attachment 8761882 [details] [diff] [review]
firefox-47-libcubeb-with-jack-support_20160609.patch

Ready-to-use patch introducing latest cubeb to Firefox 47 based on attachment 8761206 [details] [diff] [review] (there is no old.configure file though but it seems to work anyway if envvar MOZ_JACK=1 is set before building).
Attachment #8509222 - Attachment is obsolete: true
Attachment #8509806 - Attachment is obsolete: true
Duplicate of this bug: 1279832

Comment 65

a year ago
As there have been a few different versions of the JACK backend in the pipeline, I am posting this comment to clarify the *current* status of this bug:  I have written a new, event-based, duplex JACK backend and it has been reviewed and merged in upstream cubeb.  Comment 61 provides the final patch needing review to get the whole thing glued into gecko, once the JACK code is uplifted from cubeb into gecko.  All the previous patches are obsolete as far as JACK backend is concerned, as far as I am aware.  Thanks to everyone involved, let's get this done!

Comment 66

a year ago
Thanks to you, Damien!
I hope we can get this done this time!

Updated

a year ago
Attachment #8565668 - Attachment is obsolete: true

Updated

a year ago
Attachment #8565670 - Attachment is obsolete: true

Comment 67

a year ago
Marking my patches as obsolete because the change in initialization order has already made its way to upstream. A hearty +1 from me as well, Damien, looking forward to see this land in Firefox soon.
Blocks: 1221581

Comment 68

a year ago
Created attachment 8764496 [details] [diff] [review]
New JACK backend gecko glue (zamaudio) (3)

Here is the latest patch for gluing the JACK backend into gecko, I had previously left out moz.build configuration.
Attachment #8761206 - Attachment is obsolete: true
Comment on attachment 8764496 [details] [diff] [review]
New JACK backend gecko glue (zamaudio) (3)

glandium, this is a rebased version of a previous patch that did roughly the same thing. We've already imported the code that implements the jack backend, this is just the build system bits.

This new backend is not enabled by default for now.
Attachment #8764496 - Flags: review?(mh+mozilla)
Comment on attachment 8764496 [details] [diff] [review]
New JACK backend gecko glue (zamaudio) (3)

Review of attachment 8764496 [details] [diff] [review]:
-----------------------------------------------------------------

::: old-configure.in
@@ +4179,4 @@
>  AC_SUBST(MOZ_WEBM_ENCODER)
>  
>  dnl ==================================
> +dnl = Check JACK availability on Linux

Please don't add this to old-configure.in. Please do the corresponding change to toolkit/moz.configure instead. (see what is done for e.g. --enable-system-hunspell)
Attachment #8764496 - Flags: review?(mh+mozilla)

Comment 71

a year ago
Created attachment 8767914 [details] [diff] [review]
New JACK backend gecko glue (zamaudio) (4)

This patch addresses the request made by glandium.  Please review.
Attachment #8764496 - Attachment is obsolete: true
Attachment #8767914 - Flags: review?(mh+mozilla)
Comment on attachment 8767914 [details] [diff] [review]
New JACK backend gecko glue (zamaudio) (4)

Review of attachment 8767914 [details] [diff] [review]:
-----------------------------------------------------------------

What happened to the moz.build and update.sh changes?
Attachment #8767914 - Flags: review?(mh+mozilla)

Comment 73

a year ago
Created attachment 8768327 [details] [diff] [review]
New JACK backend gecko glue (zamaudio) (5)

glandium: You are completely right, I made a mistake in my git rebase.  Here is a proper patch.  I hope this will be the one!
Attachment #8767914 - Attachment is obsolete: true

Updated

a year ago
Attachment #8768327 - Flags: review?(mh+mozilla)

Comment 74

a year ago
No idea, whether this is related, but to compile Firefox 47 with the newest Cubeb (git-master) and this patch I also need cubeb_resampler_internal and cubeb_utils (Steps: checkout Firefox sources, apply Patch, clone upstream cubub, execute update.sh and then try to compile). I have to say, that beside the Jack-Patch also applied the Opensuse KDE patches.

Nevertheless, with this patch, Firefox with Jack works like a charm. Thank you for your work!

This is the part of the patch with the two files more in update.sh:
diff --git a/media/libcubeb/update.sh b/media/libcubeb/update.sh
index 07b0a98..ce2c7b2 100755
--- a/media/libcubeb/update.sh
+++ b/media/libcubeb/update.sh
@@ -8,15 +8,18 @@ cp $1/src/cubeb_alsa.c src
 cp $1/src/cubeb_winmm.c src
 cp $1/src/cubeb_audiounit.c src
 cp $1/src/cubeb_pulse.c src
+cp $1/src/cubeb_jack.cpp src
 cp $1/src/cubeb_sndio.c src
 cp $1/src/cubeb_opensl.c src
 cp $1/src/cubeb_audiotrack.c src
 cp $1/src/cubeb_wasapi.cpp src
 cp $1/src/cubeb_resampler.h src
 cp $1/src/cubeb_resampler.cpp src
+cp $1/src/cubeb_resampler_internal.h src
 cp $1/src/cubeb-speex-resampler.h src
 cp $1/src/cubeb_panner.h src
 cp $1/src/cubeb_panner.cpp src
+cp $1/src/cubeb_utils.h src
 cp $1/src/android/audiotrack_definitions.h src/android
 cp $1/src/android/sles_definitions.h src/android
 cp $1/LICENSE .
Attachment #8768327 - Flags: review?(mh+mozilla) → review+
Attachment #653015 - Attachment is obsolete: true
Attachment #798211 - Attachment is obsolete: true
Attachment #798213 - Attachment is obsolete: true
Attachment #8672926 - Attachment is obsolete: true
Attachment #8673790 - Attachment is obsolete: true
Attachment #8761882 - Attachment is obsolete: true
Keywords: checkin-needed
Needs rebasing.
Keywords: checkin-needed

Comment 76

a year ago
Created attachment 8771667 [details] [diff] [review]
New JACK backend gecko glue (zamaudio) (6)

Here is my patch (version 6), rebased on current master.  I tested it by clobbering my old build and reconfiguring/rebuilding firefox locally from scratch using this patch with --enable-jack in my mozconfig.
Attachment #8768327 - Attachment is obsolete: true
Attachment #8771667 - Flags: review?(mh+mozilla)
Changes are minimal and look good, I'm landing this carrying glandium's review forward.

Comment 78

a year ago
Pushed by paul@paul.cx:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4ab76338931e
Wire up the jack cubeb backend in the build system. r=glandium

Updated

11 months ago
Attachment #8771667 - Flags: review?(mh+mozilla) → review+

Comment 79

11 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/4ab76338931e
Status: NEW → RESOLVED
Last Resolved: 11 months ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50

Comment 80

11 months ago
Will this feature automatically appear in Firefox Beta nightly builds?  
I just tried the Beta 48 and did not see jack support. 
Or will this jack support feature only be available compiling it in? 

thanks

Comment 81

11 months ago
This is only available when making your own firefox build for now.

Comment 82

11 months ago
Ok, thanks.  I tried building this with jack-enabled.. I'm not seeing FF in QjackCtl connections.. Should I? 

~$ cat  mozconfig 
ac_add_options --enable-jack

and it seems to launch without any errors ...

 ./obj-x86_64-pc-linux-gnu/dist/bin/firefox  --enable-jack 

What am I doing wrong?  thanks.

Comment 83

11 months ago
Damien, is that expected ? This is only in Nightly for now, in mozilla-central.
Flags: needinfo?(damien)

Comment 84

11 months ago
Just to clarify. You are playing html5 content (e.g.: http://hpr.dogphilosophy.net/test/opus.opus)? Otherwise the jack connection is not created. It is not necessary to start FF with --enable-jack. Once it is build in, it should work.

Comment 85

11 months ago
kelargo: Just to clarify, you will not see "Firefox" in list of JACK connections, but "cubeb" instead.  Also, please make sure JACK is running *before* you launch a tab with audio present, otherwise a different audio backend will be chosen like pulse or alsa.  The default port connections are from cubeb's stereo output to first stereo JACK playback 1+2 ports.  Please confirm audio is working when these are verified.
Flags: needinfo?(damien)
Flags: needinfo?(ajones)

Comment 86

11 months ago
Created attachment 8777620 [details] [diff] [review]
libcubeb-with-jack-for-firefox48.patch

That's an "obsolete" patch for users of current (48) Firefox version since Nighty's (well, my FF's release version is always "Nighty" to get rid of the vendor lock) patch isn't enough and it doesn't apply.

jack support works fine, although cubeb is and always was (like I reported in 2014) the only jack client that gives a lot of xruns, especially on FF's shutdown (probably some kind of "zombie" cubeb processes from previously opened youtube videos). But I never let cubeb to directly output into "system" port, instead I redirect it via patchbay into "jamin" to fix up possible bad audio in videos such as interviews.
Flags: needinfo?(ajones)

Comment 87

10 months ago
Hi!

Thanks for this great feature, but I can't get it working with FF nightly on Debian Jessie (amd64). Steps to reproduce :

- start jackd daemon in RT mode
- download and start FF nightly without any arguments
- browsing https://webaudiodemos.appspot.com/midi-synth/index.html
- the Web Audio tab in dev tools says : "waiting for an audio context to be created"...
- no "cubeb" client in jackd connections

So, can you please specify a clear sequence to test this?

Comment 88

10 months ago
Guillaume, see comment 81.

Comment 89

10 months ago
Hi Paul! Oh OK, to me Nightly was the compiled version, but that is clear now. building.. ;)
Thanks

Comment 90

13 days ago
I've compiled mozilla-central firefox on ubuntu 17.04 (changeset:   363886:da66c4a05fda )
 with jack support and output (sources) Automatically get created when you visit a site with audio (i.e youtube).

However - input sink sources do not get created ; I don't see them appear in cadence/jack patch bay. So anything that relies on Mic input fails. i.e meet.jit.si / webrtc tests...

Another problem is that even using the pulseaudio bridge ; the same issue input sources even when connected correctly to the pulse-sink don't get recognized by firefox. In Chrome however Mic input correctly uses the plugged capture to pulse-sink. So I think this is a firefox bug in the way it detects and reports Mic presence information.

My personal view is that the Input/capture sink sources should be created on firefox start up rather than on a per-tab/site basis as with the output sources in the jack only case.
You need to log in before you can comment on or make changes to this bug.