942657 - Devirtualize AudioStream (and remove MOZ_CUBEB configure option)

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Matthew Gregan [:kinetik]]

We only have a single AudioStream implementation now, but that implementation is spread between the AudioStream abstract class and the BufferedAudioStream concrete class.  I've got a patch that flattens the hierarchy down to a single AudioStream class.

Comment 1

[Matthew Gregan [:kinetik]]

Attachment: bug942657_v0.patch

https://tbpl.mozilla.org/?tree=Try&rev=6f5b4641866a

Comment 2

[Matthew Gregan [:kinetik]]

Retry with a non-broken base: https://tbpl.mozilla.org/?tree=Try&rev=9667a5b7461c

Comment 3

[Matthew Gregan [:kinetik]]

Another attempt: https://tbpl.mozilla.org/?tree=Try&rev=ed19bb93af0b

Comment 4

[Matthew Gregan [:kinetik]]

Attachment: bug942657_v1.patch

As pushed to try.  Main difference in this version is the removal of MOZ_CUBEB, since it's messy to make AudioStream optional when it's non-virtual without littering the rest of the code with MOZ_CUBEB ifdefs and I don't think it makes sense to allow it to be disabled anyway (individual libcubeb backends can be disabled).

Comment 5

[Matthew Gregan [:kinetik]]

Apparently that broken some WebRTC stuff... https://tbpl.mozilla.org/?tree=Try&rev=f0d527368fbc

Comment 6

[Matthew Gregan [:kinetik]]

Attachment: bug942657_v2.patch

https://tbpl.mozilla.org/?tree=Try&rev=7f405148fe20

Comment 7

[Matthew Gregan [:kinetik]]

Attachment: bug942657_v3.patch

Minor fixes vs try version.

r? gps for build changes (removing MOZ_CUBEB)
r? doublec for everything else.

Comment 8

[Gregory Szorc [:gps]]

Comment on attachment 8339752 [details] [diff] [review]
bug942657_v3.patch

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

r+ covers just the build bits: I can't speak for the sanity of the rest of the patch.

::: configure.in
@@ +5546,5 @@
>  dnl = Check alsa availability on Linux if using libcubeb
>  dnl ====================================================
>  
>  dnl If using libcubeb with Linux, ensure that the alsa library is available
> +if test "$OS_TARGET" = "Linux"; then

The comment above needs updating.

@@ +5570,5 @@
>  dnl = Disable PulseAudio
>  dnl ========================================================
>  
>  dnl If using libcubeb with Linux, ensure that the PA library is available
> +if test "$OS_TARGET" = "Linux" -a -z "$MOZ_B2G"; then

Comment.

Comment 9

[Matthew Gregan [:kinetik]]

Thanks! Landed with configure.in comments fixed:

https://hg.mozilla.org/integration/mozilla-inbound/rev/dc9ebdf27e98

Comment 10

[:Benjamin Peterson]

Backed out for timeouts

https://tbpl.mozilla.org/php/getParsedLog.php?id=31243845&tree=Mozilla-Inbound

https://hg.mozilla.org/integration/mozilla-inbound/rev/cf8bda6419f4

Comment 11

[Matthew Gregan [:kinetik]]

Oops, reintroduced a deadlock.  Simple fix, pushed again:

https://hg.mozilla.org/integration/mozilla-inbound/rev/6c15f3df605a

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6c15f3df605a