[webkit-reviews] review granted: [Bug 215947] AudioParam.cancelAndHoldAtTime() is missing : [Attachment 408189] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Sep 7 11:57:26 PDT 2020
Darin Adler <darin at apple.com> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 215947: AudioParam.cancelAndHoldAtTime() is missing
https://bugs.webkit.org/show_bug.cgi?id=215947
Attachment 408189: Patch
https://bugs.webkit.org/attachment.cgi?id=408189&action=review
--- Comment #6 from Darin Adler <darin at apple.com> ---
Comment on attachment 408189
--> https://bugs.webkit.org/attachment.cgi?id=408189
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=408189&action=review
r=me assuming tests pass
> Source/WebCore/Modules/webaudio/AudioParamTimeline.cpp:196
> + size_t i;
> + // Find the first event at or just past cancelTime.
> + for (i = 0; i < m_events.size(); ++i) {
> + if (m_events[i]->time() > cancelTime)
> + break;
> + }
Vector::findMatching does this kind of thing, although it returns notFound
instead of size() when not found.
> Source/WebCore/Modules/webaudio/AudioParamTimeline.cpp:217
> + Optional<UniqueRef<ParamEvent>> newEvent;
> + Optional<UniqueRef<ParamEvent>> newSetValueEvent;
Normally we’d use unique_ptr instead of Optional<UniqueRef>.
> Source/WebCore/Modules/webaudio/AudioParamTimeline.cpp:376
> + ParamEvent& event = m_events[i].get();
> + ParamEvent* nextEvent = i < n - 1 ? &m_events[i + 1].get() :
nullptr;
auto&, auto or auto*
> Source/WebCore/Modules/webaudio/AudioParamTimeline.cpp:382
> + ParamEvent::Type nextEventType = nextEvent ?
static_cast<ParamEvent::Type>(nextEvent->type()) : ParamEvent::LastType /*
unknown */;
auto
> Source/WebCore/Modules/webaudio/AudioParamTimeline.cpp:451
> + {
This is brace style from another project, I think. But not just the new code.
> Source/WebCore/Modules/webaudio/AudioParamTimeline.cpp:605
> +inline float AudioParamTimeline::linearRampAtTime(Seconds t, float value1,
Seconds time1, float value2, Seconds time2)
Seems unlikely we have to write "inline" to get inlining here.
> Source/WebCore/Modules/webaudio/AudioParamTimeline.cpp:610
> +inline float AudioParamTimeline::exponentialRampAtTime(Seconds t, float
value1, Seconds time1, float value2, Seconds time2)
Ditto.
> Source/WebCore/Modules/webaudio/AudioParamTimeline.h:84
> + static UniqueRef<ParamEvent> createSetValueEvent(float value,
Seconds time)
> + {
> + return makeUniqueRef<ParamEvent>(ParamEvent::SetValue, value,
time, 0, Seconds { }, Vector<float> { }, 0, 0, nullptr);
> + }
Can we put most of the function bodies into the .cpp file instead of the
header?
More information about the webkit-reviews
mailing list