Add setter methods to PannerNode that will handle exceptions for setting channelCount, channelCountMode. Phased out sampleRate from AudioNode, using BaseAudioContext's sampleRate instead for processIfNecessary method instead.
Also separated ChannelCountMode, ChannelCount enums into their own files
Created attachment 403800 [details] Patch
Comment on attachment 403800 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=403800&action=review > Source/WebCore/ChangeLog:8 > + Added setter methods to PannerNode that handle exceptions for channelCount, channelCountMode, according to spec: https://www.w3.org/TR/webaudio/#dom-audionode-channelcount. Please wrap lines properly so they are not too long like this. > Source/WebCore/ChangeLog:9 > + Phased out sampleRate from AudioNode, using BaseAudioContext's sampleRate for processIfNecessary method. Moved ChannelCount, ChannelCountMode enums into their own files. Ditto. > Source/WebCore/Modules/webaudio/AudioNode.cpp:295 > +ChannelCountMode AudioNode::channelCountMode() This could be inlined in the header now. > Source/WebCore/Modules/webaudio/AudioNode.cpp:300 > +ExceptionOr<void> AudioNode::setChannelCountMode(const ChannelCountMode mode) const is not needed. > Source/WebCore/Modules/webaudio/AudioNode.cpp:309 > + if (mode == Max) The whole implementation of this method makes no sense anymore. Should be something like: m_channelCountMode = mode; > Source/WebCore/Modules/webaudio/AudioNode.cpp:324 > +ChannelInterpretation AudioNode::channelInterpretation() This could be inlined in the header now. > Source/WebCore/Modules/webaudio/AudioNode.cpp:336 > + if (interpretation == ChannelInterpretation::Speakers) The implementation of this method does not make sense anymore. > Source/WebCore/Modules/webaudio/AudioNode.cpp:381 > + m_lastNonSilentTime = (context().currentSampleFrame() + framesToProcess) / static_cast<double>(context().sampleRate()); Nothing to do with the enum string. Please split unrelated changes in separate patches. > Source/WebCore/Modules/webaudio/AudioNode.h:180 > + virtual ExceptionOr<void> setChannelCountMode(const ChannelCountMode); No need for const. > Source/WebCore/Modules/webaudio/AudioNode.h:182 > + ChannelInterpretation channelInterpretation(); should be const. > Source/WebCore/Modules/webaudio/AudioNode.h:185 > ChannelCountMode internalChannelCountMode() const { return m_channelCountMode; } This method seems identical to channelCountMode() now. It should be dropped. > Source/WebCore/Modules/webaudio/AudioNode.h:186 > + ChannelInterpretation internalChannelInterpretation() const { return m_channelInterpretation; } ditto. > Source/WebCore/Modules/webaudio/AudioNodeInput.cpp:148 > + ChannelCountMode mode = node()->internalChannelCountMode(); auto mode > Source/WebCore/Modules/webaudio/AudioNodeInput.cpp:161 > + if (mode == ClampedMax) Please make that enum an enum class. > Source/WebCore/Modules/webaudio/AudioNodeInput.cpp:199 > + ChannelInterpretation interpretation = node()->internalChannelInterpretation(); auto > Source/WebCore/Modules/webaudio/ChannelCountMode.h:30 > +enum ChannelCountMode { Should be an enum class. > Source/WebCore/Modules/webaudio/PannerNode.cpp:342 > + return Exception { NotSupportedError }; Please provide an explanation string... > Source/WebCore/Modules/webaudio/PannerNode.cpp:344 > + auto result = this->AudioNode::setChannelCount(channelCount); this-> is not needed. Also, seems like this could be return as: return AudioNode::setChannelCount(channelCount); > Source/WebCore/Modules/webaudio/PannerNode.cpp:352 > +ExceptionOr<void> PannerNode::setChannelCountMode(const ChannelCountMode mode) No const. > Source/WebCore/Modules/webaudio/PannerNode.cpp:357 > + auto result = this->AudioNode::setChannelCountMode(mode); Same comments as above. > Source/WebCore/Modules/webaudio/PannerNode.h:131 > + ExceptionOr<void> setChannelCount(unsigned) override; s/override/final > Source/WebCore/Modules/webaudio/PannerNode.h:132 > + ExceptionOr<void> setChannelCountMode(const ChannelCountMode) override; s/override/final
Created attachment 403806 [details] Patch
Created attachment 403807 [details] Patch
Created attachment 403808 [details] Patch
Comment on attachment 403808 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=403808&action=review Patch does not build in debug. > Source/WebCore/Modules/webaudio/AudioNode.cpp:299 > + if (mode != ChannelCountMode::Max && mode != ChannelCountMode::ClampedMax && mode != ChannelCountMode::Explicit) Why not we need this check? These seem to be the only values of ChannelCountMode. > Source/WebCore/Modules/webaudio/AudioNode.cpp:317 > + if (interpretation != ChannelInterpretation::Speakers && interpretation != ChannelInterpretation::Discrete) Ditto. > Source/WebCore/Modules/webaudio/AudioNode.h:176 > + unsigned channelCount() { return m_channelCount; } const > Source/WebCore/Modules/webaudio/AudioNode.h:179 > + ChannelCountMode channelCountMode() { return m_channelCountMode; } const
Comment on attachment 403808 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=403808&action=review >> Source/WebCore/Modules/webaudio/AudioNode.cpp:299 >> + if (mode != ChannelCountMode::Max && mode != ChannelCountMode::ClampedMax && mode != ChannelCountMode::Explicit) > > Why not we need this check? These seem to be the only values of ChannelCountMode. Why *do* we need this check?
Comment on attachment 403808 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=403808&action=review > Source/WebCore/Modules/webaudio/AudioNode.cpp:310 > +ExceptionOr<void> AudioNode::setChannelInterpretation(const ChannelInterpretation interpretation) Drop the const.
Created attachment 403864 [details] Patch
Comment on attachment 403864 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=403864&action=review r=me with nit fix > Source/WebCore/Modules/webaudio/AudioNode.h:183 > + ExceptionOr<void> setChannelInterpretation(const ChannelInterpretation); Drop the const.
Created attachment 403868 [details] Patch
Committed r264176: <https://trac.webkit.org/changeset/264176> All reviewed patches have been landed. Closing bug and clearing flags on attachment 403868 [details].
<rdar://problem/65275132>