[Webkit-unassigned] [Bug 131830] media foundation video element skeleton

[bugzilla-daemon at webkit.org]

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


Brent Fulgham <bfulgham at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #229602|review?                     |review+
               Flag|                            |




--- Comment #8 from Brent Fulgham <bfulgham at webkit.org>  2014-04-22 11:14:07 PST ---
(From update of attachment 229602)
View in context: https://bugs.webkit.org/attachment.cgi?id=229602&action=review

Looks good. I had a few minor suggestions (I thin you should mark the methods in MediaPlayerPrivateMediaFoundation as 'override' where appropriate.

I assume the new USE(MEDIA_FOUNDATION) stuff will be added in a future patch to FeatureDefines.props?

> Source/WebCore/platform/graphics/win/MediaPlayerPrivateMediaFoundation.cpp:160
> +        || !m_player->visible())

I think this could be one line.

> Source/WebCore/platform/graphics/win/MediaPlayerPrivateMediaFoundation.h:49
> +    virtual bool hasVideo() const;

Should these be marked 'override'?

> Source/WebCore/platform/graphics/win/MediaPlayerPrivateMediaFoundation.h:70
> +

Please get rid of this extra whitespace.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.