Closed Bug 873553 Opened 7 years ago Closed 7 years ago

Enable OfflineAudioContext to run at arbitrary sampling rates

Categories

(Core :: Web Audio, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla24

People

(Reporter: ehsan, Assigned: ehsan)

References

Details

Attachments

(11 files)

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.
Here we go!
Attachment #752467 - Flags: review?(roc)
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.