[Webkit-unassigned] [Bug 138739] start/stop method for AudioBufferSourceNodes and OscillatorNodes can take no args

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=138739

Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #241704|review?                     |review+
              Flags|                            |

--- Comment #6 from Darin Adler <darin at apple.com> ---
Comment on attachment 241704
  --> https://bugs.webkit.org/attachment.cgi?id=241704
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=241704&action=review

Not sure this code has enough test coverage.

> Source/WebCore/Modules/webaudio/AudioBufferSourceNode.cpp:375
> +    startPlaying(BufferPlaybackMode::Entire, 0, 0, buffer() ? buffer()->duration() : 0, ec);

The syntax here doesn’t seem quite right. Since BufferPlaybackMode is just an enum, not an enum class, we don’t need the enum name qualifier. Maybe it was intended to be an enum class?

> Source/WebCore/Modules/webaudio/AudioBufferSourceNode.cpp:428
> +        grainOffset = std::max(0.0, grainOffset);

This std::max is not needed since we check above for grainOffset < 0.

> Source/WebCore/Modules/webaudio/AudioBufferSourceNode.cpp:430
> +        m_grainOffset = grainOffset;

This should just be written in a single line:

    m_grainOffset = std::min(bufferDuration, grainOffset);

It’s also slightly clearer not to modify the passed in arguments.

> Source/WebCore/Modules/webaudio/AudioBufferSourceNode.cpp:433
> +        grainDuration = std::max(0.0, grainDuration);

This std::max is not needed since we check above for grainDuration < 0.

> Source/WebCore/Modules/webaudio/AudioBufferSourceNode.cpp:435
> +        m_grainDuration = grainDuration;

This should just be written in a single line:

    m_grainDuration = std::min(bufferDuration - m_grainOffset, grainDuration);

It’s also slightly clearer not to modify the passed in arguments.

> Source/WebCore/Modules/webaudio/AudioBufferSourceNode.cpp:457
> +    // Handle unspecified duration where 0 means the rest of the buffer.
> +    if (!grainDuration)
> +        grainDuration = buffer()->duration();

I don’t understand the mention of “unspecified” here and “0”. Where is the code that turns unspecified into 0? Or is 0 a magic value?

> Source/WebCore/Modules/webaudio/AudioScheduledSourceNode.cpp:184
>      when = std::max<double>(0, when);

This std::max is not needed now that we check "when < 0" above, and should be removed.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20141118/b2c57686/attachment-0002.html>