Closed Bug 837564 Opened 7 years ago Closed 7 years ago

Implement runtime backend switching for cubeb

Categories

(Core :: Audio/Video, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla22

People

(Reporter: kinetik, Assigned: kinetik)

References

Details

Attachments

(1 file)

This is required to allow PulseAudio to be enabled by default, so that the ALSA backend will cover any machines that don't support PA.  In the future, we'll also need this for selecting backends on Windows (WinMM and XAudio2).
Blocks: 783733
We will also need this to choose between the JNI/AudioTrack (once written) and the OpenSL backend on Android (since on newer Android devices, OpenSL can give us super low latency).
I'm going to need this soon (now that I have a working android 2.2 backend that does not use JNI so we have all platforms covered with cubeb).

I plan to start working on this in a day or two if there is no activity here, so if you have an idea on how you want things to look in Cubeb, shoot!
I'm already working on it.  There's a new branch in bit, switchable-backends.  Sorry, I should've assigned the bug to myself.  Please let me know if there's anything specific you need.
s/bit/git/
Assignee: nobody → kinetik
Status: NEW → ASSIGNED
(In reply to Paul Adenot (:padenot) from comment #2)
> I'm going to need this soon (now that I have a working android 2.2 backend
> that does not use JNI so we have all platforms covered with cubeb).

I don't think this blocks your work because we can switch from using AudioTrack via JNI in sydneyaudio to AudioTrack via JNI in cubeb everywhere (which is still a win, since then we can get rid of sydneyaudio), and enable the OpenSL backend once the runtime switching is done, right?

Having said that, the work is almost complete now, so the answer probably doesn't matter much. :-)

I want to clean a few things up, but the basics are in place now.  Try push of not-quite-final patch: https://tbpl.mozilla.org/?tree=Try&rev=e268e3d2edb5
Attached patch patch v0Splinter Review
Paul's probably the best person to review this given his recent work on cubeb.

The main change is the introduction of a v-table to the cubeb context which allows the backend to be selected at runtime.  Right now the selection is crude: try compiled-in backends in a defined order until one works, otherwise fail.  Hopefully we'll never need anything more sophisticated.

This also includes changes to cubeb_pulse.c to allow runtime library loading, so that we can ship the Linux build with ALSA and PulseAudio enabled without requiring PulseAudio libraries to be installed.

Other than those changes, the rest of it is mechanical transformation: renaming cubeb_* in each backend to $BACKEND_*.

Try run: https://tbpl.mozilla.org/?tree=Try&rev=9bf8d19cc9e2

The Win32 build failure was fixed: https://tbpl.mozilla.org/?tree=Try&rev=1f3877f49f78
Attachment #722629 - Flags: review?(paul)
Comment on attachment 722629 [details] [diff] [review]
patch v0

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

::: media/libcubeb/src/cubeb.c
@@ +93,5 @@
> +#ifdef USE_WINMM
> +    winmm_init,
> +#endif
> +#ifdef USE_DIRECTSOUND
> +    directsound_init,

It might be a good idea to put this before WINMM, so we open the door for lower latency if the user's system supports DirectSound (pretty much every Windows system at that point) and we finish the DirectSound backend. And depending on the driver.

I was considering writing a WASAPI backend to improve the latency further, but this is outside the scope of this bug.

::: media/libcubeb/src/cubeb_opensl.c
@@ +171,2 @@
>  {
> +  dlclose(ctx->lib);

One of the last cubeb commits on github is moving this below the two |Destroy| calls, because it can cause a crash. I believe this is not intentional.
Attachment #722629 - Flags: review?(paul) → review+
(In reply to Paul Adenot (:padenot) from comment #7)
> It might be a good idea to put this before WINMM, so we open the door for
> lower latency if the user's system supports DirectSound (pretty much every
> Windows system at that point) and we finish the DirectSound backend. And
> depending on the driver.
> 
> I was considering writing a WASAPI backend to improve the latency further,
> but this is outside the scope of this bug.

I'll move it up, but I suspect the DirectSound backend will die in favour of a WASAPI or XAudio2 one.  I thought there was a bug on file for an XAudio2 backend, but I can't find it now so I may never have filed it.  Anyway, if WASAPI looks like a better option, we can definitely go that direction.
 
> One of the last cubeb commits on github is moving this below the two
> |Destroy| calls, because it can cause a crash. I believe this is not
> intentional.

Good catch.  It's because this patch is from the switchable-backends branch, I hadn't merged back to master and picked up that fix.  I'll do that before landing.
https://hg.mozilla.org/mozilla-central/rev/463a06b91cbf
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Blocks: 851149
Blocks: 851403
You need to log in before you can comment on or make changes to this bug.