[webkit-reviews] review granted: [Bug 95149] Add the duration attribute to MediaSource : [Attachment 161336] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Aug 29 16:31:26 PDT 2012


Eric Carlson <eric.carlson at apple.com> has granted Victoria Kirst
<vrk at chromium.org>'s request for review:
Bug 95149: Add the duration attribute to MediaSource
https://bugs.webkit.org/show_bug.cgi?id=95149

Attachment 161336: Patch
https://bugs.webkit.org/attachment.cgi?id=161336&action=review

------- Additional Comments from Eric Carlson <eric.carlson at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=161336&action=review


> Source/WebCore/ChangeLog:16
> +	   * Modules/mediasource/MediaSource.cpp:
> +	   (WebCore::MediaSource::duration):
> +	   (WebCore):
> +	   (WebCore::MediaSource::setDuration):

Nit: I think it is helpful to have comments about what changed in each function
in a ChangeLog entry.

> Source/WebCore/Modules/mediasource/MediaSource.cpp:71
> +    return m_readyState == closedKeyword() ?
> +	   std::numeric_limits<float>::quiet_NaN() : m_player->duration();

Nit: I don't think the line break aids readability here.

> LayoutTests/http/tests/media/media-source/media-source.js:142
> +    for (var index = 0; index < this.mediaSegments.length; index++)

Nit: you use "index" here and "i" elsewhere.

> LayoutTests/http/tests/media/media-source/media-source.js:321
> +MediaSourceTest.floatingPointEquals_ = function(expected, actual)
> +{
> +    var delta = Math.abs(actual - expected);
> +    return delta < 0.01;
> +};

Why does the function name end with an underscore? 

What makes 0.01 an acceptable delta?


More information about the webkit-reviews mailing list