Closed
Bug 855568
Opened 12 years ago
Closed 11 years ago
Implement MediaElementAudioSourceNode
Categories
(Core :: Web Audio, defect)
Core
Web Audio
Tracking
()
RESOLVED
FIXED
mozilla26
People
(Reporter: kinetik, Assigned: roc)
References
Details
(Keywords: dev-doc-complete, Whiteboard: [qa-])
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
ehsan.akhgari
:
review+
|
Details | Diff | Splinter Review |
MediaElementAudioSourceNode is defined here: <https://dvcs.w3.org/hg/audio/raw-file/tip/webaudio/specification.html#MediaElementAudioSourceNode>.
Comment 1•12 years ago
|
||
Matthew, did you want to work on this? :-)
FWIW I think we should first implement MediaStreamAudioSourceNode (bug 856361) and then use mozCaptureStream to implement this node type.
Depends on: 856361
Reporter | ||
Comment 2•12 years ago
|
||
I don't think I've got time right now sorry, I just wanted to file it for the bug 855570 deps.
Updated•12 years ago
|
Assignee: nobody → ehsan
Comment 3•11 years ago
|
||
Mass moving Web Audio bugs to the Web Audio component. Filter on duckityduck.
Component: Video/Audio → Web Audio
Comment 4•11 years ago
|
||
Comment 5•11 years ago
|
||
Comment 6•11 years ago
|
||
Added a test, it doesn't pass and it crashes. But Bobby wanted to see it.
Attachment #761214 -
Attachment is obsolete: true
Assignee | ||
Updated•11 years ago
|
Assignee: ehsan → roc
Assignee | ||
Comment 7•11 years ago
|
||
The test originally attached to this patch won't work since playback of the media element may introduce silence (especially at the start) while the media element decoding pipeline spins up.
Assignee | ||
Comment 8•11 years ago
|
||
Basically you already had, which I've reviewed/tweaked a bit, plus a new test which is just test_mediaStreamAudioSourceNodeResampling.html but using MediaElementAudioSourceNode instead of MediaStreamAudioSourceNode. Nice and simple implementation :-).
Attachment #761761 -
Attachment is obsolete: true
Attachment #780774 -
Flags: review?(ehsan)
Comment 9•11 years ago
|
||
Comment on attachment 780774 [details] [diff] [review]
patch
Review of attachment 780774 [details] [diff] [review]:
-----------------------------------------------------------------
It would also be really great if you could add a test similar to test_mediaStreamAudioSourceNode.html. r=me with that.
Attachment #780774 -
Flags: review?(ehsan) → review+
Assignee | ||
Comment 10•11 years ago
|
||
You mean where the track sample rate is equal to the context sample rate? I don't think that's really necessary given this is a very thin wrapper around MediaStreamAudioSourceNode.
Comment 11•11 years ago
|
||
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #10)
> You mean where the track sample rate is equal to the context sample rate? I
Yes.
> don't think that's really necessary given this is a very thin wrapper around
> MediaStreamAudioSourceNode.
Hmm that's a fair point given our knowledge about the implementation, but I'd still like us to test the API surface there. If you want, I'm fine with us doing this in another bug though.
Assignee | ||
Comment 12•11 years ago
|
||
Comment 13•11 years ago
|
||
Push backed out for mochitest-1 failures:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=f7496fddb076
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/6946b8d86502
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/31453209a88a
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/c43900741e9a
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/3f150133a4de
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/ebd005b9e9d7
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/6e72a4e894e1
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/0f30f29ffa1f
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/fab0e9b04d8d
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/4382f83efcaf
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/47f46080685f
Assignee | ||
Comment 14•11 years ago
|
||
Comment 15•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
Comment 16•11 years ago
|
||
Comment on attachment 780774 [details] [diff] [review]
patch
This is needed for Web Audio in Firefox 25.
Attachment #780774 -
Flags: approval-mozilla-aurora?
Updated•11 years ago
|
Keywords: dev-doc-needed
Updated•11 years ago
|
Attachment #780774 -
Flags: approval-mozilla-aurora?
Comment 18•11 years ago
|
||
Comment 19•11 years ago
|
||
Backed out for mochitest failures.
https://hg.mozilla.org/releases/mozilla-aurora/rev/918a847c0018
https://tbpl.mozilla.org/php/getParsedLog.php?id=26380446&tree=Mozilla-Aurora
Assignee | ||
Comment 20•11 years ago
|
||
Flags: needinfo?(roc)
Updated•11 years ago
|
Keywords: branch-patch-needed
Comment 22•10 years ago
|
||
Doc is up-to-date: https://developer.mozilla.org/en-US/docs/Web/API/MediaElementAudioSourceNode
Keywords: dev-doc-needed → dev-doc-complete
You need to log in
before you can comment on or make changes to this bug.
Description
•