New issue
Advanced search Search tips

Issue 834863 link

Starred by 1 user
Status: Fixed
Owner:
rtoy@chromium.org
Closed: May 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Simplify AudioParam::Create arg list

Project Member Reported by rtoy@chromium.org, Apr 19 2018 
Consider simplifying AudioParam::Create method call.  One example is 

      gain_(AudioParam::Create(context,
                               kParamTypeGainGain,
                               "Gain.gain",
                               1.0,
                               AudioParamHandler::AutomationRate::kAudio,
                               AudioParamHandler:;AutomationRateMode::kIsVariable)) {

The "name" parameter isn't really needed anymore because it can be derived completely from kParamTypeGainGain.

A vast majority of AudioParam default to a-rate and is user-selectable.  Many nodes have a default value of 0 (but not gain.gain), so many calls could be reduced to

AudioParam::Create(context, kParamTypeFoo)

and let everything default.  More complicated ones will, of course, have to be complicated.  But we could sondier using a struct to encapsulate all the arguments.

Comment 1 by rtoy@chromium.org, Apr 24 2018 

Most AudioParams also have a default value of 0, and almost all AudioParams have a rate of kAudio with a mode of kVariable.  Maybe have those be the defaults too?

Comment 2 by rtoy@chromium.org, Apr 25 2018

Owner: rtoy@chromium.org
Status: Started (was: Available)
Project Member

Comment 3 by bugdroid1@chromium.org, Apr 30 2018 

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

commit 23eefd32741dc33f5724a8825c24a02e84d54ed5
Author: Raymond Toy <rtoy@chromium.org>
Date: Mon Apr 30 16:47:52 2018

Simplify AudioParam::Create interface

First, don't need to specify the name for the AudioParam when constructing it
because we can derive a good name from the type.  Except for
AudioWorkletNode which has user-defined AudioParam names.  So add a
method to allow setting the name (but only if the type is
AudioWorklet).

Second, add another Create method that accepts the context, the type enum,
and the default value, letting the rate, mode, and limits default to the most
common values of "a-rate", changeable, min and max float values.

Bug:  834863 
Test: no user-visible changes

Change-Id: Ic6895f8f6683cda6bc5aa6b381a1c8a1c6558759
Reviewed-on: https://chromium-review.googlesource.com/1020237
Commit-Queue: Raymond Toy <rtoy@chromium.org>
Reviewed-by: Hongchan Choi <hongchan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554789}
[modify] https://crrev.com/23eefd32741dc33f5724a8825c24a02e84d54ed5/third_party/WebKit/LayoutTests/webaudio/AudioParam/worklet-warnings-expected.txt
[modify] https://crrev.com/23eefd32741dc33f5724a8825c24a02e84d54ed5/third_party/blink/renderer/modules/webaudio/audio_buffer_source_node.cc
[modify] https://crrev.com/23eefd32741dc33f5724a8825c24a02e84d54ed5/third_party/blink/renderer/modules/webaudio/audio_listener.cc
[modify] https://crrev.com/23eefd32741dc33f5724a8825c24a02e84d54ed5/third_party/blink/renderer/modules/webaudio/audio_param.cc
[modify] https://crrev.com/23eefd32741dc33f5724a8825c24a02e84d54ed5/third_party/blink/renderer/modules/webaudio/audio_param.h
[modify] https://crrev.com/23eefd32741dc33f5724a8825c24a02e84d54ed5/third_party/blink/renderer/modules/webaudio/audio_worklet_node.cc
[modify] https://crrev.com/23eefd32741dc33f5724a8825c24a02e84d54ed5/third_party/blink/renderer/modules/webaudio/biquad_filter_node.cc
[modify] https://crrev.com/23eefd32741dc33f5724a8825c24a02e84d54ed5/third_party/blink/renderer/modules/webaudio/constant_source_node.cc
[modify] https://crrev.com/23eefd32741dc33f5724a8825c24a02e84d54ed5/third_party/blink/renderer/modules/webaudio/delay_node.cc
[modify] https://crrev.com/23eefd32741dc33f5724a8825c24a02e84d54ed5/third_party/blink/renderer/modules/webaudio/dynamics_compressor_node.cc
[modify] https://crrev.com/23eefd32741dc33f5724a8825c24a02e84d54ed5/third_party/blink/renderer/modules/webaudio/gain_node.cc
[modify] https://crrev.com/23eefd32741dc33f5724a8825c24a02e84d54ed5/third_party/blink/renderer/modules/webaudio/oscillator_node.cc
[modify] https://crrev.com/23eefd32741dc33f5724a8825c24a02e84d54ed5/third_party/blink/renderer/modules/webaudio/panner_node.cc
[modify] https://crrev.com/23eefd32741dc33f5724a8825c24a02e84d54ed5/third_party/blink/renderer/modules/webaudio/stereo_panner_node.cc

Comment 4 by rtoy@chromium.org, May 1 2018

Status: Fixed (was: Started)
►

Sign in to add a comment