Bug 20764 - It makes no sense to use EventListener for onaudioprocess if ScriptProcessorNode isn't EventTarget
Summary: It makes no sense to use EventListener for onaudioprocess if ScriptProcessorN...
Status: CLOSED WONTFIX
Alias: None
Product: AudioWG - OBSOLETE - Moved to Github
Classification: Unclassified
Component: Web Audio API - OBSOLETE - See Github (show other bugs)
Version: unspecified
Hardware: PC Linux
: P2 normal
Target Milestone: TBD
Assignee: Chris Rogers
QA Contact: public-audio
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 17351
  Show dependency treegraph
 
Reported: 2013-01-24 22:57 UTC by Olli Pettay
Modified: 2014-10-28 17:16 UTC (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Olli Pettay 2013-01-24 22:57:14 UTC
First, EventListener should not be used for onfoo handlers.
EventHandler is the right type (or in very special legacy cases OnErrorEventHandler).

But more importantly, if ScriptProcessorNode isn't EventTarget dispatching
event to it isn't really possible, and the event wouldn't have any
.target.

So, either ScriptProcessorNode needs to become EventTarget somehow, and 
get normal event handling, or the callback needs to not use Event objects.
Comment 1 Dominic Cooney 2013-02-19 05:14:30 UTC
Bug 17493 comment 1 [1] is relevant.

ScriptProcessorNode should have its own type of callback for onaudioprocess.

[1] <https://www.w3.org/Bugs/Public/show_bug.cgi?id=17493#c1>
Comment 2 Ehsan Akhgari [:ehsan] 2013-02-19 05:16:40 UTC
That works too, as long as it's not an event target.
Comment 3 Ehsan Akhgari [:ehsan] 2013-02-19 05:19:39 UTC
Also, in that case, we should rename onaudioprocess for consistency as well.
Comment 4 Chris Rogers 2013-02-19 06:01:26 UTC
(In reply to comment #3)
> Also, in that case, we should rename onaudioprocess for consistency as well.

This would be a breaking API change for shipping versions, so we'd have to be careful to move it to the "deprecated" and recommended section if we thought it important to change.  Personally, I don't think there's any reason it can't still be called onaudioprocess even though it has nothing to do with EventListener
Comment 5 Ehsan Akhgari [:ehsan] 2013-02-19 06:12:01 UTC
(In reply to comment #4)
> (In reply to comment #3)
> > Also, in that case, we should rename onaudioprocess for consistency as well.
> 
> This would be a breaking API change for shipping versions, so we'd have to
> be careful to move it to the "deprecated" and recommended section if we
> thought it important to change.  Personally, I don't think there's any
> reason it can't still be called onaudioprocess even though it has nothing to
> do with EventListener

webkit can still support the old API I guess, but the onXXX naming convention is used throughout the web platform for things using event targets, and using such a naming scheme will be very surprising to all web developers, so I think that is a really bad idea.

Also the usage of callbacks can enable us clean up the API by accepting the callback as an argument to the createScriptProcessor function, which makes it unnecessary to expose it on ScriptProcessorNode at all.  Speaking of which, I don't think exposing bufferSize on that node type makes much sense either.
Comment 6 Srikumar Subramanian (Kumar) 2013-02-19 08:04:40 UTC
(In reply to comment #5)

> Also the usage of callbacks can enable us clean up the API by accepting the
> callback as an argument to the createScriptProcessor function, which makes
> it unnecessary to expose it on ScriptProcessorNode at all.  Speaking of
> which, I don't think exposing bufferSize on that node type makes much sense
> either.

It is useful to be able to change the callback after creating the script node. Please lets not change that.

Exposing buffer size is also useful. The script node currently shares CPU with the main thread. Given this architecture, the amount of time available for audio can be different for different kinds of applications. A graphics rich application may not be able to guarantee enough callbacks just in time, in which case longer buffers would be desirable. An application that spends most of its time doing audio or being idle can use smaller buffer sizes to get lower latency. It is unnecessary to mandate an implementation to automatically determine the required buffering.
Comment 7 Robert O'Callahan (Mozilla) 2013-03-04 22:40:05 UTC
(In reply to comment #6)
> Exposing buffer size is also useful. The script node currently shares CPU
> with the main thread. Given this architecture, the amount of time available
> for audio can be different for different kinds of applications. A graphics
> rich application may not be able to guarantee enough callbacks just in time,
> in which case longer buffers would be desirable. An application that spends
> most of its time doing audio or being idle can use smaller buffer sizes to
> get lower latency. It is unnecessary to mandate an implementation to
> automatically determine the required buffering.

The problem with the author choosing the buffer size is that they almost certainly won't be able to choose a size that will work on all kinds of platforms and devices. The UA is best placed to do that. Anyway, there has been discussion about this on the list, and further discussion should take place there, definitely not in this bug :-)
Comment 8 Olivier Thereaux 2013-03-14 17:36:06 UTC
At the 2013-03-14 teleconference, we agreed that the best solution was probably the one suggested by Dominic in http://lists.w3.org/Archives/Public/public-audio/2013JanMar/0387.html

Ehsan, would you be able to fill in the blanks and make the change?
Comment 9 Dominic Cooney 2013-03-15 02:05:55 UTC
I feel like the change proposed here is not in line with recent discussion on public-audio ("The issue of ScriptProcessorNode not being an EventTarget.") <http://lists.w3.org/Archives/Public/public-audio/2013JanMar/0216.html>
Comment 10 Dominic Cooney 2013-03-15 02:07:10 UTC
Please disregard Comment 9--I did not see Comment 8.
Comment 11 Ehsan Akhgari [:ehsan] 2013-03-15 22:36:02 UTC
OK, I edited the spec accordingly:

https://dvcs.w3.org/hg/audio/rev/c68c2551f6c8

Please let me know if it looks good.

Now's the time to talk about AudioProcessingEvent.node.  I don't think that it makes sense for that attribute to exist, since it will be the same as AudioProcessingEvent.target.  Do I need to file another bug on that?
Comment 12 Olli Pettay 2013-03-15 22:45:17 UTC
"This interface is a type of Event which is passed to the onaudioprocess event handler used by ScriptProcessorNode. "
is still a bit odd, since event handling uses now the normal model.

And there is also
"This value controls how frequently the onaudioprocess event handler is called"

The spec also doesn't mention when audioprocess event is dispatched, only
vaguely talks about how bufferSize affects to the frequency of those events.
Comment 13 Dominic Cooney 2013-03-18 09:12:33 UTC
Where is EventHandler defined? In the [DOM] reference, only EventListener is defined. I think the spec might be missing a reference.
Comment 14 Ehsan Akhgari [:ehsan] 2013-03-21 03:56:45 UTC
(In reply to comment #12)
> "This interface is a type of Event which is passed to the onaudioprocess
> event handler used by ScriptProcessorNode. "
> is still a bit odd, since event handling uses now the normal model.
> 
> And there is also
> "This value controls how frequently the onaudioprocess event handler is
> called"

I cleared up the wording in https://dvcs.w3.org/hg/audio/rev/e26b49247d95.

> The spec also doesn't mention when audioprocess event is dispatched, only
> vaguely talks about how bufferSize affects to the frequency of those events.

That is a good question.  I'll leave that part to crogers.

(In reply to comment #13)
> Where is EventHandler defined? In the [DOM] reference, only EventListener is
> defined. I think the spec might be missing a reference.

The HTML spec.  I added a reference in https://dvcs.w3.org/hg/audio/rev/2056740735fd.
Comment 15 Olivier Thereaux 2014-10-28 17:14:06 UTC
Web Audio API issues have been migrated to Github. 
See https://github.com/WebAudio/web-audio-api/issues
Comment 16 Olivier Thereaux 2014-10-28 17:16:59 UTC
Closing. See https://github.com/WebAudio/web-audio-api/issues for up to date list of issues for the Web Audio API.