Closed Bug 1387454 Opened 7 years ago Closed 6 years ago

Have multiple MediaStreamGraphs per process, one per audio sampling rate

Categories

(Core :: Audio/Video: MediaStreamGraph, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: achronop, Assigned: achronop)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete)

Attachments

(9 files)

Allow to have different instances of MSG running on different threads based on the sample rate. For every different sample rate a new instance of MSG will be created and will be driven by a different instance of AudioCallbackDriver. The cubeb stream will be initialized with the current MSG sample rate.

On the top of that the AudioCotext constructor will be enhanced to accept sample rate as an input. This provides the opportunity to instantiate an AudioContext with the desired sample rate instead of the default. Moreover two or more AudioContext with different sample rate on the same doc is possible. Communication between AudioContext with different sample rate is not implemented yet (it will be the subject of a following bug).
Assignee: nobody → achronop
Rank: 15
OS: Unspecified → All
Priority: -- → P1
Hardware: Unspecified → All
See Also: → 1330360
Attachment #8893852 - Flags: feedback?(padenot)
Attached patch dom-options WIPSplinter Review
Attachment #8893853 - Flags: feedback?(padenot)
Attachment #8893854 - Flags: feedback?(padenot)
Attachment #8893852 - Attachment is patch: true
Attachment #8893853 - Attachment is patch: true
Attachment #8893854 - Attachment is patch: true
Attachment #8893852 - Flags: feedback?(padenot) → feedback+
Attachment #8893853 - Flags: feedback?(padenot) → feedback+
Comment on attachment 8893854 [details] [diff] [review]
samplerate-on-msg WIP

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

Looks like it would work yeah.

::: dom/locales/en-US/chrome/dom/dom.properties
@@ +108,5 @@
>  # LOCALIZATION NOTE: Do not translate MediaStream and createMediaStreamSource.
>  MediaStreamAudioSourceNodeCrossOrigin=The MediaStream passed to createMediaStreamSource has a cross-origin resource, the node will output silence.
>  MediaLoadExhaustedCandidates=All candidate resources failed to load. Media load paused.
>  MediaLoadSourceMissingSrc=<source> element has no “src” attribute. Media resource load failed.
> +MediaStreamAudioSourceNodexWrongRate=Connect nodes with different sample rate.

We'll have to fix this sentence.

::: dom/media/MediaStreamGraph.cpp
@@ +3532,5 @@
>    // to the gloabl MediaStreamGraph hashtable. Effectively, this means there is
>    // a graph per document and AudioChannel.
>  
> +  TrackRate sampleRate = aSampleRate ? aSampleRate : CubebUtils::PreferredSampleRate();
> +  uint32_t hashkey = ChannelAndWindowToHash(aChannel, aWindow, sampleRate);

We'll have to rename this to be more generic.

