[Webkit-unassigned] [Bug 122831] [GStreamer] Store video-sink in a bin

[bugzilla-daemon at webkit.org]

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


Gustavo Noronha (kov) <gns at gnome.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #215374|review?                     |review-
               Flag|                            |




--- Comment #4 from Gustavo Noronha (kov) <gns at gnome.org>  2013-10-29 04:58:15 PST ---
(From update of attachment 215374)
View in context: https://bugs.webkit.org/attachment.cgi?id=215374&action=review

LGTM in general, but it lacks a changelog entry.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:589
>      }
>  
>      if (!m_fpsSink) {

This should be an else block. I think the actualVideoSink name is misleading because it may be something that is not the actual video sink heh. Is videoSink taken?

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:603
> -    return actualVideoSink;
> +    firstChild = actualVideoSink;

You could initialize firstChild to this instead of doing it in the else block, just a nit.

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