Skip to content

Commit 9202c6f

Browse files
philnphiln
committed
[GStreamer] Fix various mediastream and pad probe buffer leaks reported by the GStreamer leak tracer
https://bugs.webkit.org/show_bug.cgi?id=298834 Reviewed by Xabier Rodriguez-Calvar. The most important leaks were in the buffer pad probes that modify buffers, the previous ones were not un-reffed. The other leaks were about the pipewire device manager and several other GStreamer objects not cleared before gst_deinit() was called. Driving-by in the video capturer modify existing caps instead of doing copies. * Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.cpp: (gst_pad_probe_info_set_buffer): * Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h: * Source/WebCore/platform/graphics/gstreamer/VideoFrameMetadataGStreamer.cpp: (videoFrameMetadataGetInfo): (webkitGstTraceProcessingTimeForElement): * Source/WebCore/platform/mediastream/gstreamer/GStreamerCaptureDeviceManager.h: * Source/WebCore/platform/mediastream/gstreamer/GStreamerCapturer.cpp: (WebCore::GStreamerCapturer::~GStreamerCapturer): (WebCore::GStreamerCapturer::tearDown): (WebCore::GStreamerCapturer::createSource): * Source/WebCore/platform/mediastream/gstreamer/GStreamerCapturer.h: * Source/WebCore/platform/mediastream/gstreamer/GStreamerIncomingTrackProcessor.cpp: (WebCore::GStreamerIncomingTrackProcessor::installRtpBufferPadProbe): * Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoCapturer.cpp: (WebCore::GStreamerVideoCapturer::tearDown): (WebCore::GStreamerVideoCapturer::setSize): (WebCore::GStreamerVideoCapturer::setFrameRate): * Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoCapturer.h: Canonical link: https://commits.webkit.org/299959@main
1 parent 278bcb0 commit 9202c6f

File tree

9 files changed

+54
-19
lines changed

9 files changed

+54
-19
lines changed

‎Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.cpp‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1898,4 +1898,14 @@ GstBuffer* gst_buffer_new_memdup(gconstpointer data, gsize size)
18981898
}
18991899
#endif
19001900

1901+
#if !GST_CHECK_VERSION(1, 27, 0)
1902+
void gst_pad_probe_info_set_buffer(GstPadProbeInfo* info, GstBuffer* buffer)
1903+
{
1904+
g_return_if_fail(info->type & GST_PAD_PROBE_TYPE_BUFFER);
1905+
1906+
gst_clear_mini_object(&info->data);
1907+
info->data = buffer;
1908+
}
1909+
#endif
1910+
19011911
#endif // USE(GSTREAMER)

