783733 - JACK audio output support [initial version] [cubeb]

Status: RESOLVED FIXED

(Core :: Audio/Video: cubeb, enhancement, P5)

Description

[David Richards]

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

Comment 1

[Ralph Giles (:rillian)]

I've also wished firefox had jack support. For example, when watching video tutorials with music software running.

Comment 3

[Sebastien Alaiwan]

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

[Sebastien Alaiwan]

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

Comment 5

[David Richards]

Attachment: firefox_jack_build.patch

new option for .mozconfig:

ac_add_options --enable-jack

Comment 6

[Sebastien Alaiwan]

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" ?

Comment 7

[David Richards]

Attachment: firefox_jack_build.patch

Comment 8

[David Richards]

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

Comment 9

[David Richards]

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

[Sebastien Alaiwan]

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

[Sebastien Alaiwan]

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

[Sebastien Alaiwan]

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

Comment 13

[Sebastien Alaiwan]

Attachment: firefox_cubeb_jack_output.diff

Comment 14

[David Richards]

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!

Comment 15

[Matthew Gregan [:kinetik]]

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

[Sergey Kondakov]

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

[Sebastien Alaiwan]

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

[Sebastien Alaiwan]

Attachment: #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.

Comment 19

[Sebastien Alaiwan]

Attachment: #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

[Sergey Kondakov]

