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
Patch (38.76 KB, patch)
2020-08-03 14:54 PDT, Chris Dumez
no flags
Chris Dumez
Comment 1 2020-08-03 13:00:03 PDT
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
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
Note You need to log in before you can comment on or make changes to this bug.