918861 - Allow the MediaStreamGraph to run at different samplerates

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Paul Adenot (:padenot)]

It can be important to allow the MSG to use something different than the fixed 48000Hz we use right now:

- It can save a resampling pass: if the MSG runs at the same sampling rate as the system mixer/hardware/etc. This means lower latency and load. 
- It can trigger a fastpath on Android, when using OpenSL (an exact samplerate and the exact buffer size has to be provided on stream opening to get allocated a FastTrack in the mixer). I can go as low as 12.5ms (instead of the 100+ms/ Number are theoretical, based on samplerate and buffer sizes) this way, on my Galaxy Nexus.
- It can reduce resampling in AudioBufferSourceNodes: most of the media that go through decodeAudioData are at 44100Hz (which is not really surprising for audio-only files), not 48000Hz (which is usually for videos).

First, we need to be able to query the hardware/mixer/etc. to get the right samplerate, then pass this value to the MSG. I have most of the platforms done for the former, and I haven't tried the latter.

Comment 1

[Paul Adenot (:padenot)]

Attachment: Add an API to get the native samplerate for a given audio backend. r=

Breakdown by platform as well:
- WASAPI, easy, we just query the mixer format.
- alsa, it will not work if we are dealing with pulse with an Alsa
API. Asked around, people didn't know how to do it, so I picked 44100.
- WinMM, the mixer can decide to change the sampling rate, based on what it is
playing and the hardware, so I picked some reasonable value.
- Audiounit, we just query the hardware samplerate.
- pulse, we use the samplerate of the sink, to avoid resampling in the sink.
- OpenSL, same technique as for the latency, we get the value from the .so
- AudioTrack, we use an API
- sndio, no idea

Comment 2

[Paul Adenot (:padenot)]

Attachment: Expose a better samplerate though the AudioStream interface. r=

Since this can do IO (dlopen on a terrible flash memory), I put in a little
cache.

Comment 3

[Paul Adenot (:padenot)]

Attachment: Allow the MSG to choose a better samplerate than 48000Hz. r=

roc, this _seems_ to not bust WebAudio, anything in particular I should be
afraid of?

Comment 4

[Paul Adenot (:padenot)]

Attachment: Update tests that were relying on a 48000Hz samplerate. r=

This is what I had to update to have a variable samplerate MSG. I'll have to
adjust the fuzzing values again for test_mediaDecode.html, though.

Comment 5

[(no longer active)]

Comment on attachment 810662 [details] [diff] [review]
Update tests that were relying on a 48000Hz samplerate. r=

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

