[webkit-reviews] review granted: [Bug 220091] [MSE][GStreamer] SIGSEV in webKitMediaSrcFreeStream : [Attachment 423827] Patch

[bugzilla-daemon at webkit.org]

Xabier Rodríguez Calvar <calvaris at igalia.com> has granted Philippe Normand
<pnormand at igalia.com>'s request for review:
Bug 220091: [MSE][GStreamer] SIGSEV in webKitMediaSrcFreeStream
https://bugs.webkit.org/show_bug.cgi?id=220091

Attachment 423827: Patch

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




--- Comment #17 from Xabier Rodríguez Calvar <calvaris at igalia.com> ---
Comment on attachment 423827
  --> https://bugs.webkit.org/attachment.cgi?id=423827
Patch

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

>>
Source/WebCore/platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamer.cpp:7
09
>> +	    stream->appsrc = gst_element_factory_make("appsrc", nullptr);
> 
> If we are going to keep a reference here, we should ref_sink it, otherwise in
line 720 we end up having the appsrc with 2 references but still the floating
flag just after evaluating the arguments before calling the function, which is
semantically wrong.
> 
> Well, after calling gst_bin_add things go to their place because the
gst_bin_add is going to perform a ref_sink so it is going to remove the
floating ref and the result is the same, but it is semantically wrong. You
should ref_sink here, avoid reffing below, and when the gst_bin_add does its
ref_sink, it will just increase the reference because the floating ref was
already sinked before.
> 
> Turning it into a GRefPtr, as I said above would solve this issue.

Ok, please solve this then and land.