[webkit-reviews] review denied: [Bug 64731] Add MediaSource API to HTMLMediaElement : [Attachment 101481] Patch

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has denied Aaron Colwell
<acolwell at chromium.org>'s request for review:
Bug 64731: Add MediaSource API to HTMLMediaElement
https://bugs.webkit.org/show_bug.cgi?id=64731

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

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


Two meta-issues:

1) I think this is complex enough that is needs to be completely described in a
spec before any changes are checked in.

2) Even if this feature will only be enabled in Chromium, the code is in the
main WebKit repository so we need tests in the same repository.


> Source/WebCore/html/HTMLMediaElement.cpp:1682
> +    if (m_sourceState == SOURCE_CLOSED) {
> +	   scheduleEvent(eventNames().closeEvent);
> +	   return;
> +    }

Shouldn't this be "webkitsourcecloseEvent" for symmetry?

> Source/WebCore/platform/graphics/MediaPlayer.cpp:144
> +    virtual bool sourceAppend(const unsigned char* data, unsigned length) {
return false; }

These parameter names aren't needed.

> Source/WebCore/platform/graphics/MediaPlayerPrivate.h:151
> +    virtual bool sourceAppend(const unsigned char* data, unsigned length) {
return false; }

These parameter names aren't needed.