[Webkit-unassigned] [Bug 50770] [GStreamer] PlatformVideoWindowMac implementation

[bugzilla-daemon at webkit.org]

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


Eric Carlson <eric.carlson at apple.com> changed:

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




--- Comment #9 from Eric Carlson <eric.carlson at apple.com>  2011-01-11 09:00:46 PST ---
(From update of attachment 78513)
View in context: https://bugs.webkit.org/attachment.cgi?id=78513&action=review

r+ if you fix the RetainPtr assignment problem.

> Source/WebCore/platform/graphics/gstreamer/PlatformVideoWindow.h:48
> +        PlatformWidget window() const
> +{
> +#if !PLATFORM(MAC)
> +           return m_window;
> +#else
> +           return m_window.get();
> +#endif
> +}

I know the style bot is happy with it, but this brace/#if indentation is strange. It might be easier to read if this function was moved to the .mm file.


> Source/WebCore/platform/graphics/gstreamer/PlatformVideoWindowMac.mm:2
> + * Copyright (C) 2010 Igalia S.L

Probably want 2011 here.


> Source/WebCore/platform/graphics/gstreamer/PlatformVideoWindowMac.mm:31
> +    m_window = [[NSView alloc] init];

This should be m_window.adoptNS([[NSView alloc] init]);


> Source/WebCore/platform/graphics/gstreamer/PlatformVideoWindowMac.mm:44
> +        m_videoWindow = static_cast<PlatformWidget>(g_value_get_pointer(gst_structure_get_value(message->structure, "nsview")));
> +        [m_window.get() addSubview:m_videoWindow];

Might be worth ASSERT-ing m_videoWindow here.

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