907817 - Choose a right latency value depending on the platform

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Paul Adenot (:padenot)]

Now that we have (kind of) decent audio outputs backend for all platforms, we should make a more educated choice about what latency we request to the backend than "100ms".

The plan is to add a function to cubeb to get the possible latency range (upper and lower bound) for the chosen backend, and then use that to decide on what to do.

For some backends (PulseAudio, for example), the latency will adjust itself based on the CPU load and observed xruns. For some other backend, we would have to be more conservative, because it will glitch (we might a way to get notified, though, depending on the backend). 
On some platforms (I'm thinking of WASAPI, here), the docs say the hard limit for the lower bound is some fixed number of ms (30ms, but it would not be the first time MS docs are lying).

I'm thinking of keeping <audio> and <video> tags relatively high latency (the current 100ms seems fine), and request low latency for everything that comes from a MediaStream, so we get better latency on WebAudio, WebRTC, and the like, so we can save CPU.

Comment 1

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

"For some backends (PulseAudio, for example), the latency will adjust itself based on the CPU load and observed xruns."

This concerns me.... AEC demands that the delay be (relatively) constant and known.  If the backend is adjusting delay on the fly, you have to either a) reset the AEC and retrain (and lose AEC for a short period; typically half-duplex) or b) tell the AEC what the (delta in) delay is so it can adjust the internal matrix to offset it and try to save the training.  Typically this only works well for "jumps" in delay due to underrun/overrun causing drops or insertions of silence.

If delay changes occur without the AEC knowing, you will get temporary echo of some form until it can retrain.  How long that takes, I don't know, but I'd assume 0.5-2 seconds generally from the time "enough" audio is output.

A variable resampler would be a problem....

Comment 2

[Paul Adenot (:padenot)]

Let me rephrase:

"For some backends (PulseAudio, for example), the latency will adjust itself based on xruns, caused by high CPU load".

I could not observe change in latency during normal browser usage (even quite intensive browser usage). I observed this when using the stress command, spinning all core at 100%, doing intensive disk and memory transfer and such.

Comment 3

[Paul Adenot (:padenot)]

Also if this proves to be a problem for WebRTC, note that we can disabled this feature from PulseAudio 2.0 (released in may 2012).

Comment 4

[Paul Adenot (:padenot)]

Attachment: Actually set the latency when using the audiounit cubeb backend. r=

Comment 5

[Paul Adenot (:padenot)]

Attachment: Add a cubeb API to get a valid audio latency range per platform. r=

Comment 6

[Paul Adenot (:padenot)]

Attachment: Allow AudioStream users to pass-in latency requirements. r=

Comment 7

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

Comment on attachment 806769 [details] [diff] [review]
Add a cubeb API to get a valid audio latency range per platform. r=

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

Needs a line in symbols.def.in for cubeb_get)minimal_latency

Comment 8

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

I ran it and ran a quick gum test (debug build!) and another browser too.

I also added an PR_LOG statement to log the result: 10ms

Comment 9

[Paul Adenot (:padenot)]

Attachment: Actually set the latency when using the audiounit cubeb backend. r=

Comment 10

[Paul Adenot (:padenot)]

Attachment: Add a cubeb API to get a valid audio latency range per platform. r=

A breakdown per platform:
- Alsa/Pulse: I asked people, they told me that there were no reliable API to
get the number I wanted, and I should just use 40ms
- AudioUnit/WASAPI: straightforward
- OpenSL: this required some reading of AOSP code. We simply get the two symbols in
the libmedia.so library. The usual way to do this is to either use Java or JNI
to get the values, but I figured directly calling the method java calls would be
simpler. I checked this gives us correct numbers on Nexus 4 and Galaxy Nexus
(comparing my C implementation with what Java returns).
- WinMM: No idea how to get a number
- sndio: No idea how to get a number

Comment 11

[Paul Adenot (:padenot)]

Attachment: Allow AudioStream users to pass-in latency requirements. r=

This tells the mediastream to use try to be low-latency when using
the MediaStreamGraph. It does not change regular media playback.

Comment 12

[Matthew Gregan [:kinetik]]

Comment on attachment 810640 [details] [diff] [review]
Actually set the latency when using the audiounit cubeb backend. r=

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

::: media/libcubeb/src/cubeb_audiounit.c
@@ +308,5 @@
>      audiounit_stream_destroy(stm);
>      return CUBEB_ERROR;
>    }
>  
> +  buffer_size = latency / 1000. * ss.mSampleRate;

1000.0

@@ +339,5 @@
> +  if (r != 0) {
> +    audiounit_stream_destroy(stm);
> +    return CUBEB_ERROR;
> +  }
> +  if (buffer_size < (unsigned int)buffer_size_range.mMinimum) {

Space between ) and b

Comment 13

[Matthew Gregan [:kinetik]]

Comment on attachment 810653 [details] [diff] [review]
Allow AudioStream users to pass-in latency requirements. r=

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

r+ with pref issue fixed