::: dom/media/webaudio/MediaStreamAudioSourceNode.cpp
@@ +57,5 @@
>    }
>  
> +  if (aAudioContext.Graph() != aOptions.mMediaStream->GetPlaybackStream()->Graph()) {
> +    nsCOMPtr<nsPIDOMWindowInner> pWindow = aAudioContext.GetParentObject();
> +    nsIDocument* document = pWindow ? pWindow->GetExtantDoc() : nullptr;

I think we have a thing in webaudioutils.cpp that does all this.

@@ +62,5 @@
> +    nsContentUtils::ReportToConsole(nsIScriptError::warningFlag,
> +                                     NS_LITERAL_CSTRING("Media"),
> +                                     document,
> +                                     nsContentUtils::eDOM_PROPERTIES,
> +                                     "MediaStreamAudioSourceNodexWrongRate");

why the x here?
Attachment #8893854 - Flags: feedback?(padenot) → feedback+
(In reply to Paul Adenot (:padenot) from comment #4)
> Comment on attachment 8893854 [details] [diff] [review]
> samplerate-on-msg WIP
> 
> Review of attachment 8893854 [details] [diff] [review]:
> -----------------------------------------------------------------
> why the x here?

It's a typo
This is an assigned P1 bug without activity in two weeks. 

If you intend to continue working on this bug for the current release/iteration/sprint, remove the 'stale-bug' keyword.

Otherwise we'll reset the priority of the bug back to '--' on Monday, August 28th.
Keywords: stale-bug
Keywords: stale-bug
Summary: Multi-threaded MediaStreamGraph according to sample rate → Have multiple MediaStreamGraphs per process, one per audio sampling rate
Mass change P1->P2 to align with new Mozilla triage process
Priority: P1 → P2
Depends on: multimsg
Blocks: multimsg
No longer depends on: multimsg
Are there any news on the resolution of this issue? Also, could someone shed some light on how exactly this issue blocks bug 934425?
(In reply to bpantazhs from comment #8)
> Are there any news on the resolution of this issue? Also, could someone shed
> some light on how exactly this issue blocks bug 934425?

Different outputs mean different output clocks.  Since we're output-driven, each output must have it's own graph -- and the fun part is dealing with data that crosses graphs (input clocked in sync with output A, run through webaudio (or not) and then routed to media element with an output to output B.  The issue is that even if they're the same frequency, you need buffers (due to out-of-sync/size pulls) and to handle clock drift between A and B.
(In reply to Randell Jesup [:jesup] from comment #9)

Thank you for the reply. Is there any documentation available for this?
Flags: needinfo?(jib)
See https://webaudio.github.io/web-audio-api/#dom-audiocontextoptions-samplerate

(In reply to Randell Jesup [:jesup] from comment #9)
>  -- and the fun part is dealing with data that crosses graphs

Note that that is outside the scope of this bug, as noted in comment 0. We plan to land a version that throws in those cases for now.
Flags: needinfo?(jib)
Could you please review and test fiddle in the comment above. I would like to verify that it covers the basic functionality and I am not missing something.
Flags: needinfo?(padenot)
Comment on attachment 8950164 [details]
Bug 1387454 - Add AudioContextOptions in webidl.

https://reviewboard.mozilla.org/r/219426/#review225586
Attachment #8950164 - Flags: review?(padenot) → review+
Attachment #8950164 - Flags: review?(bugs)
Comment on attachment 8950164 [details]
Bug 1387454 - Add AudioContextOptions in webidl.

https://reviewboard.mozilla.org/r/219426/#review225602
Attachment #8950164 - Flags: review?(bugs) → review+
Comment on attachment 8950165 [details]
Bug 1387454 - Set sample rate in AudioContext constructor.

https://reviewboard.mozilla.org/r/219428/#review225588

::: dom/media/webaudio/AudioContext.cpp:208
(Diff revision 1)
>    nsCOMPtr<nsPIDOMWindowInner> window = do_QueryInterface(aGlobal.GetAsSupports());
>    if (!window) {
>      aRv.Throw(NS_ERROR_FAILURE);
>      return nullptr;
>    }
>  

Here, limit the rate to the sample-rate range that are valid: https://searchfox.org/mozilla-central/source/dom/media/webaudio/WebAudioUtils.h#38-39

And add a test for that.

::: dom/media/webaudio/AudioContext.cpp:212
(Diff revision 1)
>    }
>  
>    uint32_t maxChannelCount = std::min<uint32_t>(WebAudioUtils::MaxChannelCount,
>        CubebUtils::MaxNumberOfChannels());
>    RefPtr<AudioContext> object =
> -    new AudioContext(window, false,maxChannelCount);
> +    new AudioContext(window, false,maxChannelCount,

space after `false,`.
Attachment #8950165 - Flags: review?(padenot)
Comment on attachment 8950167 [details]
Bug 1387454 - Mochitests for non default rate on webaudio and media recorder.

https://reviewboard.mozilla.org/r/219432/#review225584

::: dom/media/tests/mochitest/mochitest.ini:326
(Diff revision 1)
>  [test_peerConnection_sender_and_receiver_stats.html]
>  skip-if = (android_version == '18') # android(Bug 1189784, timeouts on 4.3 emulator)
>  [test_peerConnection_verifyDescriptions.html]
>  skip-if = (android_version == '18')
>  [test_fingerprinting_resistance.html]
> +[test_audioContextParams_sampleRate.html]

I want tests for:
- Connecting a stream from gUM to a graph with a non-default rate
- Connecting a stream from a PC to a graph with non default rate
- Connecting a graph with non-default rate to a MediaRecorder
- Connecting a graph with non-default rate to a PeerConnection

... that kind of things.
Attachment #8950167 - Flags: review?(padenot)
Comment on attachment 8950166 [details]
Bug 1387454 - Create a MediaStreamGraph according to the given sample rate.

https://reviewboard.mozilla.org/r/219430/#review225580

::: dom/locales/en-US/chrome/dom/dom.properties:107
(Diff revision 1)
>  MediaElementAudioSourceNodeCrossOrigin=The HTMLMediaElement passed to createMediaElementSource has a cross-origin resource, the node will output silence.
>  # LOCALIZATION NOTE: Do not translate MediaStream and createMediaStreamSource.
>  MediaStreamAudioSourceNodeCrossOrigin=The MediaStream passed to createMediaStreamSource has a cross-origin resource, the node will output silence.
>  MediaLoadExhaustedCandidates=All candidate resources failed to load. Media load paused.
>  MediaLoadSourceMissingSrc=<source> element has no “src” attribute. Media resource load failed.
> +MediaStreamAudioSourceNodexWrongRate=Connect nodes with different sample rate.

"Connecting AudioNodes from AudioContexts with different sample-rate is currently not supported."

::: dom/locales/en-US/chrome/dom/dom.properties:107
(Diff revision 1)
>  MediaElementAudioSourceNodeCrossOrigin=The HTMLMediaElement passed to createMediaElementSource has a cross-origin resource, the node will output silence.
>  # LOCALIZATION NOTE: Do not translate MediaStream and createMediaStreamSource.
>  MediaStreamAudioSourceNodeCrossOrigin=The MediaStream passed to createMediaStreamSource has a cross-origin resource, the node will output silence.
>  MediaLoadExhaustedCandidates=All candidate resources failed to load. Media load paused.
>  MediaLoadSourceMissingSrc=<source> element has no “src” attribute. Media resource load failed.
> +MediaStreamAudioSourceNodexWrongRate=Connect nodes with different sample rate.

s/xWrong/Different/

::: dom/media/webaudio/AudioDestinationNode.cpp:341
(Diff revision 1)
>    nsPIDOMWindowInner* window = aContext->GetParentObject();
>    MediaStreamGraph* graph =
>      aIsOffline
>        ? MediaStreamGraph::CreateNonRealtimeInstance(aSampleRate, window)
>        : MediaStreamGraph::GetInstance(
> -          MediaStreamGraph::AUDIO_THREAD_DRIVER, window);
> +          MediaStreamGraph::AUDIO_THREAD_DRIVER, window, aSampleRate);

Probably not needed.

::: dom/media/webaudio/MediaStreamAudioSourceNode.cpp:61
(Diff revision 1)
>  
>    if (aAudioContext.CheckClosed(aRv)) {
>      return nullptr;
>    }
>  
> +  if (aAudioContext.Graph() != aOptions.mMediaStream->GetPlaybackStream()->Graph()) {

We want to audit our code, at each call site for `MediaStreamGraph::GetInstance`, and determine if it's going to do the right thing.

::: dom/media/webaudio/MediaStreamAudioSourceNode.cpp:68
(Diff revision 1)
> +    nsIDocument* document = pWindow ? pWindow->GetExtantDoc() : nullptr;
> +    nsContentUtils::ReportToConsole(nsIScriptError::warningFlag,
> +                                     NS_LITERAL_CSTRING("Media"),
> +                                     document,
> +                                     nsContentUtils::eDOM_PROPERTIES,
> +                                     "MediaStreamAudioSourceNodexWrongRate");

s/xWrong/Different/
Attachment #8950166 - Flags: review?(padenot)
Comment on attachment 8950166 [details]
Bug 1387454 - Create a MediaStreamGraph according to the given sample rate.

https://reviewboard.mozilla.org/r/219430/#review225580

> Probably not needed.

Why not? How can we pass sample rate in MSG wihtout it?
Comment on attachment 8950166 [details]
Bug 1387454 - Create a MediaStreamGraph according to the given sample rate.

https://reviewboard.mozilla.org/r/219430/#review225580

> We want to audit our code, at each call site for `MediaStreamGraph::GetInstance`, and determine if it's going to do the right thing.

That's the list:

dom/html/HTMLMediaElement.cpp
3720	MediaStreamGraph::GetInstance(graphDriverType, window); // found in mozilla::dom::HTMLMediaElement::MozCaptureStream
3753	MediaStreamGraph::GetInstance(graphDriverType, window); // found in mozilla::dom::HTMLMediaElement::MozCaptureStreamUntilEnded
7576	MediaStreamGraph::GetInstance(MediaStreamGraph::AUDIO_THREAD_DRIVER, window); // found in mozilla::dom::HTMLMediaElement::AudioCaptureStreamChange

dom/media/CanvasCaptureMediaStream.cpp
290	MediaStreamGraph::GetInstance(MediaStreamGraph::SYSTEM_THREAD_DRIVER, aWindow); // found in mozilla::dom::CanvasCaptureMediaStream::CreateSourceStream

dom/media/DOMMediaStream.cpp
562	MediaStreamGraph::GetInstance(MediaStreamGraph::SYSTEM_THREAD_DRIVER, ownerWindow); // found in mozilla::DOMMediaStream::Constructor

dom/media/MediaManager.cpp
1188	MediaStreamGraph::GetInstance(graphDriverType, window); // found in mozilla::GetUserMediaStreamRunnable::Run
4154	MediaStreamGraph::GetInstance(MediaStreamGraph::AUDIO_THREAD_DRIVER, window); // found in mozilla::SourceListener::StopSharing

dom/media/webaudio/AudioDestinationNode.cpp
340	: MediaStreamGraph::GetInstance( // found in mozilla::dom::AudioDestinationNode::AudioDestinationNode

media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
2378	MediaStreamGraph* graph = MediaStreamGraph::GetInstance( // found in mozilla::PeerConnectionImpl::CreateReceiveTrack
Comment on attachment 8950166 [details]
Bug 1387454 - Create a MediaStreamGraph according to the given sample rate.

https://reviewboard.mozilla.org/r/219430/#review225990
Comment on attachment 8950166 [details]
Bug 1387454 - Create a MediaStreamGraph according to the given sample rate.

https://reviewboard.mozilla.org/r/219430/#review225996
Comment on attachment 8950166 [details]
Bug 1387454 - Create a MediaStreamGraph according to the given sample rate.

https://reviewboard.mozilla.org/r/219430/#review225580

> That's the list:
> 
> dom/html/HTMLMediaElement.cpp
> 3720	MediaStreamGraph::GetInstance(graphDriverType, window); // found in mozilla::dom::HTMLMediaElement::MozCaptureStream
> 3753	MediaStreamGraph::GetInstance(graphDriverType, window); // found in mozilla::dom::HTMLMediaElement::MozCaptureStreamUntilEnded
> 7576	MediaStreamGraph::GetInstance(MediaStreamGraph::AUDIO_THREAD_DRIVER, window); // found in mozilla::dom::HTMLMediaElement::AudioCaptureStreamChange
> 
> dom/media/CanvasCaptureMediaStream.cpp
> 290	MediaStreamGraph::GetInstance(MediaStreamGraph::SYSTEM_THREAD_DRIVER, aWindow); // found in mozilla::dom::CanvasCaptureMediaStream::CreateSourceStream
> 
> dom/media/DOMMediaStream.cpp
> 562	MediaStreamGraph::GetInstance(MediaStreamGraph::SYSTEM_THREAD_DRIVER, ownerWindow); // found in mozilla::DOMMediaStream::Constructor
> 
> dom/media/MediaManager.cpp
> 1188	MediaStreamGraph::GetInstance(graphDriverType, window); // found in mozilla::GetUserMediaStreamRunnable::Run
> 4154	MediaStreamGraph::GetInstance(MediaStreamGraph::AUDIO_THREAD_DRIVER, window); // found in mozilla::SourceListener::StopSharing
> 
> dom/media/webaudio/AudioDestinationNode.cpp
> 340	: MediaStreamGraph::GetInstance( // found in mozilla::dom::AudioDestinationNode::AudioDestinationNode
> 
> media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
> 2378	MediaStreamGraph* graph = MediaStreamGraph::GetInstance( // found in mozilla::PeerConnectionImpl::CreateReceiveTrack

About dom/html/HTMLMediaElement.cpp

3720    MediaStreamGraph::GetInstance(graphDriverType, window); // found in mozilla::dom::HTMLMediaElement::MozCaptureStream
3753    MediaStreamGraph::GetInstance(graphDriverType, window); // found in mozilla::dom::HTMLMediaElement::MozCaptureStreamUntilEnded

We can pair these 2 cases which have similar code flow. This does repro when we capture a stream from MediaElement. I could see a benefit to set the sample rate. If the playback stream for example is 16KHz by creating a MSG at that rate we avoid the extra resampling. Otherwise we pay the cost for resampling as we do now. If we decide to update it the biggest chalenge is that the sample rate is availlable (through `HTMLMediaElement::mMediaInfo::mAudio::mRate`) only after metadata loaded event (`HTMLMediaElement::MetadataLoaded()`), so if we get a stream capture request before that we do not really know the sample rate. In that case we should implement some special handling to wait for the event, then create the MSG and do the rest of the capture.

7576    MediaStreamGraph::GetInstance(MediaStreamGraph::AUDIO_THREAD_DRIVER, window); // found in mozilla::dom::HTMLMediaElement::AudioCaptureStreamChange

We reach that method when we do a gUM request using `mediaSource: "audioCapture"` constraints (and we have enabled a pref). Before that MSG has already been created in MediaManager (MediaManager.cpp:1188). So we need to decide first what we will do in MediaManager case and then to make sure that we call `GetInstance()` method here using the same arguments in order to get the already created MSG.
Comment on attachment 8950166 [details]
Bug 1387454 - Create a MediaStreamGraph according to the given sample rate.

https://reviewboard.mozilla.org/r/219430/#review225580

> About dom/html/HTMLMediaElement.cpp
> 
> 3720    MediaStreamGraph::GetInstance(graphDriverType, window); // found in mozilla::dom::HTMLMediaElement::MozCaptureStream
> 3753    MediaStreamGraph::GetInstance(graphDriverType, window); // found in mozilla::dom::HTMLMediaElement::MozCaptureStreamUntilEnded
> 
> We can pair these 2 cases which have similar code flow. This does repro when we capture a stream from MediaElement. I could see a benefit to set the sample rate. If the playback stream for example is 16KHz by creating a MSG at that rate we avoid the extra resampling. Otherwise we pay the cost for resampling as we do now. If we decide to update it the biggest chalenge is that the sample rate is availlable (through `HTMLMediaElement::mMediaInfo::mAudio::mRate`) only after metadata loaded event (`HTMLMediaElement::MetadataLoaded()`), so if we get a stream capture request before that we do not really know the sample rate. In that case we should implement some special handling to wait for the event, then create the MSG and do the rest of the capture.
> 
> 7576    MediaStreamGraph::GetInstance(MediaStreamGraph::AUDIO_THREAD_DRIVER, window); // found in mozilla::dom::HTMLMediaElement::AudioCaptureStreamChange
> 
> We reach that method when we do a gUM request using `mediaSource: "audioCapture"` constraints (and we have enabled a pref). Before that MSG has already been created in MediaManager (MediaManager.cpp:1188). So we need to decide first what we will do in MediaManager case and then to make sure that we call `GetInstance()` method here using the same arguments in order to get the already created MSG.

dom/media/CanvasCaptureMediaStream.cpp
290 MediaStreamGraph::GetInstance(MediaStreamGraph::SYSTEM_THREAD_DRIVER, aWindow); // found in mozilla::dom::CanvasCaptureMediaStream::CreateSourceStream

`CreateSourceStream` is a static method that comes from JS when we call `captureStream()` on a cavas HTML element. There are not many things we can do here. The sample rate is not available. Probably we have to stick with the deafault (preferred device rate).

dom/media/DOMMediaStream.cpp
562 MediaStreamGraph::GetInstance(MediaStreamGraph::SYSTEM_THREAD_DRIVER, ownerWindow); // found in mozilla::DOMMediaStream::Constructor

This is called when we create a new `MediaStream` object in JS. There are 3 constructors:
```
let s = new MediaStream()
let s = new MedisStream(stream);
let s = new MediaStream(track[]);
```
For the first constructor without arguments there are not many things we can do. The case is similar to the previous one. 

For the other two cases the code makes use of the existing graph from the provided stream/tracks (DOMMediaStream.cpp#554) and the `GetInstance()` method is never called.
Comment on attachment 8950166 [details]
Bug 1387454 - Create a MediaStreamGraph according to the given sample rate.

https://reviewboard.mozilla.org/r/219430/#review225580

> dom/media/CanvasCaptureMediaStream.cpp
> 290 MediaStreamGraph::GetInstance(MediaStreamGraph::SYSTEM_THREAD_DRIVER, aWindow); // found in mozilla::dom::CanvasCaptureMediaStream::CreateSourceStream
> 
> `CreateSourceStream` is a static method that comes from JS when we call `captureStream()` on a cavas HTML element. There are not many things we can do here. The sample rate is not available. Probably we have to stick with the deafault (preferred device rate).
> 
> dom/media/DOMMediaStream.cpp
> 562 MediaStreamGraph::GetInstance(MediaStreamGraph::SYSTEM_THREAD_DRIVER, ownerWindow); // found in mozilla::DOMMediaStream::Constructor
> 
> This is called when we create a new `MediaStream` object in JS. There are 3 constructors:
> ```
> let s = new MediaStream()
> let s = new MedisStream(stream);
> let s = new MediaStream(track[]);
> ```
> For the first constructor without arguments there are not many things we can do. The case is similar to the previous one. 
> 
> For the other two cases the code makes use of the existing graph from the provided stream/tracks (DOMMediaStream.cpp#554) and the `GetInstance()` method is never called.

dom/media/MediaManager.cpp
1188    MediaStreamGraph::GetInstance(graphDriverType, window); // found in mozilla::GetUserMediaStreamRunnable::Run

This is where the MSG is created in a basic gUM request. The most resonable thing to do here would be to use tha sample rate from the constarint but it's not currently supported. Other than that we do not have information about sample rate at that level.

4154    MediaStreamGraph::GetInstance(MediaStreamGraph::AUDIO_THREAD_DRIVER, window); // found in mozilla::SourceListener::StopSharing

This is called at stop of AudioCapture. Since we stop an operating stream the MSG is up and running. We could get that instance from the stream (`mStream::Graph()`) and stop calling the `MediaStreamGraph::GetInstance()` method.
Comment on attachment 8950167 [details]
Bug 1387454 - Mochitests for non default rate on webaudio and media recorder.

https://reviewboard.mozilla.org/r/219432/#review225584

> I want tests for:
> - Connecting a stream from gUM to a graph with a non-default rate
> - Connecting a stream from a PC to a graph with non default rate
> - Connecting a graph with non-default rate to a MediaRecorder
> - Connecting a graph with non-default rate to a PeerConnection
> 
> ... that kind of things.

gUM and PeerConnection make use of the default rate. In all cases above we will fail to connect graphs of different sample rate. we can veiry though that we fail nicesely. Is that your intention here?
yes.
Flags: needinfo?(padenot)
New manual tests according to the comments: https://fiddle.jshell.net/achronop/dwu7u30q/
Depends on: 1447097
Comment on attachment 8963930 [details]
Bug 1387454 - Mochitests for interaction of non default rate with WebRTC.

https://reviewboard.mozilla.org/r/232766/#review238770
Attachment #8963930 - Flags: review?(padenot) → review+
Comment on attachment 8950167 [details]
Bug 1387454 - Mochitests for non default rate on webaudio and media recorder.

https://reviewboard.mozilla.org/r/219432/#review238772
Attachment #8950167 - Flags: review?(padenot) → review+
Comment on attachment 8950166 [details]
Bug 1387454 - Create a MediaStreamGraph according to the given sample rate.

https://reviewboard.mozilla.org/r/219430/#review238778

::: dom/media/MediaStreamGraph.h:1301
(Diff revision 2)
>    };
>    static const uint32_t AUDIO_CALLBACK_DRIVER_SHUTDOWN_TIMEOUT = 20*1000;
>  
>    // Main thread only
> -  static MediaStreamGraph* GetInstanceIfExists(nsPIDOMWindowInner* aWindow);
> +  static MediaStreamGraph* GetInstanceIfExists(nsPIDOMWindowInner* aWindow,
> +                                               TrackRate aSampleRate = 0);

Remove the default argument. Create a constant that is equal to zero, means "default rate" and change all call sites so tht it's explicit everywhere and we don't make mistakes.

::: dom/media/webaudio/MediaStreamAudioSourceNode.cpp:65
(Diff revision 2)
>  
> +  if (aAudioContext.Graph() != aOptions.mMediaStream->GetPlaybackStream()->Graph()) {
> +    nsCOMPtr<nsPIDOMWindowInner> pWindow = aAudioContext.GetParentObject();
> +    nsIDocument* document = pWindow ? pWindow->GetExtantDoc() : nullptr;
> +    nsContentUtils::ReportToConsole(nsIScriptError::warningFlag,
> +                                     NS_LITERAL_CSTRING("Media"),

indent is off.
Attachment #8950166 - Flags: review?(padenot)
Comment on attachment 8950165 [details]
Bug 1387454 - Set sample rate in AudioContext constructor.

https://reviewboard.mozilla.org/r/219428/#review238768

::: dom/media/webaudio/AudioContext.cpp:212
(Diff revision 2)
>    }
>  
> +  if (aOptions.mSampleRate > 0 &&
> +      (aOptions.mSampleRate - WebAudioUtils::MinSampleRate < 0.0 ||
> +      WebAudioUtils::MaxSampleRate - aOptions.mSampleRate < 0.0)) {
> +    aRv.Throw(NS_ERROR_INVALID_ARG);

https://webaudio.github.io/web-audio-api/#dom-audiocontextoptions-samplerate says NotSupportedError.
Attachment #8950165 - Flags: review?(padenot)
Comment on attachment 8950165 [details]
Bug 1387454 - Set sample rate in AudioContext constructor.

https://reviewboard.mozilla.org/r/219428/#review239174

::: dom/media/webaudio/AudioContext.cpp:211
(Diff revision 3)
>      return nullptr;
>    }
>  
> +  if (aOptions.mSampleRate > 0 &&
> +      (aOptions.mSampleRate - WebAudioUtils::MinSampleRate < 0.0 ||
> +      WebAudioUtils::MaxSampleRate - aOptions.mSampleRate < 0.0)) {

Put this behind a pref for now. We need more testing before enabling it on the web, but we want to use this in mochitests.
Attachment #8950165 - Flags: review?(padenot)
Comment on attachment 8950166 [details]
Bug 1387454 - Create a MediaStreamGraph according to the given sample rate.

https://reviewboard.mozilla.org/r/219430/#review239176
Attachment #8950166 - Flags: review?(padenot) → review+
Comment on attachment 8950165 [details]
Bug 1387454 - Set sample rate in AudioContext constructor.

https://reviewboard.mozilla.org/r/219428/#review239178

Of course we'll need to adjust the tests as well.
Comment on attachment 8965284 [details]
Bug 1387454 - Create preference to enable setting of sample rate in AudioContext.

https://reviewboard.mozilla.org/r/234014/#review239718

::: dom/media/tests/mochitest/head.js:424
(Diff revision 1)
>        // If loopback devices are set they will be chosen instead of fakes in gecko.
>        ['media.navigator.streams.fake', WANT_FAKE_AUDIO || WANT_FAKE_VIDEO],
>        ['media.getusermedia.screensharing.enabled', true],
>        ['media.getusermedia.audiocapture.enabled', true],
> -      ['media.recorder.audio_node.enabled', true]
> +      ['media.recorder.audio_node.enabled', true],
> +      ['webaudio.samplerate.enable', true]

"media.webaudio.audiocontextoptions-samplerate.enabled" would be more in line with how we name our prefs.

::: dom/media/webaudio/AudioContext.cpp:210
(Diff revision 1)
>    if (!window) {
>      aRv.Throw(NS_ERROR_FAILURE);
>      return nullptr;
>    }
>  
> +  float sampleRate = 0.0;

Name a constant that is worth 0.0 and use it instead of this hardcoded 0.0.

We want to convey the meaning that 0.0 means "system defaults".

::: modules/libpref/init/all.js:669
(Diff revision 1)
>  pref("media.audioipc.stack_size", 65536);
>  #else
>  pref("media.cubeb.sandbox", false);
>  #endif
>  
> +// Parallel MediaStreamGraph when sample rate is set in AudioContext

I don't think this comment is very useful.
Attachment #8965284 - Flags: review?(padenot) → review+
Comment on attachment 8950165 [details]
Bug 1387454 - Set sample rate in AudioContext constructor.

https://reviewboard.mozilla.org/r/219428/#review239724
Attachment #8950165 - Flags: review?(padenot) → review+
Pushed by achronop@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/b3a05d5d6216
Add AudioContextOptions in webidl. r=padenot,smaug
https://hg.mozilla.org/integration/autoland/rev/e6ab07a07c3b
Set sample rate in AudioContext constructor. r=padenot
https://hg.mozilla.org/integration/autoland/rev/6e227df35207
Create a MediaStreamGraph according to the given sample rate. r=padenot
https://hg.mozilla.org/integration/autoland/rev/a8e2605332f3
Mochitests for non default rate on webaudio and media recorder. r=padenot
https://hg.mozilla.org/integration/autoland/rev/b33c66f1d444
Mochitests for interaction of non default rate with WebRTC. r=padenot
https://hg.mozilla.org/integration/autoland/rev/2a13706881b8
Create preference to enable setting of sample rate in AudioContext. r=padenot
See Also: → 1452993
Blocks: 1498512
You need to log in before you can comment on or make changes to this bug.