Bug 234134 – [GStreamer][WebRTC] Huge memory leak

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Philippe Normand]

Initially reported in https://github.com/Igalia/meta-webkit/issues/335

Also happens with mock capture sources.

[Philippe Normand]

There's at least one leak here, https://github.com/WebKit/WebKit/blob/main/Source/WebCore/platform/graphics/gstreamer/MediaSampleGStreamer.cpp#L114

I don't have a fix yet though...

[Philippe Normand]

(In reply to Philippe Normand from comment #1)
> There's at least one leak here,
> https://github.com/WebKit/WebKit/blob/main/Source/WebCore/platform/graphics/
> gstreamer/MediaSampleGStreamer.cpp#L114
> 
> I don't have a fix yet though...

That's not the culprit actually... Patch incoming

[Philippe Normand]

Created attachment 446889 [details]
Patch

[Xabier Rodríguez Calvar]

Comment on attachment 446889 [details]
Patch

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

> Source/WebCore/platform/graphics/gstreamer/MediaSampleGStreamer.cpp:140
> +    const GstClockTime minimumDuration = 1000; // 1 us

period at the end.

> Source/WebCore/platform/mediastream/libwebrtc/gstreamer/GStreamerVideoFrameLibWebRTC.cpp:65
> +rtc::scoped_refptr<webrtc::VideoFrameBuffer> GStreamerVideoFrameLibWebRTC::create(GRefPtr<GstSample> sample)

Why did you remove the &&? We should either have const & or &&

> Source/WebCore/platform/mediastream/libwebrtc/gstreamer/GStreamerVideoFrameLibWebRTC.h:40
> +    GStreamerVideoFrameLibWebRTC(GRefPtr<GstSample> sample, GstVideoInfo info)

Ditto

> Source/WebCore/platform/mediastream/libwebrtc/gstreamer/GStreamerVideoFrameLibWebRTC.h:44
> +    static rtc::scoped_refptr<webrtc::VideoFrameBuffer> create(GRefPtr<GstSample>);

Ditto.

[Philippe Normand]

Comment on attachment 446889 [details]
Patch

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

>> Source/WebCore/platform/graphics/gstreamer/MediaSampleGStreamer.cpp:140
>> +    const GstClockTime minimumDuration = 1000; // 1 us
> 
> period at the end.

Why? This is not a sentence.

>> Source/WebCore/platform/mediastream/libwebrtc/gstreamer/GStreamerVideoFrameLibWebRTC.cpp:65
>> +rtc::scoped_refptr<webrtc::VideoFrameBuffer> GStreamerVideoFrameLibWebRTC::create(GRefPtr<GstSample> sample)
> 
> Why did you remove the &&? We should either have const & or &&

&& would transfer ownership iiuc? As mentioned in the changelog we can't do that. Maybe a const& though.

>> Source/WebCore/platform/mediastream/libwebrtc/gstreamer/GStreamerVideoFrameLibWebRTC.h:40
>> +    GStreamerVideoFrameLibWebRTC(GRefPtr<GstSample> sample, GstVideoInfo info)
> 
> Ditto

Ditto

>> Source/WebCore/platform/mediastream/libwebrtc/gstreamer/GStreamerVideoFrameLibWebRTC.h:44
>> +    static rtc::scoped_refptr<webrtc::VideoFrameBuffer> create(GRefPtr<GstSample>);
> 
> Ditto.

Ditto!.

[Xabier Rodríguez Calvar]

Comment on attachment 446889 [details]
Patch

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

>>> Source/WebCore/platform/mediastream/libwebrtc/gstreamer/GStreamerVideoFrameLibWebRTC.cpp:65
>>> +rtc::scoped_refptr<webrtc::VideoFrameBuffer> GStreamerVideoFrameLibWebRTC::create(GRefPtr<GstSample> sample)
>> 
>> Why did you remove the &&? We should either have const & or &&
> 
> && would transfer ownership iiuc? As mentioned in the changelog we can't do that. Maybe a const& though.

const & sounds good.

[Philippe Normand]

Created attachment 447347 [details]
Patch

[EWS]

Committed r287180 (245349@main): <https://commits.webkit.org/245349@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 447347 [details].

[Radar WebKit Bug Importer]

<rdar://problem/86627091>