::: content/media/webaudio/test/test_mediaDecoding.html
@@ +213,5 @@
>    var result = true;
>    var difference = 0;
>    is(buf1.length, buf2.length, "same length");
>    for (var i = 0; i < buf1.length; ++i) {
> +    if (Math.abs(buf1[i] - buf2[i]) > 90) {

Why 90?

@@ +243,5 @@
> +  dump("SR: " + test.sampleRate + "\n");
> +  var SRCRate = test.sampleRate / cx.sampleRate;
> +  dump("SRCRate: " + SRCRate + "\n");
> +  dump("buffer.length " + buffer.length + "\n");
> +  dump("test.frames " + test.frames + "\n");

Please remove the dumps.

@@ +249,5 @@
>  
>    var wave = createWaveFileData(buffer);
> +//  var blob = new Blob([wave], {type: "application/octet-binary"});
> +//  var url = URL.createObjectURL(blob);
> +//  location.href = url;

And the commented code.

Comment 6

[Matthew Gregan [:kinetik]]

Comment on attachment 810657 [details] [diff] [review]
Expose a better samplerate though the AudioStream interface. r=

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

::: content/media/AudioStream.cpp
@@ +42,5 @@
>  static double gVolumeScale;
>  static uint32_t gCubebLatency;
>  
> +StaticMutex AudioStream::mMutex;
> +uint32_t AudioStream::mPreferredSamplerate = 0;

SampleRate

@@ +443,5 @@
> +  // often, and the complexity of the function call below depends on the
> +  // backend used platform.
> +  const int fallbackSamplerate = 44100;
> +  if (mPreferredSamplerate == 0) {
> +    if(cubeb_get_preferred_samplerate(GetCubebContext(), &mPreferredSamplerate) != CUBEB_OK) {

Space between if and (

::: media/libcubeb/src/cubeb_opensl.c
@@ +513,5 @@
>    .init = opensl_init,
>    .get_backend_id = opensl_get_backend_id,
>    .get_max_channel_count = opensl_get_max_channel_count,
>    .get_minimal_latency = opensl_get_minimal_latency,
> +  .get_preferred_samplerate = opensl_get_preferred_samplerate,

This should be in the cubeb patch.

Comment 7

[Matthew Gregan [:kinetik]]

Comment on attachment 810656 [details] [diff] [review]
Add an API to get the native samplerate for a given audio backend.  r=

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

r- due to the compile errors

::: media/libcubeb/include/cubeb.h
@@ +198,5 @@
>      @retval CUBEB_ERROR_INVALID_PARAMETER
>      @retval CUBEB_OK */
>  int cubeb_get_minimal_latency(cubeb * context, cubeb_stream_params * params, uint32_t * latency_ms);
>  
> +/** Get the preferred samplerate for this backed: this is hardware and platform

sample rate, backend

@@ +204,5 @@
> +   @param context
> +   @param samplerate The samplerate (in Hz) the current configuration prefers.
> +   @return CUBEB_ERROR_INVALID_PARAMETER
> +   @return CUBEB_OK */
> +int cubeb_get_preferred_samplerate(cubeb * context, uint32_t * samplerate);

sample_rate in the function name, rate for the parameter

::: media/libcubeb/src/cubeb_alsa.c
@@ +933,5 @@
> +alsa_get_preferred_samplerate(cubeb * ctx, uint32_t * samplerate)
> +{
> +  int rv, dir;
> +  snd_pcm_t *pcm;
> +  snd_pcm_hw_params_t* hw_params;

Spaces around *.

@@ +966,5 @@
> +    snd_pcm_close(pcm);
> +    return CUBEB_ERROR;
> +  }
> +
> +  rv = snd_pcm_close(pcm);

Either check rv, or don't store the return value if you're going to ignore it.

::: media/libcubeb/src/cubeb_audiounit.c
@@ +236,5 @@
>    AudioValueRange latency_range;
>    audiounit_get_acceptable_latency_range(&latency_range);
>  
>    *latency_ms = latency_range.mMinimum * 1000 / params->rate;
> +static int

Looks like this was inserted in the middle of get_minimal_latency?

::: media/libcubeb/src/cubeb_opensl.c
@@ +513,5 @@
>    .init = opensl_init,
>    .get_backend_id = opensl_get_backend_id,
>    .get_max_channel_count = opensl_get_max_channel_count,
>    .get_minimal_latency = opensl_get_minimal_latency,
> +  .get_preferred_sampelrate = opensl_get_preferred_samplerate,

sample

::: media/libcubeb/src/cubeb_sndio.c
@@ +269,5 @@
>  
> +static int
> +sndio_get_preferred_samplerate(cubeb * ctx, uint32_t * samplerate)
> +{
> +  // XXX

"Not yet implemented", just to be clearer

::: media/libcubeb/src/cubeb_winmm.c
@@ +532,5 @@
>  
>  static int
> +winmm_get_preferred_samplerate(cubeb * ctx, uint32_t * samplerate)
> +{
> +  *samplerate = 44100;

It should be possible to query this via waveOutGetDevCaps?

Comment 8

[(no longer active)]

Comment on attachment 810662 [details] [diff] [review]
Update tests that were relying on a 48000Hz samplerate. r=

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

Clearing the request pending an answer to the question about 90.

Comment 9

[Paul Adenot (:padenot)]

Attachment: Update tests that were relying on a 48000Hz samplerate. r=

The "> 90" bit was just some debugging code, the test passes without.

I also addressed the other two comments.

Comment 10

[Paul Adenot (:padenot)]

Attachment: Add an API to get the native samplerate for a given audio backend. r=

This is green on try, now. Sorry about the other patch.

Comment 11

[Matthew Gregan [:kinetik]]

Comment on attachment 815418 [details] [diff] [review]
Add an API to get the native samplerate for a given audio backend.  r=

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

Thanks!

Sorry about the delay reviewing this, the r? email got lost in my mail.

Comment 12

[Paul Adenot (:padenot)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7049d17385d9
https://hg.mozilla.org/integration/mozilla-inbound/rev/ac461ea5f961
https://hg.mozilla.org/integration/mozilla-inbound/rev/8c159c637fdb
https://hg.mozilla.org/integration/mozilla-inbound/rev/c988007d5cd1

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7049d17385d9
https://hg.mozilla.org/mozilla-central/rev/ac461ea5f961
https://hg.mozilla.org/mozilla-central/rev/8c159c637fdb
https://hg.mozilla.org/mozilla-central/rev/c988007d5cd1

Comment 14

[Ioana (away)]

This fix seem to be covered by the tests here:
https://hg.mozilla.org/mozilla-central/rev/c988007d5cd1

Anything else needing testing here?