From 0c25863253ec00d91cf0d9d64edddd43bfa3592d Mon Sep 17 00:00:00 2001 From: Mark Nauwelaerts Date: Mon, 4 Jul 2011 14:30:09 +0200 Subject: [PATCH 1/5] jpegdec: avoid crashing on invalid input without components --- ext/jpeg/gstjpegdec.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/ext/jpeg/gstjpegdec.c b/ext/jpeg/gstjpegdec.c index 4d1be83f6a..23e8f23b8c 100644 --- a/ext/jpeg/gstjpegdec.c +++ b/ext/jpeg/gstjpegdec.c @@ -1346,12 +1346,16 @@ again: GST_WARNING_OBJECT (dec, "reading the header failed, %d", hdr_ok); } + GST_LOG_OBJECT (dec, "num_components=%d", dec->cinfo.num_components); + GST_LOG_OBJECT (dec, "jpeg_color_space=%d", dec->cinfo.jpeg_color_space); + + if (!dec->cinfo.num_components || !dec->cinfo.comp_info) + goto components_not_supported; + r_h = dec->cinfo.comp_info[0].h_samp_factor; r_v = dec->cinfo.comp_info[0].v_samp_factor; GST_LOG_OBJECT (dec, "r_h = %d, r_v = %d", r_h, r_v); - GST_LOG_OBJECT (dec, "num_components=%d", dec->cinfo.num_components); - GST_LOG_OBJECT (dec, "jpeg_color_space=%d", dec->cinfo.jpeg_color_space); if (dec->cinfo.num_components > 3) goto components_not_supported; @@ -1624,7 +1628,8 @@ drop_buffer: components_not_supported: { gst_jpeg_dec_set_error (dec, GST_FUNCTION, __LINE__, - "more components than supported: %d > 3", dec->cinfo.num_components); + "number of components not supported: %d (max 3)", + dec->cinfo.num_components); ret = GST_FLOW_ERROR; goto done; } From f2c8761a5041c2356acf95d0ac86631484f889a1 Mon Sep 17 00:00:00 2001 From: David Schleef Date: Mon, 4 Jul 2011 12:58:38 -0700 Subject: [PATCH 2/5] goom: Don't answer lantency queries before negotiation --- gst/goom/gstgoom.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/gst/goom/gstgoom.c b/gst/goom/gstgoom.c index 30a6c8ce66..c8e13ed65d 100644 --- a/gst/goom/gstgoom.c +++ b/gst/goom/gstgoom.c @@ -400,7 +400,7 @@ gst_goom_sink_event (GstPad * pad, GstEvent * event) static gboolean gst_goom_src_query (GstPad * pad, GstQuery * query) { - gboolean res; + gboolean res = FALSE; GstGoom *goom; goom = GST_GOOM (gst_pad_get_parent (pad)); @@ -415,6 +415,9 @@ gst_goom_src_query (GstPad * pad, GstQuery * query) GstClockTime our_latency; guint max_samples; + if (goom->rate == 0) + break; + if ((res = gst_pad_peer_query (goom->sinkpad, query))) { gst_query_parse_latency (query, &us_live, &min_latency, &max_latency); From 3589cee762e8bb6bf4da6bb52a58554b61de66a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Stadler?= Date: Wed, 29 Jun 2011 19:50:42 +0300 Subject: [PATCH 3/5] pulsesink: remove unused member variable and misleading log message Wim changed it in commit 8bfd80 so that pa_defer_ran is not read anywhere. The log message used to annotate a mainloop_wait call which is gone. --- ext/pulse/pulsesink.c | 5 ----- ext/pulse/pulsesink.h | 2 -- 2 files changed, 7 deletions(-) diff --git a/ext/pulse/pulsesink.c b/ext/pulse/pulsesink.c index 3464799f0f..2537f7558d 100644 --- a/ext/pulse/pulsesink.c +++ b/ext/pulse/pulsesink.c @@ -1006,7 +1006,6 @@ mainloop_enter_defer_cb (pa_mainloop_api * api, void *userdata) gst_element_post_message (GST_ELEMENT (pulsesink), message); /* signal the waiter */ - pulsesink->pa_defer_ran = TRUE; pa_threaded_mainloop_signal (mainloop, 0); } @@ -1023,7 +1022,6 @@ gst_pulseringbuffer_start (GstRingBuffer * buf) pa_threaded_mainloop_lock (mainloop); GST_DEBUG_OBJECT (psink, "scheduling stream status"); - psink->pa_defer_ran = FALSE; pa_mainloop_api_once (pa_threaded_mainloop_get_api (mainloop), mainloop_enter_defer_cb, psink); @@ -1083,7 +1081,6 @@ mainloop_leave_defer_cb (pa_mainloop_api * api, void *userdata) gst_message_set_stream_status_object (message, &val); gst_element_post_message (GST_ELEMENT (pulsesink), message); - pulsesink->pa_defer_ran = TRUE; pa_threaded_mainloop_signal (mainloop, 0); gst_object_unref (pulsesink); } @@ -1129,12 +1126,10 @@ cleanup: } GST_DEBUG_OBJECT (psink, "scheduling stream status"); - psink->pa_defer_ran = FALSE; gst_object_ref (psink); pa_mainloop_api_once (pa_threaded_mainloop_get_api (mainloop), mainloop_leave_defer_cb, psink); - GST_DEBUG_OBJECT (psink, "waiting for stream status"); pa_threaded_mainloop_unlock (mainloop); return res; diff --git a/ext/pulse/pulsesink.h b/ext/pulse/pulsesink.h index 0f75fcc648..e3cbbca4f8 100644 --- a/ext/pulse/pulsesink.h +++ b/ext/pulse/pulsesink.h @@ -64,8 +64,6 @@ struct _GstPulseSink gboolean mute:1; gboolean mute_set:1; - gboolean pa_defer_ran:1; - gint notify; /* atomic */ const gchar *pa_version; From f8456e2a1aaa9ae3a146ffb2a4510bff581eb88d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Stadler?= Date: Mon, 4 Jul 2011 08:58:14 +0300 Subject: [PATCH 4/5] pulsesink: small cleanup of copy-paste code --- ext/pulse/pulsesink.c | 39 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/ext/pulse/pulsesink.c b/ext/pulse/pulsesink.c index 2537f7558d..469d946d5d 100644 --- a/ext/pulse/pulsesink.c +++ b/ext/pulse/pulsesink.c @@ -2560,6 +2560,23 @@ gst_pulsesink_event (GstBaseSink * sink, GstEvent * event) return GST_BASE_SINK_CLASS (parent_class)->event (sink, event); } +static void +gst_pulsesink_release_mainloop (GstPulseSink * psink) +{ + if (!mainloop) + return; + + g_mutex_lock (pa_shared_resource_mutex); + mainloop_ref_ct--; + if (!mainloop_ref_ct) { + GST_INFO_OBJECT (psink, "terminating pa main loop thread"); + pa_threaded_mainloop_stop (mainloop); + pa_threaded_mainloop_free (mainloop); + mainloop = NULL; + } + g_mutex_unlock (pa_shared_resource_mutex); +} + static GstStateChangeReturn gst_pulsesink_change_state (GstElement * element, GstStateChange transition) { @@ -2602,17 +2619,7 @@ gst_pulsesink_change_state (GstElement * element, GstStateChange transition) GST_BASE_AUDIO_SINK (pulsesink)->provided_clock)); break; case GST_STATE_CHANGE_READY_TO_NULL: - if (mainloop) { - g_mutex_lock (pa_shared_resource_mutex); - mainloop_ref_ct--; - if (!mainloop_ref_ct) { - GST_INFO_OBJECT (element, "terminating pa main loop thread"); - pa_threaded_mainloop_stop (mainloop); - pa_threaded_mainloop_free (mainloop); - mainloop = NULL; - } - g_mutex_unlock (pa_shared_resource_mutex); - } + gst_pulsesink_release_mainloop (pulsesink); break; default: break; @@ -2633,15 +2640,7 @@ state_failure: if (transition == GST_STATE_CHANGE_NULL_TO_READY) { /* Clear the PA mainloop if baseaudiosink failed to open the ring_buffer */ g_assert (mainloop); - g_mutex_lock (pa_shared_resource_mutex); - mainloop_ref_ct--; - if (!mainloop_ref_ct) { - GST_INFO_OBJECT (element, "terminating pa main loop thread"); - pa_threaded_mainloop_stop (mainloop); - pa_threaded_mainloop_free (mainloop); - mainloop = NULL; - } - g_mutex_unlock (pa_shared_resource_mutex); + gst_pulsesink_release_mainloop (pulsesink); } return ret; } From ae87731de5dc9a1f39edd2b4dd12db30ad1ff0fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Stadler?= Date: Wed, 29 Jun 2011 20:59:26 +0300 Subject: [PATCH 5/5] pulsesink: prevent race condition causing ref leak Since commit 8bfd80, gst_pulseringbuffer_stop doesn't wait for the deferred call to be run before returning. This causes a race when READY->NULL is executed shortly after, which stops the mainloop. This leaks the element reference which is passed as userdata for the callback (introduced in commit 7cf996, bug #614765). The correct fix is to wait in READY->NULL for all outstanding calls to be fired (since libpulse doesn't provide a DestroyNotify for the userdata). We get rid of the reference passing from 7cf996 altogether, since finalization from the callback would anyways lead to a deadlock. Re-fixes bug #614765. --- ext/pulse/pulsesink.c | 18 +++++++++++++++--- ext/pulse/pulsesink.h | 2 ++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/ext/pulse/pulsesink.c b/ext/pulse/pulsesink.c index 469d946d5d..83ccdb2950 100644 --- a/ext/pulse/pulsesink.c +++ b/ext/pulse/pulsesink.c @@ -988,6 +988,7 @@ gst_pulseringbuffer_clear (GstRingBuffer * buf) pa_threaded_mainloop_unlock (mainloop); } +/* called from pulse with the mainloop lock */ static void mainloop_enter_defer_cb (pa_mainloop_api * api, void *userdata) { @@ -1005,7 +1006,8 @@ mainloop_enter_defer_cb (pa_mainloop_api * api, void *userdata) gst_element_post_message (GST_ELEMENT (pulsesink), message); - /* signal the waiter */ + g_return_if_fail (pulsesink->defer_pending); + pulsesink->defer_pending--; pa_threaded_mainloop_signal (mainloop, 0); } @@ -1022,6 +1024,7 @@ gst_pulseringbuffer_start (GstRingBuffer * buf) pa_threaded_mainloop_lock (mainloop); GST_DEBUG_OBJECT (psink, "scheduling stream status"); + psink->defer_pending++; pa_mainloop_api_once (pa_threaded_mainloop_get_api (mainloop), mainloop_enter_defer_cb, psink); @@ -1065,6 +1068,7 @@ gst_pulseringbuffer_pause (GstRingBuffer * buf) return res; } +/* called from pulse with the mainloop lock */ static void mainloop_leave_defer_cb (pa_mainloop_api * api, void *userdata) { @@ -1081,8 +1085,9 @@ mainloop_leave_defer_cb (pa_mainloop_api * api, void *userdata) gst_message_set_stream_status_object (message, &val); gst_element_post_message (GST_ELEMENT (pulsesink), message); + g_return_if_fail (pulsesink->defer_pending); + pulsesink->defer_pending--; pa_threaded_mainloop_signal (mainloop, 0); - gst_object_unref (pulsesink); } /* stop playback, we flush everything. */ @@ -1126,7 +1131,7 @@ cleanup: } GST_DEBUG_OBJECT (psink, "scheduling stream status"); - gst_object_ref (psink); + psink->defer_pending++; pa_mainloop_api_once (pa_threaded_mainloop_get_api (mainloop), mainloop_leave_defer_cb, psink); @@ -2566,6 +2571,13 @@ gst_pulsesink_release_mainloop (GstPulseSink * psink) if (!mainloop) return; + pa_threaded_mainloop_lock (mainloop); + while (psink->defer_pending) { + GST_DEBUG_OBJECT (psink, "waiting for stream status message emission"); + pa_threaded_mainloop_wait (mainloop); + } + pa_threaded_mainloop_unlock (mainloop); + g_mutex_lock (pa_shared_resource_mutex); mainloop_ref_ct--; if (!mainloop_ref_ct) { diff --git a/ext/pulse/pulsesink.h b/ext/pulse/pulsesink.h index e3cbbca4f8..e3029295ef 100644 --- a/ext/pulse/pulsesink.h +++ b/ext/pulse/pulsesink.h @@ -64,6 +64,8 @@ struct _GstPulseSink gboolean mute:1; gboolean mute_set:1; + guint defer_pending; + gint notify; /* atomic */ const gchar *pa_version;