New issue
Advanced search Search tips

Issue 808226 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Add handler class to all audio nodes that don't have an explicit handler class

Project Member Reported by rtoy@chromium.org, Feb 1 2018

Issue description

Many classes such as the IIRFIlterNode class don't have an explict
matchine IIRFilterHandler class. Instead they explicitly set the
handler by creating an AudioBasicPRocessorHandler.

When reading the header files, this is confusing because it's hard to
tell where the handler is, and because it's different from all the
other nodes which have an explicit handler class.

Thus, it would be nice if these classes added an explicit handler
class that is a very thin subclass of AudioBasicProcessorHandler.
Then each node type looks the same.


 
Project Member

Comment 1 by bugdroid1@chromium.org, Feb 12 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/76e19961dc5ff8f27812cef9de0e4b85621f2e7c

commit 76e19961dc5ff8f27812cef9de0e4b85621f2e7c
Author: Raymond Toy <rtoy@chromium.org>
Date: Mon Feb 12 18:36:59 2018

Add Node handler classes to the nodes that don't have them

Many AudioNodes such as BiquadFilterNode don't have an explicit
BiquadFilterHandler, unlike GainNode, PannerNode, and so on.  Instead
they internally create the required handler using
AudioBasicProcessorHandler.

Because the node doesn't have an explicit handler, it's always
confusing to understand where all of the magic processing comes from.
By adding a very simple explicit handler class the connection is much
clearer and is the same as all the other nodes.

Bug:  808226 
Test: Must not affect any test
Change-Id: I6d0269a007fb40f0b5328a34c7bd058e37c91dc6
Reviewed-on: https://chromium-review.googlesource.com/898306
Reviewed-by: Hongchan Choi <hongchan@chromium.org>
Commit-Queue: Raymond Toy <rtoy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536138}
[modify] https://crrev.com/76e19961dc5ff8f27812cef9de0e4b85621f2e7c/third_party/WebKit/Source/modules/webaudio/AudioBasicProcessorHandler.cpp
[modify] https://crrev.com/76e19961dc5ff8f27812cef9de0e4b85621f2e7c/third_party/WebKit/Source/modules/webaudio/AudioBasicProcessorHandler.h
[modify] https://crrev.com/76e19961dc5ff8f27812cef9de0e4b85621f2e7c/third_party/WebKit/Source/modules/webaudio/AudioBasicProcessorHandlerTest.cpp
[modify] https://crrev.com/76e19961dc5ff8f27812cef9de0e4b85621f2e7c/third_party/WebKit/Source/modules/webaudio/BiquadFilterNode.cpp
[modify] https://crrev.com/76e19961dc5ff8f27812cef9de0e4b85621f2e7c/third_party/WebKit/Source/modules/webaudio/BiquadFilterNode.h
[modify] https://crrev.com/76e19961dc5ff8f27812cef9de0e4b85621f2e7c/third_party/WebKit/Source/modules/webaudio/DelayNode.cpp
[modify] https://crrev.com/76e19961dc5ff8f27812cef9de0e4b85621f2e7c/third_party/WebKit/Source/modules/webaudio/DelayNode.h
[modify] https://crrev.com/76e19961dc5ff8f27812cef9de0e4b85621f2e7c/third_party/WebKit/Source/modules/webaudio/IIRFilterNode.cpp
[modify] https://crrev.com/76e19961dc5ff8f27812cef9de0e4b85621f2e7c/third_party/WebKit/Source/modules/webaudio/IIRFilterNode.h
[modify] https://crrev.com/76e19961dc5ff8f27812cef9de0e4b85621f2e7c/third_party/WebKit/Source/modules/webaudio/WaveShaperNode.cpp
[modify] https://crrev.com/76e19961dc5ff8f27812cef9de0e4b85621f2e7c/third_party/WebKit/Source/modules/webaudio/WaveShaperNode.h

Comment 2 by rtoy@chromium.org, Feb 13 2018

Owner: rtoy@chromium.org
Status: Fixed (was: Available)

Sign in to add a comment