[Webkit-unassigned] [Bug 115352] Make sure gstreamer source element is thread-safe

[bugzilla-daemon at webkit.org]

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





--- Comment #13 from Andre Moreira Magalhaes <andrunko at gmail.com>  2013-05-03 05:13:28 PST ---
(In reply to comment #12)
> (From update of attachment 200022 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=200022&action=review
> 
> > Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:453
> > +            gst_app_src_set_size(priv->appsrc, -1);
> 
> Any reason why you move these at the very end?
> 
IIRC, it was because of the locks, this call doesn't need to be done under locks and I refactored this method a bit to avoid locking/unlocking many times.

> > Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:-632
> > -    return g_strdup(WEBKIT_WEB_SRC(handler)->priv->uri);
> 
> A const gchar* function returning a copy a copy of a string? That looks wrong, but it was like that before already :) But then this is 0.10 code, which was broken exactly because of things like this. So decide for yourself if you want to leak here or potentially crash
> 
Yep, the issue here is that the uri may change, thus we need a lock/dup to get the uri. But as you said this is for 0.10 only so I would rather leak than crash.

> > Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:865
> > +    GST_OBJECT_UNLOCK(src);
> 
> I think these locks are actually unnecessary, as that function will only be called right after creating the instance and then never again. But they don't hurt
Yeah, not change here, but let me know if you want me to remove these extra locks, I wouldn't mind tbh, they won't hurt anyway :)

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