[Webkit-unassigned] [Bug 100261] [gtk] core implementation for getUserMedia API

[bugzilla-daemon at webkit.org]

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





--- Comment #16 from Jonathon Jongsma (jonner) <jonathon at quotidian.org>  2013-02-05 08:42:49 PST ---
(In reply to comment #15)
> Looks good overall, the Stream player seems quite incomplete though, I wonder if it'd make sense to have only the refactoring in this patch and keep a minimal-working stream player for a separate patch?

Yeah, I was debating that as well.  I can do that if you'd like.

> 
> > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h:156
> > +            guint m_videoCapsHandler;
> 
> This doesn't seem used anywhere?

Oops, left over from a different approach 

> > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:283
> > +    // FIXME: use implementation from StreamMediaPlayer??
> 
> I don't understand this FIXME, is it a left-over from a previous version of the patch?

yeah, I'll remove.

> > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.h:110
> > +    GRefPtr<GstStreamVolume> m_volume;
> 
> This name could be confusing, what about m_volumeElement? Also, OOC don't we need a GRefPtr specific implementation for GstStreamVolume, which is an interface, I'm not sure how it combines with GRefPtr in this patch.

Well, since there's no template specialization for GstStreamVolume, it should just use the un-specialized implementation (GRefPtr<T>), which is the implementation for GObject (g_object_ref_sink/g_object_unref).  Since GstStreamVolume has 'GObject' as its only prerequisite, this is probably correct behavior, right?

> > Source/WebCore/platform/graphics/gstreamer/StreamMediaPlayerPrivateGStreamer.cpp:126
> > +        // go go go
> 
> Hum :)

Hmm, left over from the implementation.  if I remove the stream player from this patch, we can deal with that later :)

> > Source/WebCore/platform/graphics/gstreamer/StreamMediaPlayerPrivateGStreamer.h:89
> > +    GstElement* m_videoSinkBin;
> > +    GstElement* m_audioSinkBin;
> 
> Why not reuse the ones from the base class?

Oops, thanks for catching that.

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