From 1f68027942885d0e52b072df2f941999c2c8a5e0 Mon Sep 17 00:00:00 2001 From: Thibault Saunier Date: Mon, 15 Jul 2019 12:36:23 -0400 Subject: [PATCH] validate: Check that pull_range is called from the streaming thread `gst_pad_pull_range` should always be called from the streaming thread, we now check that when pull_range is called, and if the sinkpad calling the function has a GstTask with a running thread, the function is called from that thread. --- .../gst/validate/gst-validate-pad-monitor.c | 44 +++++++++++++++++++ .../gst/validate/gst-validate-pad-monitor.h | 1 + validate/gst/validate/gst-validate-report.c | 4 ++ validate/gst/validate/gst-validate-report.h | 2 + 4 files changed, 51 insertions(+) diff --git a/validate/gst/validate/gst-validate-pad-monitor.c b/validate/gst/validate/gst-validate-pad-monitor.c index 082f8bb71d..dd8f4c4792 100644 --- a/validate/gst/validate/gst-validate-pad-monitor.c +++ b/validate/gst/validate/gst-validate-pad-monitor.c @@ -2468,6 +2468,44 @@ gst_validate_pad_monitor_activatemode_func (GstPad * pad, GstObject * parent, return ret; } +static GstFlowReturn +gst_validate_pad_monitor_get_range_func (GstPad * pad, GstObject * parent, + guint64 offset, guint length, GstBuffer ** buffer) +{ + GstValidatePadMonitor *pad_monitor = _GET_PAD_MONITOR (pad); + + if (pad_monitor->get_range_func) { + GstPad *peer = gst_pad_get_peer (pad); + GstTask *task = NULL; + GThread *thread = NULL; + + if (peer) { + GST_OBJECT_LOCK (peer); + task = GST_PAD_TASK (peer); + if (task) { + GST_OBJECT_LOCK (task); + /* Only doing pointer comparison, no need to hold a ref */ + thread = task->thread; + GST_OBJECT_UNLOCK (task); + } + GST_OBJECT_UNLOCK (peer); + + if (thread && thread != g_thread_self ()) { + GST_VALIDATE_REPORT (pad_monitor, PULL_RANGE_FROM_WRONG_THREAD, + "Pulling from wrong thread, expected pad thread: %p, got %p", + task->thread, g_thread_self ()); + } + + gst_object_unref (peer); + } + + return pad_monitor->get_range_func (pad, parent, offset, length, buffer); + } + + return GST_FLOW_NOT_SUPPORTED; + +} + /* The interval between two buffer frequency checks */ #define BUF_FREQ_CHECK_INTERVAL (GST_SECOND) @@ -2909,6 +2947,7 @@ gst_validate_pad_monitor_do_setup (GstValidateMonitor * monitor) pad_monitor->event_full_func = GST_PAD_EVENTFULLFUNC (pad); pad_monitor->query_func = GST_PAD_QUERYFUNC (pad); pad_monitor->activatemode_func = GST_PAD_ACTIVATEMODEFUNC (pad); + pad_monitor->get_range_func = GST_PAD_GETRANGEFUNC (pad); if (GST_PAD_DIRECTION (pad) == GST_PAD_SINK) { pad_monitor->chain_func = GST_PAD_CHAINFUNC (pad); @@ -2936,6 +2975,11 @@ gst_validate_pad_monitor_do_setup (GstValidateMonitor * monitor) gst_pad_set_activatemode_function (pad, gst_validate_pad_monitor_activatemode_func); + if (GST_PAD_IS_SRC (pad)) { + gst_pad_set_getrange_function (pad, + gst_validate_pad_monitor_get_range_func); + } + gst_validate_reporter_set_name (GST_VALIDATE_REPORTER (monitor), g_strdup_printf ("%s:%s", GST_DEBUG_PAD_NAME (pad))); diff --git a/validate/gst/validate/gst-validate-pad-monitor.h b/validate/gst/validate/gst-validate-pad-monitor.h index b2fc6b8f5b..a50d5c6fde 100644 --- a/validate/gst/validate/gst-validate-pad-monitor.h +++ b/validate/gst/validate/gst-validate-pad-monitor.h @@ -63,6 +63,7 @@ struct _GstValidatePadMonitor { GstPadEventFullFunction event_full_func; GstPadQueryFunction query_func; GstPadActivateModeFunction activatemode_func; + GstPadGetRangeFunction get_range_func; gulong pad_probe_id; diff --git a/validate/gst/validate/gst-validate-report.c b/validate/gst/validate/gst-validate-report.c index de72824d3a..a4c45fe846 100644 --- a/validate/gst/validate/gst-validate-report.c +++ b/validate/gst/validate/gst-validate-report.c @@ -414,6 +414,10 @@ gst_validate_report_load_issues (void) REGISTER_VALIDATE_ISSUE (CRITICAL, G_LOG_CRITICAL, "We got a g_log critical issue", NULL); REGISTER_VALIDATE_ISSUE (ISSUE, G_LOG_ISSUE, "We got a g_log issue", NULL); + + REGISTER_VALIDATE_ISSUE (CRITICAL, PULL_RANGE_FROM_WRONG_THREAD, + "gst_pad_pull_range called from wrong thread", + _("gst_pad_pull_range has to be called from the sinkpad task thread.")); } gboolean diff --git a/validate/gst/validate/gst-validate-report.h b/validate/gst/validate/gst-validate-report.h index 95a672fd43..098b74b93a 100644 --- a/validate/gst/validate/gst-validate-report.h +++ b/validate/gst/validate/gst-validate-report.h @@ -81,6 +81,8 @@ typedef enum { #define FLOW_ERROR_WITHOUT_ERROR_MESSAGE _QUARK("buffer::flow-error-without-error-message") #define BUFFER_MISSING_DISCONT _QUARK("buffer::missing-discont") +#define PULL_RANGE_FROM_WRONG_THREAD _QUARK("threading::pull-range-from-wrong-thread") + #define CAPS_IS_MISSING_FIELD _QUARK("caps::is-missing-field") #define CAPS_FIELD_HAS_BAD_TYPE _QUARK("caps::field-has-bad-type") #define CAPS_EXPECTED_FIELD_NOT_FOUND _QUARK("caps::expected-field-not-found")