[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


Andre Moreira Magalhaes <andrunko at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #200022|0                           |1
        is obsolete|                            |




--- Comment #16 from Andre Moreira Magalhaes <andrunko at gmail.com>  2013-05-20 10:49:12 PST ---
Created an attachment (id=202292)
 --> (https://bugs.webkit.org/attachment.cgi?id=202292&action=review)
Patch

(In reply to comment #14)
> > Source/WebCore/ChangeLog:3
> > +        Make sure gstreamer source element is thread-safe.
> 
> Sorry to be annoying about this but the  Changelog entry   format is not correct, please check other entries. Usually thee structure is:
> 
> bug title
> bug link
> 
> Reviewed by...
> 
> description of change
> 
Np, thanks for the review. Changelog updated.

> > Source/WebCore/ChangeLog:15
> > +        No new tests (OOPS!).
> 
> If no new test is added please remove this line.
> 
Done

> > Source/WebCore/ChangeLog:18
> > +        (_WebKitWebSrcPrivate):
> 
> It would bee good, I think, to have a little summary of each function/method change.
> 
All changes are adding/removing locks here and there and making sure some methods are called from main thread, so not updated here as it would be difficult to describe changes per function/method.

> >>> 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 :)
> 
> Right, please remove these locks :)
> 
Done

> > Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:1075
> > +    uri = g_strdup(m_src->priv->uri);
> 
> I think it'd make sense to use a GOwnPtr<gchar> here
> 
Done

> > Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:1089
> > +    uri = g_strdup(m_src->priv->uri);
> 
> Ditto
Done

Thanks everyone for the review.

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