[Webkit-unassigned] [Bug 150890] [GStreamer] Use MainThreadNotifier to send notifications to main thread in WebKitWebSourceGStreamer

[bugzilla-daemon at webkit.org]

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

--- Comment #5 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(In reply to comment #3)
> Comment on attachment 264788 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=264788&action=review
> 
> All the uses of MainThreadNotifier<>::notify() need updating, otherwise
> looks fine.
> 
> > Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:177
> > +    // NeedData
> 
> I usually write these in the same C-style they're declared in: need_data,
> enough_data, seek_data.
> 
> > Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:194
> > +        {
> > +            WTF::GMutexLocker<GMutex> locker(*GST_OBJECT_GET_LOCK(src));
> > +            if (!priv->paused)
> > +                return;
> > +
> > +            if (!isMainThread()) {
> > +                GRefPtr<WebKitWebSrc> protector(src);
> > +                priv->notifier.notify(MainThreadSourceNotification::NeedData, [protector] { webKitWebSrcNeedData(protector.get()); });
> > +                return;
> > +            }
> > +        }
> > +
> > +        webKitWebSrcNeedData(src);
> 
> Can webKitWebSrcNeedData(), webKitWebSrcEnoughData() and webKitWebSrcSeek()
> all be rewritten and used with the expectation that the
> GST_OBJECT_GET_LOCK() is already locked? That way the lock can be held
> throughout the lambda scope, and there's no need for the isMainThread()
> check at this level.

I thought it was possible but it's not that easy. We don't want to keep the lock until the notification happens in the main thread, so we need to take lock again before calling the callback in the lambda. 
The notifier is thread say, so we can just release the lock before calling notify()

> > Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:656
> > +        if (isMainThread())
> > +            webKitWebSrcStart(src);
> > +        else {
> > +            GRefPtr<WebKitWebSrc> protector(src);
> > +            priv->notifier.notify(MainThreadSourceNotification::Start, [protector] { webKitWebSrcStart(protector.get()); });
> > +        }
> 
> Similarly, webKitWebSrcStart() and webKitWebSrcStop() could just work with
> the assumption (or an actual run-time assertion) that the GstObject lock is
> locked.
> 
> > Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:833
> > +static void webKitWebSrcSeek(WebKitWebSrc* src)
> 
> If all the above is possible, don't forget to set up the lock here.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20151106/6690ef11/attachment.html>