[webkit-reviews] review requested: [Bug 214150] [GStreamer][1.18] mediastreamsrc element hits assert in gst -core : [Attachment 403892] Patch

[bugzilla-daemon at webkit.org]

Philippe Normand <pnormand at igalia.com> has asked  for review:
Bug 214150: [GStreamer][1.18] mediastreamsrc element hits assert in gst -core
https://bugs.webkit.org/show_bug.cgi?id=214150

Attachment 403892: Patch

https://bugs.webkit.org/attachment.cgi?id=403892&action=review




--- Comment #4 from Philippe Normand <pnormand at igalia.com> ---
Comment on attachment 403892
  --> https://bugs.webkit.org/attachment.cgi?id=403892
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=403892&action=review

>> Source/WebCore/ChangeLog:9
>> +	    GRefPtr is doing more harm than good here.
> 
> You state you have a problem but you don't say which one. I guess you have
two extra refs, right? Your problem is that you're explicitly reffing the
object when you're implicitly creating the observers. GRefPtr constructor
(which is implicitly called) already ref_sinks the object you're passing,
hence, you either ref and adopt or you don't ref and let the implicit GRefPtr
constructor do the ref for you.

Of course I tried that already, and it doesn't work. I still get the critical
warning, for which the backtrace is not very useful anyway. The problem is that
we end up calling gst_object_ref_sink() from `template <> GstElement*
refGPtr<GstElement>(GstElement* ptr)` called from the src element constructor.
I don't think we can use a GRefPtr like this, here. If you disagree, I'm happy
to let you take ownership of this bug.