WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
215093
Add constructor for GainNode
https://bugs.webkit.org/show_bug.cgi?id=215093
Summary
Add constructor for GainNode
Chris Dumez
Reported
2020-08-03 12:11:17 PDT
Add constructor for GainNode: -
https://www.w3.org/TR/webaudio/#gainnode
Attachments
Patch
(42.64 KB, patch)
2020-08-03 13:00 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Patch
(38.76 KB, patch)
2020-08-03 14:54 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Chris Dumez
Comment 1
2020-08-03 13:00:03 PDT
Created
attachment 405856
[details]
Patch
Eric Carlson
Comment 2
2020-08-03 13:21:48 PDT
Comment on
attachment 405856
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=405856&action=review
> Source/WebCore/Modules/webaudio/GainNode.cpp:59 > + auto result = gainNode->setChannelCount(options.channelCount.valueOr(2)); > + if (result.hasException()) > + return result.releaseException(); > + > + result = gainNode->setChannelCountMode(options.channelCountMode.valueOr(ChannelCountMode::Max)); > + if (result.hasException()) > + return result.releaseException(); > + > + result = gainNode->setChannelInterpretation(options.channelInterpretation.valueOr(ChannelInterpretation::Speakers)); > + if (result.hasException()) > + return result.releaseException();
This coded is duplicated in at least six nodes now. It would be better to have an AudioNode constructor or method that takes an AudioNodeOptions.
> Source/WebCore/Modules/webaudio/GainNode.h:42 > + static ExceptionOr<Ref<GainNode>> create(BaseAudioContext& context, const GainOptions& = { });
Won't the bindings generator always pass a default-initialized dictionary?
Chris Dumez
Comment 3
2020-08-03 13:28:18 PDT
Comment on
attachment 405856
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=405856&action=review
>> Source/WebCore/Modules/webaudio/GainNode.cpp:59 >> + return result.releaseException(); > > This coded is duplicated in at least six nodes now. It would be better to have an AudioNode constructor or method that takes an AudioNodeOptions.
Yes I agree with you. The default values (when not specified in the dictionary) differ from node to node though so we have to watch out for that. I will try and find a good way to factor this is a follow-up patch. Maybe it can wait until we're done adding all constructor to avoid conflicting.
>> Source/WebCore/Modules/webaudio/GainNode.h:42 >> + static ExceptionOr<Ref<GainNode>> create(BaseAudioContext& context, const GainOptions& = { }); > > Won't the bindings generator always pass a default-initialized dictionary?
Yes, but this is also called from BaseAudioContext::createGain() above.
EWS
Comment 4
2020-08-03 14:51:17 PDT
Tools/Scripts/svn-apply failed to apply
attachment 405856
[details]
to trunk. Please resolve the conflicts and upload a new patch.
Chris Dumez
Comment 5
2020-08-03 14:54:03 PDT
Created
attachment 405868
[details]
Patch
Chris Dumez
Comment 6
2020-08-03 15:41:37 PDT
Comment on
attachment 405868
[details]
Patch Clearing flags on attachment: 405868 Committed
r265227
: <
https://trac.webkit.org/changeset/265227
>
Chris Dumez
Comment 7
2020-08-03 15:41:39 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 8
2020-08-03 16:25:27 PDT
<
rdar://problem/66497105
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug