Enable OfflineAudioContext to run at arbitrary sampling rates

RESOLVED FIXED in mozilla24

Status

()

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Ehsan, Assigned: Ehsan)

Tracking

Trunk
mozilla24
x86
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(11 attachments)

1.21 KB, patch
roc
: review+
Details | Diff | Splinter Review
21.30 KB, patch
roc
: review+
Details | Diff | Splinter Review
6.41 KB, patch
roc
: review+
Details | Diff | Splinter Review
7.23 KB, patch
roc
: review+
Details | Diff | Splinter Review
10.69 KB, patch
roc
: review+
Details | Diff | Splinter Review
3.50 KB, patch
roc
: review+
Details | Diff | Splinter Review
4.40 KB, patch
roc
: review+
Details | Diff | Splinter Review
1.94 KB, patch
roc
: review+
Details | Diff | Splinter Review
4.44 KB, patch
roc
: review+
Details | Diff | Splinter Review
2.84 KB, patch
roc
: review+
Details | Diff | Splinter Review
1.93 KB, patch
roc
: review+
Details | Diff | Splinter Review
Follow-up from bug 836599.  Currently, our OfflineAudioContext can only run at 48KHz.
The try server is loving this stuff! \o/
Comment on attachment 752468 [details] [diff] [review]
Part 2: Teach each AudioNodeStream about its sampling rate, and store the value inside AudioContext

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

::: content/media/MediaStreamGraph.cpp
@@ +989,5 @@
> +        AudioNodeStream* n = mStreams[i]->AsAudioNodeStream();
> +        if (n) {
> +          // We know that the rest of the streams will run at the same rate.
> +          sampleRate = n->SampleRate();
> +          break;

I think we should store this in the graph rather than recomputing it here.

Can we avoid having to store it in all the AudioNodeStreams then?
(In reply to comment #14)
> Comment on attachment 752468 [details] [diff] [review]
>   --> https://bugzilla.mozilla.org/attachment.cgi?id=752468
> Part 2: Teach each AudioNodeStream about its sampling rate, and store the value
> inside AudioContext
> 
> Review of attachment 752468 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: content/media/MediaStreamGraph.cpp
> @@ +989,5 @@
> > +        AudioNodeStream* n = mStreams[i]->AsAudioNodeStream();
> > +        if (n) {
> > +          // We know that the rest of the streams will run at the same rate.
> > +          sampleRate = n->SampleRate();
> > +          break;
> 
> I think we should store this in the graph rather than recomputing it here.
> 
> Can we avoid having to store it in all the AudioNodeStreams then?

I thought about doing that, I'm not sure if it's safe to keep the assumption that all streams will run at the same sampling rate forever, but I would like to experiment with MediaStreamAudioSource/DestinationNode when they land a bit.  For now I think it's more conservative to store them on AudioNodeStreams, so I'd like to proceed with this right now, and later on switch to storeing them in the graph if we decide that makes sense.
Flags: needinfo?(roc)
Flags: needinfo?(roc)
Mass moving Web Audio bugs to the Web Audio component.  Filter on duckityduck.
Component: Video/Audio → Web Audio
You need to log in before you can comment on or make changes to this bug.