::: content/media/AudioStream.cpp
@@ +567,5 @@
> +  uint32_t latency;
> +  if (aLatencyRequest == AudioStream::LowLatency) {
> +    cubeb_get_minimal_latency(cubebContext, &params, &latency);
> +  } else {
> +    latency = GetCubebLatency();

I'd prefer to keep the pref working for all cases, so change this so that the pref overrides everything else only if the pref is set explicitly.

Comment 14

[Matthew Gregan [:kinetik]]

Comment on attachment 810652 [details] [diff] [review]
Add a cubeb API to get a valid audio latency range per platform. r=

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

r+ with comments fixed

::: layout/media/symbols.def.in
@@ +114,5 @@
>  #ifdef MOZ_CUBEB
>  cubeb_destroy
>  cubeb_init
>  cubeb_get_max_channel_count
> +cubeb_get_minimal_latency

Call it get_min_latency

::: media/libcubeb/include/cubeb.h
@@ +188,5 @@
>      @retval CUBEB_ERROR */
>  int cubeb_get_max_channel_count(cubeb * context, uint32_t * max_channels);
>  
> +/** Get the minimal latency value, in milliseconds, that is guaranteed to work
> +    when creating a stream for a certain samplerate. This is platform and

"for a certain" -> "for the specified"

@@ +196,5 @@
> +                  the characteristics of the stream.
> +    @param latency The latency value, in ms, to pass to cubeb_stream_init.
> +    @retval CUBEB_ERROR_INVALID_PARAMETER
> +    @retval CUBEB_OK */
> +int cubeb_get_minimal_latency(cubeb * context, cubeb_stream_params * params, uint32_t * latency_ms);

Pass params by value.

::: media/libcubeb/src/cubeb_alsa.c
@@ +932,5 @@
> +static int
> +alsa_get_minimal_latency(cubeb * ctx, cubeb_stream_params * params, uint32_t * latency_ms)
> +{
> +  // This is found to be an acceptable minimum, even on a super low-end
> +  // machine.

C style comments.

::: media/libcubeb/src/cubeb_audiotrack.c
@@ +294,5 @@
> +audiotrack_get_minimal_latency(cubeb * ctx, cubeb_stream_params * params, uint32_t * latency_ms)
> +{
> +  // We always use the lowest latency possible when using this backend (see
> +  // audiotrack_stream_init), so this value is not going to be used.
> +  *latency_ms = 100;

We should still return the correct value.

::: media/libcubeb/src/cubeb_audiounit.c
@@ +154,5 @@
> +
> +  return CUBEB_OK;
> +}
> +
> +// Get the acceptable buffer size (in frames) that this device can work with.

C style comments

@@ +233,5 @@
> +static int
> +audiounit_get_minimal_latency(cubeb * ctx, cubeb_stream_params * params, uint32_t * latency_ms)
> +{
> +  AudioValueRange latency_range;
> +  audiounit_get_acceptable_latency_range(&latency_range);

Check return code?

::: media/libcubeb/src/cubeb_opensl.c
@@ +226,5 @@
> +static int
> +opensl_get_minimal_latency(cubeb * ctx, cubeb_stream_params * params, uint32_t * latency_ms)
> +{
> +  /* https://android.googlesource.com/platform/ndk.git/+/master/docs/opensles/index.html
> +   * We don't want to deal with JNI here (and we don't have Java on b2g anyways,

Missing closing ) in comment

@@ +232,5 @@
> +
> +  int rv;
> +  void * libmedia;
> +  uint32_t (*get_primary_output_samplingrate)();
> +  size_t (*get_primary_output_frame_count)();

(void) x2 here

@@ +260,5 @@
> +
> +  primary_sampling_rate = get_primary_output_samplingrate();
> +  primary_buffer_size = get_primary_output_frame_count();
> +
> +  // to trigger get a fast track in Android's mixer, we need to be at the native

"fast path"?

::: media/libcubeb/src/cubeb_sndio.c
@@ +260,5 @@
>  
> +static int
> +sndio_get_minimal_latency(cubeb * ctx, cubeb_stream_params * params, uint32_t * latency_ms)
> +{
> +  // XXX

"Not yet implemented"

::: media/libcubeb/src/cubeb_winmm.c
@@ +523,5 @@
>  
>  static int
> +winmm_get_minimal_latency(cubeb * ctx, cubeb_stream_params * params, uint32_t * latency)
> +{
> +  // 100ms minimum, if we are not in a bizarre configuration.

Just change calculate_minimum_latency to return 100 for the regular path, then return ctx->minimum_latency here.

Comment 15

[Paul Adenot (:padenot)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6c1e6cff2e43
https://hg.mozilla.org/integration/mozilla-inbound/rev/af8f1f9b11be
https://hg.mozilla.org/integration/mozilla-inbound/rev/b77402bffaf7

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6c1e6cff2e43
https://hg.mozilla.org/mozilla-central/rev/af8f1f9b11be
https://hg.mozilla.org/mozilla-central/rev/b77402bffaf7