Skip to content

Commit cd85f94

Browse files
committed
[gst1-plugins-base] appsink: Revert a patch that causes a memory leak in Netflix5
1 parent 7e79bd0 commit cd85f94

1 file changed

Lines changed: 173 additions & 0 deletions

File tree

Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
From 3a7e9b548e64212772ab4bfae783434969daa6d7 Mon Sep 17 00:00:00 2001
2+
From: =?UTF-8?q?Enrique=20Oca=C3=B1a=20Gonz=C3=A1lez?= <eocanha@igalia.com>
3+
Date: Fri, 21 Feb 2020 14:51:18 +0000
4+
Subject: [PATCH] Revert "appsink: Reuse sample object in pull_sample if
5+
possible"
6+
7+
This reverts commit d00e0b612dd1a8f9ad15aa78282defc8df21a86a.
8+
9+
This revert solves a GPU memory leak in Netflix5. The last played sample of
10+
every video was being leaked.
11+
---
12+
gst-libs/gst/app/gstappsink.c | 29 +++-----------------
13+
tests/check/elements/appsink.c | 49 ----------------------------------
14+
2 files changed, 4 insertions(+), 74 deletions(-)
15+
16+
diff --git a/gst-libs/gst/app/gstappsink.c b/gst-libs/gst/app/gstappsink.c
17+
index cac9821fa..5fe371341 100644
18+
--- a/gst-libs/gst/app/gstappsink.c
19+
+++ b/gst-libs/gst/app/gstappsink.c
20+
@@ -107,8 +107,6 @@ struct _GstAppSinkPrivate
21+
GstAppSinkCallbacks callbacks;
22+
gpointer user_data;
23+
GDestroyNotify notify;
24+
-
25+
- GstSample *sample;
26+
};
27+
28+
GST_DEBUG_CATEGORY_STATIC (app_sink_debug);
29+
@@ -460,7 +458,6 @@ gst_app_sink_init (GstAppSink * appsink)
30+
g_mutex_init (&priv->mutex);
31+
g_cond_init (&priv->cond);
32+
priv->queue = gst_queue_array_new (16);
33+
- priv->sample = gst_sample_new (NULL, NULL, NULL, NULL);
34+
35+
priv->emit_signals = DEFAULT_PROP_EMIT_SIGNALS;
36+
priv->max_buffers = DEFAULT_PROP_MAX_BUFFERS;
37+
@@ -496,10 +493,6 @@ gst_app_sink_dispose (GObject * obj)
38+
gst_buffer_replace (&priv->preroll_buffer, NULL);
39+
gst_caps_replace (&priv->preroll_caps, NULL);
40+
gst_caps_replace (&priv->last_caps, NULL);
41+
- if (priv->sample) {
42+
- gst_sample_unref (priv->sample);
43+
- priv->sample = NULL;
44+
- }
45+
g_mutex_unlock (&priv->mutex);
46+
47+
G_OBJECT_CLASS (parent_class)->dispose (obj);
48+
@@ -650,11 +643,6 @@ gst_app_sink_start (GstBaseSink * psink)
49+
priv->started = TRUE;
50+
gst_segment_init (&priv->preroll_segment, GST_FORMAT_TIME);
51+
gst_segment_init (&priv->last_segment, GST_FORMAT_TIME);
52+
- priv->sample = gst_sample_make_writable (priv->sample);
53+
- gst_sample_set_buffer (priv->sample, NULL);
54+
- gst_sample_set_buffer_list (priv->sample, NULL);
55+
- gst_sample_set_caps (priv->sample, NULL);
56+
- gst_sample_set_segment (priv->sample, NULL);
57+
g_mutex_unlock (&priv->mutex);
58+
59+
return TRUE;
60+
@@ -841,14 +829,10 @@ dequeue_buffer (GstAppSink * appsink)
61+
gst_event_parse_caps (event, &caps);
62+
GST_DEBUG_OBJECT (appsink, "activating caps %" GST_PTR_FORMAT, caps);
63+
gst_caps_replace (&priv->last_caps, caps);
64+
- priv->sample = gst_sample_make_writable (priv->sample);
65+
- gst_sample_set_caps (priv->sample, priv->last_caps);
66+
break;
67+
}
68+
case GST_EVENT_SEGMENT:
69+
gst_event_copy_segment (event, &priv->last_segment);
70+
- priv->sample = gst_sample_make_writable (priv->sample);
71+
- gst_sample_set_segment (priv->sample, &priv->last_segment);
72+
GST_DEBUG_OBJECT (appsink, "activated segment %" GST_SEGMENT_FORMAT,
73+
&priv->last_segment);
74+
break;
75+
@@ -881,7 +865,6 @@ restart:
76+
if (G_UNLIKELY (!priv->last_caps &&
77+
gst_pad_has_current_caps (GST_BASE_SINK_PAD (psink)))) {
78+
priv->last_caps = gst_pad_get_current_caps (GST_BASE_SINK_PAD (psink));
79+
- gst_sample_set_caps (priv->sample, priv->last_caps);
80+
GST_DEBUG_OBJECT (appsink, "activating pad caps %" GST_PTR_FORMAT,
81+
priv->last_caps);
82+
}
83+
@@ -1666,16 +1649,12 @@ gst_app_sink_try_pull_sample (GstAppSink * appsink, GstClockTime timeout)
84+
obj = dequeue_buffer (appsink);
85+
if (GST_IS_BUFFER (obj)) {
86+
GST_DEBUG_OBJECT (appsink, "we have a buffer %p", obj);
87+
- priv->sample = gst_sample_make_writable (priv->sample);
88+
- gst_sample_set_buffer_list (priv->sample, NULL);
89+
- gst_sample_set_buffer (priv->sample, GST_BUFFER_CAST (obj));
90+
- sample = gst_sample_ref (priv->sample);
91+
+ sample = gst_sample_new (GST_BUFFER_CAST (obj), priv->last_caps,
92+
+ &priv->last_segment, NULL);
93+
} else {
94+
GST_DEBUG_OBJECT (appsink, "we have a list %p", obj);
95+
- priv->sample = gst_sample_make_writable (priv->sample);
96+
- gst_sample_set_buffer (priv->sample, NULL);
97+
- gst_sample_set_buffer_list (priv->sample, GST_BUFFER_LIST_CAST (obj));
98+
- sample = gst_sample_ref (priv->sample);
99+
+ sample = gst_sample_new (NULL, priv->last_caps, &priv->last_segment, NULL);
100+
+ gst_sample_set_buffer_list (sample, GST_BUFFER_LIST_CAST (obj));
101+
}
102+
gst_mini_object_unref (obj);
103+
104+
diff --git a/tests/check/elements/appsink.c b/tests/check/elements/appsink.c
105+
index 9acbdcb9b..03adf063a 100644
106+
--- a/tests/check/elements/appsink.c
107+
+++ b/tests/check/elements/appsink.c
108+
@@ -666,54 +666,6 @@ GST_START_TEST (test_query_drain)
109+
110+
GST_END_TEST;
111+
112+
-GST_START_TEST (test_pull_sample_refcounts)
113+
-{
114+
- GstElement *sink;
115+
- GstBuffer *buffer;
116+
- GstSample *s1, *s2, *s3;
117+
-
118+
- sink = setup_appsink ();
119+
-
120+
- ASSERT_SET_STATE (sink, GST_STATE_PLAYING, GST_STATE_CHANGE_ASYNC);
121+
-
122+
- buffer = gst_buffer_new_and_alloc (4);
123+
- fail_unless (gst_pad_push (mysrcpad, buffer) == GST_FLOW_OK);
124+
-
125+
- s1 = gst_app_sink_pull_sample (GST_APP_SINK (sink));
126+
- fail_unless (s1 != NULL);
127+
- fail_unless (gst_buffer_get_size (gst_sample_get_buffer (s1)) == 4);
128+
- gst_sample_unref (s1);
129+
-
130+
- buffer = gst_buffer_new_and_alloc (6);
131+
- fail_unless (gst_pad_push (mysrcpad, buffer) == GST_FLOW_OK);
132+
- s2 = gst_app_sink_pull_sample (GST_APP_SINK (sink));
133+
- fail_unless (s2 != NULL);
134+
- fail_unless (gst_buffer_get_size (gst_sample_get_buffer (s2)) == 6);
135+
-
136+
- /* We unreffed s1, appsink should thus reuse the same sample,
137+
- * avoiding an extra allocation */
138+
- fail_unless (s1 == s2);
139+
-
140+
- buffer = gst_buffer_new_and_alloc (8);
141+
- fail_unless (gst_pad_push (mysrcpad, buffer) == GST_FLOW_OK);
142+
- s3 = gst_app_sink_pull_sample (GST_APP_SINK (sink));
143+
- fail_unless (s3 != NULL);
144+
- fail_unless (gst_buffer_get_size (gst_sample_get_buffer (s2)) == 6);
145+
- fail_unless (gst_buffer_get_size (gst_sample_get_buffer (s3)) == 8);
146+
-
147+
-
148+
- /* We didn't unref s2, appsink should thus have created a new sample */
149+
- fail_unless (s2 != s3);
150+
-
151+
- gst_sample_unref (s2);
152+
- gst_sample_unref (s3);
153+
-
154+
- ASSERT_SET_STATE (sink, GST_STATE_NULL, GST_STATE_CHANGE_SUCCESS);
155+
- cleanup_appsink (sink);
156+
-}
157+
-
158+
-GST_END_TEST;
159+
-
160+
static Suite *
161+
appsink_suite (void)
162+
{
163+
@@ -734,7 +686,6 @@ appsink_suite (void)
164+
tcase_add_test (tc_chain, test_query_drain);
165+
tcase_add_test (tc_chain, test_pull_preroll);
166+
tcase_add_test (tc_chain, test_do_not_care_preroll);
167+
- tcase_add_test (tc_chain, test_pull_sample_refcounts);
168+
169+
return s;
170+
}
171+
--
172+
2.17.1
173+

0 commit comments

Comments
 (0)