[webkit-reviews] review granted: [Bug 44539] API changes for Video Frame sharing between WebKit and Chromium : [Attachment 65477] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Aug 25 15:18:57 PDT 2010


Darin Fisher (:fishd, Google) <fishd at chromium.org> has granted Victoria
<vrk at google.com>'s request for review:
Bug 44539: API changes for Video Frame sharing between WebKit and Chromium
https://bugs.webkit.org/show_bug.cgi?id=44539

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

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
WebKit/chromium/public/WebMediaPlayer.h:132
 +	// For getting frames to and from media player.
nit: Something like the comments on VideoFrameProvider should be here as well.
Ideally, the user of the WebKit API should not need to read WebCore files to
understand how to use the API.

WebKit/chromium/public/WebVideoFrame.h:31
 +  
nit: extra new line here that should be removed

WebKit/chromium/public/WebVideoFrame.h:41
 +	// These enums must be kept in sync with media::VideoFormat.
nit: I'd prefer that we avoid references to code in the Chromium repository.
These things can easily get out of sync as folks changing code in Chromium
may not realize they need to make a patch to WebKit to fix up the name of
something.  I'd just drop this comment.

WebKit/chromium/src/VideoFrameChromiumImpl.h:46
 +	static WebVideoFrame* toWebVideoFrame(VideoFrameChromium*);
nit: i'd recommend a new line after here for readability

R=me w/ these nits fixed.


More information about the webkit-reviews mailing list