‎Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -461,4 +461,8 @@ class GstIteratorAdaptor {
461461
GstBuffer* gst_buffer_new_memdup(gconstpointer data, gsize size);
462462
#endif
463463

464+
#if !GST_CHECK_VERSION(1, 27, 0)
465+
void gst_pad_probe_info_set_buffer(GstPadProbeInfo*, GstBuffer*);
466+
#endif
467+
464468
#endif // USE(GSTREAMER)

‎Source/WebCore/platform/graphics/gstreamer/VideoFrameMetadataGStreamer.cpp‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -61,14 +61,17 @@ GType videoFrameMetadataAPIGetType()
6161

6262
const GstMetaInfo* videoFrameMetadataGetInfo();
6363

64-
std::pair<GstBuffer*, VideoFrameMetadataGStreamer*> ensureVideoFrameMetadata(GstBuffer* buffer)
64+
static std::pair<GRefPtr<GstBuffer>, VideoFrameMetadataGStreamer*> ensureVideoFrameMetadata(GRefPtr<GstBuffer>&& buffer)
6565
{
66-
auto* meta = getInternalVideoFrameMetadata(buffer);
66+
auto* meta = getInternalVideoFrameMetadata(buffer.get());
6767
if (meta)
68-
return { buffer, meta };
68+
return { WTFMove(buffer), meta };
6969

70-
buffer = gst_buffer_make_writable(buffer);
71-
return { buffer, VIDEO_FRAME_METADATA_CAST(gst_buffer_add_meta(buffer, videoFrameMetadataGetInfo(), nullptr)) };
70+
IGNORE_WARNINGS_BEGIN("cast-align");
71+
auto modifiedBuffer = adoptGRef(gst_buffer_make_writable(buffer.leakRef()));
72+
IGNORE_WARNINGS_END;
73+
meta = VIDEO_FRAME_METADATA_CAST(gst_buffer_add_meta(modifiedBuffer.get(), videoFrameMetadataGetInfo(), nullptr));
74+
return { WTFMove(modifiedBuffer), meta };
7275
}
7376

7477
const GstMetaInfo* videoFrameMetadataGetInfo()
@@ -90,8 +93,8 @@ const GstMetaInfo* videoFrameMetadataGetInfo()
9093
if (!GST_META_TRANSFORM_IS_COPY(type))
9194
return FALSE;
9295

93-
auto* frameMeta = VIDEO_FRAME_METADATA_CAST(meta);
94-
auto [buf, copyMeta] = ensureVideoFrameMetadata(buffer);
96+
auto frameMeta = VIDEO_FRAME_METADATA_CAST(meta);
97+
auto copyMeta = VIDEO_FRAME_METADATA_CAST(gst_buffer_add_meta(buffer, videoFrameMetadataGetInfo(), nullptr));
9598
copyMeta->priv->videoSampleMetadata = frameMeta->priv->videoSampleMetadata;
9699

97100
Locker frameMetaLocker { frameMeta->priv->lock };
@@ -137,8 +140,8 @@ void webkitGstTraceProcessingTimeForElement(GstElement* element)
137140
static auto probeType = static_cast<GstPadProbeType>(GST_PAD_PROBE_TYPE_PUSH | GST_PAD_PROBE_TYPE_BUFFER);
138141

139142
gst_pad_add_probe(sinkPad.get(), probeType, [](GstPad*, GstPadProbeInfo* info, gpointer userData) -> GstPadProbeReturn {
140-
auto [modifiedBuffer, meta] = ensureVideoFrameMetadata(GST_PAD_PROBE_INFO_BUFFER(info));
141-
GST_PAD_PROBE_INFO_DATA(info) = modifiedBuffer;
143+
auto [modifiedBuffer, meta] = ensureVideoFrameMetadata(GRefPtr(GST_PAD_PROBE_INFO_BUFFER(info)));
144+
gst_pad_probe_info_set_buffer(info, modifiedBuffer.leakRef());
142145
Locker locker { meta->priv->lock };
143146
meta->priv->processingTimes.set(GST_ELEMENT_CAST(userData), std::make_pair(gst_util_get_timestamp(), GST_CLOCK_TIME_NONE));
144147
return GST_PAD_PROBE_OK;

‎Source/WebCore/platform/mediastream/gstreamer/GStreamerCaptureDeviceManager.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ class GStreamerCaptureDeviceManager : public CaptureDeviceManager, public Realti
5757
void unregisterCapturer(const GStreamerCapturer&);
5858
void stopCapturing(const String& persistentId);
5959

60-
void teardown();
60+
virtual void teardown();
6161

6262
private:
6363
void addDevice(GRefPtr<GstDevice>&&);
@@ -83,6 +83,7 @@ class GStreamerVideoCaptureDeviceManager final : public GStreamerCaptureDeviceMa
8383
friend class NeverDestroyed<GStreamerVideoCaptureDeviceManager>;
8484
public:
8585
static GStreamerVideoCaptureDeviceManager& singleton();
86+
8687
CaptureDevice::DeviceType deviceType() final { return CaptureDevice::DeviceType::Camera; }
8788
};
8889

‎Source/WebCore/platform/mediastream/gstreamer/GStreamerCapturer.cpp‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ GStreamerCapturer::GStreamerCapturer(const char* sourceFactory, GRefPtr<GstCaps>
6969

7070
GStreamerCapturer::~GStreamerCapturer()
7171
{
72-
tearDown();
72+
tearDown(true);
7373
}
7474

7575
void GStreamerCapturer::tearDown(bool disconnectSignals)
@@ -93,7 +93,9 @@ void GStreamerCapturer::tearDown(bool disconnectSignals)
9393
m_valve = nullptr;
9494
m_src = nullptr;
9595
m_capsfilter = nullptr;
96+
m_sink = nullptr;
9697
m_pipeline = nullptr;
98+
m_caps = nullptr;
9799
}
98100

99101
GStreamerCapturerObserver::~GStreamerCapturerObserver()
@@ -163,8 +165,9 @@ GstElement* GStreamerCapturer::createSource()
163165
gst_pad_add_probe(srcPad.get(), static_cast<GstPadProbeType>(GST_PAD_PROBE_TYPE_PUSH | GST_PAD_PROBE_TYPE_BUFFER), [](GstPad*, GstPadProbeInfo* info, gpointer) -> GstPadProbeReturn {
164166
VideoFrameTimeMetadata metadata;
165167
metadata.captureTime = MonotonicTime::now().secondsSinceEpoch();
166-
auto modifiedBuffer = webkitGstBufferSetVideoFrameTimeMetadata(GRefPtr(GST_PAD_PROBE_INFO_BUFFER(info)), metadata);
167-
GST_PAD_PROBE_INFO_DATA(info) = modifiedBuffer.leakRef();
168+
auto buffer = GST_PAD_PROBE_INFO_BUFFER(info);
169+
auto modifiedBuffer = webkitGstBufferSetVideoFrameTimeMetadata(GRefPtr(buffer), metadata);
170+
gst_pad_probe_info_set_buffer(info, modifiedBuffer.leakRef());
168171
return GST_PAD_PROBE_OK;
169172
}, nullptr, nullptr);
170173
}

‎Source/WebCore/platform/mediastream/gstreamer/GStreamerCapturer.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ class GStreamerCapturer : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr
5555
GStreamerCapturer(const char* sourceFactory, GRefPtr<GstCaps>&&, CaptureDevice::DeviceType);
5656
virtual ~GStreamerCapturer();
5757

58-
void tearDown(bool disconnectSignals = true);
58+
virtual void tearDown(bool disconnectSignals);
5959

6060
void addObserver(GStreamerCapturerObserver&);
6161
void removeObserver(GStreamerCapturerObserver&);

‎Source/WebCore/platform/mediastream/gstreamer/GStreamerIncomingTrackProcessor.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,8 @@ void GStreamerIncomingTrackProcessor::installRtpBufferPadProbe(const GRefPtr<Gst
316316
}
317317

318318
auto modifiedBuffer = webkitGstBufferSetVideoFrameTimeMetadata(GRefPtr(buffer), WTFMove(videoFrameTimeMetadata));
319-
GST_PAD_PROBE_INFO_DATA(info) = modifiedBuffer.leakRef();
319+
gst_pad_probe_info_set_buffer(info, modifiedBuffer.leakRef());
320+
320321
return GST_PAD_PROBE_OK;
321322
}, gst_caps_new_empty_simple("timestamp/x-ntp"), reinterpret_cast<GDestroyNotify>(gst_caps_unref));
322323
}

