[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 #229599|review?                     |review-
               Flag|                            |




--- Comment #3 from Brent Fulgham <bfulgham at webkit.org>  2014-04-17 17:23:09 PST ---
(From update of attachment 229599)
View in context: https://bugs.webkit.org/attachment.cgi?id=229599&action=review

This looks like a great start. Please use "notImplemented()" in the various stubs you have. Also, are there specific build requirements you need to use MediaFoundation? Is it only available under Windows 8 or something?

We might want to protect the MediaFoundation stuff inside an SDK guard or something.

> Source/WebCore/platform/graphics/win/MediaPlayerPrivateMediaFoundation.cpp:55
> +{

I'd recommend putting "notImplemented()" in all of these locations. Then we can remove them as you flesh out support.

> Source/WebCore/platform/graphics/win/MediaPlayerPrivateMediaFoundation.cpp:69
> +void MediaPlayerPrivateMediaFoundation::load(const String&) { }

Ditto for all of these stubs:

load(const String&)
{
    notImplemented();
}

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

Get rid of this single space character!

> Source/WebCore/platform/graphics/win/MediaPlayerPrivateMediaFoundation.h:34
> +    static void registerMediaEngine(MediaEngineRegistrar registrar);

Remove 'registrar' please.

> Source/WebKit/win/FullscreenVideoController.cpp:28
> +#if ENABLE(VIDEO) && !USE(GSTREAMER) && !USE(MEDIA_FOUNDATION)

Why don't you want to support full screen with media foundation?

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