Allow building against sndio on more than just OpenBSD

NEW
Unassigned

Status

enhancement
2 years ago
5 months ago

People

(Reporter: jbeich, Unassigned)

Tracking

Trunk
Unspecified
FreeBSD
Dependency tree / graph

Firefox Tracking Flags

(firefox55 affected)

Details

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
sndio supports SunAudio, ALSA and OSS. Its libcubeb backend is 3x smaller than ALSA and PulseAudio.
Comment hidden (mozreview-request)
(Reporter)

Comment 2

2 years ago
Comment on attachment 8852071 [details]
Bug 1351378 - Expose sndio as an option for Linux and FreeBSD.

Landry, can you check for regressions?
Attachment #8852071 - Flags: feedback?(landry)
Comment hidden (obsolete)
Comment hidden (mozreview-request)
(Reporter)

Updated

2 years ago
Attachment #8852071 - Flags: feedback?(landry)
Comment on attachment 8852071 [details]
Bug 1351378 - Expose sndio as an option for Linux and FreeBSD.

Diff reads good to me, will have to test next week (hopefully) - dont wait for me !

Comment 6

2 years ago
mozreview-review
Comment on attachment 8852071 [details]
Bug 1351378 - Expose sndio as an option for Linux and FreeBSD.

https://reviewboard.mozilla.org/r/124310/#review127108

::: toolkit/moz.configure:86
(Diff revision 2)
>  set_config('MOZ_JACK', jack)
>  set_define('MOZ_JACK', jack)
>  
> +# SNDIO cubeb and audio_device backend
> +# ==============================================================
> +option('--enable-sndio', env='MOZ_SNDIO',

I'd rather add a --enable-audio-backends option that can take one or more of jack,alsa,pulseaudio,sndio (check out --enable-default-toolkit)

--enable-audio-backends=jack would imply --enable-jack, =alsa would imply --enable-alsa, etc.
(using imply_option())
Attachment #8852071 - Flags: review?(mh+mozilla)
Comment on attachment 8852071 [details]
Bug 1351378 - Expose sndio as an option for Linux and FreeBSD.

The proposed patch doesnt cause regressions on OpenBSD:

$grep MOZ_SNDIO /usr/obj/m-c/config.*                                                                  
/usr/obj/m-c/config.status:    'MOZ_SNDIO': '1',

- but the direction glandium suggested is probably better...
Attachment #8852071 - Flags: feedback?(landry) → feedback+
Fwiw, alex and i redid more or less the same patch since he wants to test sndio/cubeb on linux - mike, it's been 7 months, can we at least move forward on this ? paul, any opinion ? Since we have runtime cubeb switching now...
Flags: needinfo?(padenot)
Flags: needinfo?(mh+mozilla)
(Reporter)

Comment 9

2 years ago
mozreview-review
Comment on attachment 8852071 [details]
Bug 1351378 - Expose sndio as an option for Linux and FreeBSD.

https://reviewboard.mozilla.org/r/124310/#review199338

::: toolkit/moz.configure:86
(Diff revision 2)
>  set_config('MOZ_JACK', jack)
>  set_define('MOZ_JACK', jack)
>  
> +# SNDIO cubeb and audio_device backend
> +# ==============================================================
> +option('--enable-sndio', env='MOZ_SNDIO',

Creating --enable-audio-backends is not so simple. The current state is a mess:
- alsa[1] and sndio lack lazy bindings (e.g., to enable by default downstream)
- sndio doesn't play well with other backends under media/webrtc/
- alsa, jack, pulseaudio haven't been converted to moz.configure yet
- alsa and jack have broken full_duplex in libcubeb
- jack lacks a backend under media/webrtc/
- jack and pulseaudio code hasn't been tested on Android, OS X or Windows
- Android, OS X or Windows backends are unusable on (free)desktop Unix

[1] bug 1021761 has a patch
If multiple backends are not supported properly, then it's even more a reason for having *one* option, and not n, where you need to make all of them conflict with each other.
Flags: needinfo?(mh+mozilla)

Updated

a year ago
Product: Core → Firefox Build System
Comment hidden (mozreview-request)
(Reporter)

Comment 12

a year ago
Comment on attachment 8852071 [details]
Bug 1351378 - Expose sndio as an option for Linux and FreeBSD.

Rebased but still stalled due to scope creep requested by :glandium. moz.configure reminds me of Waf except for complete lack of documentation.

Comment 13

a year ago
mozreview-review
Comment on attachment 8852071 [details]
Bug 1351378 - Expose sndio as an option for Linux and FreeBSD.

https://reviewboard.mozilla.org/r/124310/#review240816

Comment 10 still applies.
Attachment #8852071 - Flags: review?(mh+mozilla)
Flags: needinfo?(padenot)
You need to log in before you can comment on or make changes to this bug.