‎Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoCapturer.cpp‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,13 @@ GstElement* GStreamerVideoCapturer::createSource()
9090
return src;
9191
}
9292

93+
void GStreamerVideoCapturer::tearDown(bool disconnectSignals)
94+
{
95+
GStreamerCapturer::tearDown(disconnectSignals);
96+
if (disconnectSignals)
97+
m_videoSrcMIMETypeFilter = nullptr;
98+
}
99+
93100
GstElement* GStreamerVideoCapturer::createConverter()
94101
{
95102
if (isCapturingDisplay()) {
@@ -159,8 +166,9 @@ bool GStreamerVideoCapturer::setSize(const IntSize& size)
159166

160167
GST_INFO_OBJECT(m_pipeline.get(), "Setting size to %dx%d", width, height);
161168
m_size = size;
162-
m_caps = adoptGRef(gst_caps_copy(m_caps.get()));
163-
gst_caps_set_simple(m_caps.get(), "width", G_TYPE_INT, width, "height", G_TYPE_INT, height, nullptr);
169+
auto modifiedCaps = adoptGRef(gst_caps_make_writable(m_caps.leakRef()));
170+
gst_caps_set_simple(modifiedCaps.get(), "width", G_TYPE_INT, width, "height", G_TYPE_INT, height, nullptr);
171+
gst_caps_take(&m_caps.outPtr(), modifiedCaps.leakRef());
164172

165173
g_object_set(m_capsfilter.get(), "caps", m_caps.get(), nullptr);
166174
return true;
@@ -190,8 +198,9 @@ bool GStreamerVideoCapturer::setFrameRate(double frameRate)
190198
if (UNLIKELY(!m_capsfilter))
191199
return false;
192200

193-
m_caps = adoptGRef(gst_caps_copy(m_caps.get()));
194-
gst_caps_set_simple(m_caps.get(), "framerate", GST_TYPE_FRACTION, numerator, denominator, nullptr);
201+
auto modifiedCaps = adoptGRef(gst_caps_make_writable(m_caps.leakRef()));
202+
gst_caps_set_simple(modifiedCaps.get(), "framerate", GST_TYPE_FRACTION, numerator, denominator, nullptr);
203+
gst_caps_take(&m_caps.outPtr(), modifiedCaps.leakRef());
195204

196205
GST_INFO_OBJECT(m_pipeline.get(), "Setting framerate to %f fps", frameRate);
197206
g_object_set(m_capsfilter.get(), "caps", m_caps.get(), nullptr);

‎Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoCapturer.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,11 @@ class GStreamerVideoCapturer final : public GStreamerCapturer {
3838
GStreamerVideoCapturer(const char* sourceFactory, CaptureDevice::DeviceType);
3939
~GStreamerVideoCapturer() = default;
4040

41+
4142
GstElement* createSource() final;
43+
44+
void tearDown(bool disconnectSignals) final;
45+
4246
GstElement* createConverter() final;
4347
const char* name() final { return "Video"; }
4448

0 commit comments

Comments
 (0)