(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

[Sebastien Alaiwan]

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

[Sergey Kondakov]

(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

[Sebastien Alaiwan]

Attachment: Quick-and-dirty patch to be applied to firefox version 33

Comment 24

[Sebastien Alaiwan]

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

Comment 25

[Sergey Kondakov]

Attachment: 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

[Sebastien Alaiwan]

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

Comment 27

[Sergey Kondakov]

Attachment: 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

[Sergey Kondakov]

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

[Sergey Kondakov]

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

[Andras Peteri]

Attachment: 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

[Andras Peteri]

Attachment: cubeb-init-order.patch

Comment 32

[Sergey Kondakov]

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.

Comment 33

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Anyone still working on this?

Comment 34

[Sebastien Alaiwan]

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

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

Looks good to me.

Comment 35

[Sebastien Alaiwan]

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?

Comment 36

[Paul Adenot (:padenot)]

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.

Comment 37

[Sebastien Alaiwan]

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!

Comment 38

[Sebastien Alaiwan]

Attachment: Wiring of JACK in the FF build system (configure.in, moz.build, update.sh)

Comment 39

[Sebastien Alaiwan]

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

Comment 40

[Paul Adenot (: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.

Comment 41

[Sebastien Alaiwan]

Attachment: mozilla-cubeb-jack.diff

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

Comment 42

[Paul Adenot (: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.

Comment 43

[Paul Adenot (:padenot)]

We're updating cubeb to a more recent version in another bug, so I think the jack changes will go in as well.

Comment 44

[Sebastien Alaiwan]

Excellent, thanks!

Comment 45

[Randell Jesup [:jesup] (needinfo me)]

Cubeb has been updated - should this bug be marked resolved?

Comment 46

[Sebastien Alaiwan]

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

[Sebastien Alaiwan]

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 48

[Randell Jesup [:jesup] (needinfo me)]

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

Comment 49

[Mike Hommey [:glandium]]

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.

Comment 50

[Sebastien Alaiwan]

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?

Comment 51

[Paul Adenot (:padenot)]

(this is taken care of by kinetik and glandium, clearing NI).

Comment 52

[Matthew Gregan [: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]:
-----------------------------------------------------------------

> 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

Comment 53

[Sergey Kondakov]

(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

[Sebastien Alaiwan]

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

[Sergey Kondakov]

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

Comment 56

[Matthew Gregan [:kinetik]]

(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

[Gerion]

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?

Comment 58

[Paul Adenot (:padenot)]

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

[Gerion]

You mean cubeb is replaced with another audiobackend? What is necessary for getting JACK in it?

Comment 60

[Paul Adenot (:padenot)]

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

[Damien Zammit]

Attachment: 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

[Damien Zammit]

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

Comment 63

[Sergey Kondakov]

Attachment: 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).

Comment 65

[Damien Zammit]

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

[Sebastien Alaiwan]

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

Comment 67

[Andras Peteri]

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.

Comment 68

[Damien Zammit]

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

Comment 69

[Paul Adenot (:padenot)]

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.

Comment 70

[Mike Hommey [:glandium]]

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)

Comment 71

[Damien Zammit]

Attachment: New JACK backend gecko glue (zamaudio) (4)

This patch addresses the request made by glandium.  Please review.

Comment 72

[Mike Hommey [:glandium]]

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?

Comment 73

[Damien Zammit]

Attachment: 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!

Comment 74

[Gerion]

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 .

Comment 75

[Ryan VanderMeulen [:RyanVM]]

Needs rebasing.

Comment 76

[Damien Zammit]

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

Comment 77

[Paul Adenot (:padenot)]

Changes are minimal and look good, I'm landing this carrying glandium's review forward.

Comment 78

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4ab76338931e
Wire up the jack cubeb backend in the build system. r=glandium

Comment 79

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4ab76338931e

Comment 80

[kelargo1000]

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

[Paul Adenot (:padenot)]

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

Comment 82

[kelargo1000]

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

[Paul Adenot (:padenot)]

Damien, is that expected ? This is only in Nightly for now, in mozilla-central.

Comment 84

[Gerion]

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

[Damien Zammit]

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.

Comment 86

[Sergey Kondakov]

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

Comment 87

[guillaume.pellerin]

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

[Paul Adenot (:padenot)]

Guillaume, see comment 81.

Comment 89

[guillaume.pellerin]

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

Comment 90

[aenertia]

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.

Comment 91

[Mark Constable]

Just a usability comment but when patching FF into ie; Audour the FF audio system makes a typical connection directly to the playback ports every time a new ie; youtube video starts playing (not just on tab open) which renders using any kind of Jack mixer almost useless. As aenertia mentioned, if it was a once off auto-connection to capture/playback ports on startup then that would be manageable. Better still would be an about:config option to disable auto-connection altogether so apps like patchage and qjackctl/patchbay can work as expected.

Comment 92

[milasudril]

There is some more to it than auto-connection. It is also important to have a model of port lifespan.

Here are some ideas:

 * One client per tab
 * One port per Mono!! source per client. If there are multi-channel audio, create one port per channel in that source (2 ie two for stereo)
 * When a tab loads a new webpage, keep current ports alive. Add a function so the user can remove unused ports. This can be implemented by adding a "Reopen this tab"-feature (Close the tab, and open the same URL in a new tab), which may be useful for other explicit cleaning purpose.

OR:

 * One client per tab
 * A user-configurable number of ports that are persistent throughout the lifespan of this tab. Firefox mixes audio to any of these ports

OR:

 * Firefox one client
 * A user-configurable number of ports that Firefox uses at its best (maybe it needs an internal mixer)

The last option is guaranteed to work with an external mixer, due to the longest possible port lifetime. The other two may work, if there is a clever reconnection scheme.

Comment 93

[Mark Constable]

Yes, port lifespan is another issue. I get a serious audio glitch every time a ie; youtube video starts playing because the input port suddenly appears. For me this is a secondary issue because the main one remains that it also re-establishes a connection to the system playback ports which overrides my patchbay established connection to ie; a dedicated Ardour track. It's not feasible to manually intervene and disconnect the system playback ports whenever a new audio source starts playing. It may be possible to use some pre gating plugin to remove the audio glitch on Jack input port startup but the auto-connect to system playback ports blows away the option of even trying to fiddle with port persistence. I will test "jackd --autoconnect OR -a <modechar> ... e - Ignore self connect requests to external ports only" and see if that helps with the auto-connect issue.

Paul Davis mentioned there may be work on a "websocket <-> OSC" bridge so FF (or any bowser) could provide control of a headless(?) Ardour. If the Jack support in FF was slightly improved then there could be a world of interesting possibilities available.

Comment 94

[milasudril]

My point was: With a proper port management system, connections only requires to be established once. There is no need for any automatic reconnection. Manual port connection does not work when ports are dynamically allocated. By the way, Audacity have the same issue: It creates ports when playback/record starts, but the JACK system requires that ports are managed at session (not stream) level.

Comment 95

[Turbo]

hi, 

With FF version 59, (Arch Linux stock version) JACK doesn't work no more (even with a loopback device ALSA >> JACK).
In the Arch PKGBUILD JACK is enable by -enable -- jack and it was working quite well till this update.

Comment 96

[Turbo]

Here is the output message:

Sandbox: seccomp sandbox violation: pid 2279, tid 2393, syscall 49, args 57 139826132706112 110 0 13 0.
[Child 2279, MediaPlayback #3] WARNING: 7f2bd2842940 OpenCubeb() failed to init cubeb: file /build/firefox/src/mozilla-unified/dom/media/AudioStream.cpp, line 387
[Child 2279, MediaPlayback #3] WARNING: Decoder=7f2bd03fe5c0 [OnMediaSinkAudioError]: file /build/firefox/src/mozilla-unified/dom/media/MediaDecoderStateMachine.cpp, line 3755

Comment 97

[Matthew Gregan [:kinetik]]

> Sandbox: seccomp sandbox violation: pid 2279, tid 2393, syscall 49, args 57 139826132706112 110 0 13 0.

Looks like the sandbox is blocking bind().  You could try Firefox 60 (where we've moved audio to the chrome process to avoid sandbox restrictions in the content process), although it's untested with JACK.  Alternatively, you could lower the sandbox level via the security.sandbox.content.level pref, but be aware that you're reducing your security to get JACK working in that case.

Comment 98

[Turbo]

JACK is working with security.sandbox.content.level set to 0, and i can use a loopback device [ALSA >>JACK] when i set security.sandbox.content.level to 2. I will let the settings like this till FireFox 60 will be released, because JACK is back in FF 60, i have try it and it seems to be working well [may be some more intensive tests are needed but it seems ok].

Thanks a lot.

Comment 100

[fullmoonrabbit]

I am not sure where to paste what where in firefox..
Will some one please spell this out for a programing illiterate?
i want fire fox to show up in the jack connections.
Mad respect, please and thank you!

Comment 101

[Alex Chronopoulos [:achronop]]

I'm afraid you need some basic programming skills. You need to build Firefox from source using a specific option (ac_add_options --enable-jack) and follow up from there. You can see earlier discussions on this bug (like comment 85). In [1] you can find instructions about building firefox from source.

[1] https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions

Comment 102

[fullmoonrabbit]

thank you for the lead!

Comment 103

[Turbo]

hi,
I recently update FF from 83 to 84 version, and FF no longer connect automatically to Jack output, have to do it manually in a patch-bay, it disconnect itself when you go forward in a video ..