Implement DelayNode

RESOLVED FIXED in mozilla19

Status

()

Core
Web Audio
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: Away for a while, Assigned: Away for a while)

Tracking

(Blocks: 1 bug)

Trunk
mozilla19
x86
Mac OS X
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Comment hidden (empty)
(Assignee)

Comment 1

5 years ago
Created attachment 677231 [details] [diff] [review]
Patch (v1)
Assignee: nobody → ehsan
Status: NEW → ASSIGNED
Attachment #677231 - Flags: review?(bzbarsky)
Comment on attachment 677231 [details] [diff] [review]
Patch (v1)

>+++ b/content/media/webaudio/AudioContext.cpp
>+AudioContext::CreateDelay(const Optional<float>& aMaxDelayTime)

I think the spec here is bogus.  The IDL should say:

    DelayNode createDelay(optional double maxDelayTime = 1.0);

instead of describing the behavior in prose and making you implement it in C++.

Again, we could annotate DelayTime as returning a weak ref.  And again, at some point we need to have something actually happen when the mDelay's value is changed, right?

r=me modulo all that.
Attachment #677231 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 3

5 years ago
(In reply to Boris Zbarsky (:bz) from comment #2)
> Comment on attachment 677231 [details] [diff] [review]
> Patch (v1)
> 
> >+++ b/content/media/webaudio/AudioContext.cpp
> >+AudioContext::CreateDelay(const Optional<float>& aMaxDelayTime)
> 
> I think the spec here is bogus.  The IDL should say:
> 
>     DelayNode createDelay(optional double maxDelayTime = 1.0);
> 
> instead of describing the behavior in prose and making you implement it in
> C++.

Right.  I raised this yesterday at the spec level (https://www.w3.org/Bugs/Public/show_bug.cgi?id=19808) and I just changed the spec to do this, so I'll change my code here accordingly.

> Again, we could annotate DelayTime as returning a weak ref.

Will do.

>  And again, at
> some point we need to have something actually happen when the mDelay's value
> is changed, right?

Yes, hopefully very soon!
(Assignee)

Updated

5 years ago
Depends on: 807697
https://hg.mozilla.org/mozilla-central/rev/54ef53ee4af9
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
(Assignee)

Comment 6

5 years ago
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.