diff --git a/validate/gst/validate/gst-validate-pad-monitor.c b/validate/gst/validate/gst-validate-pad-monitor.c index 63d30a8a35..85ba3b0ef8 100644 --- a/validate/gst/validate/gst-validate-pad-monitor.c +++ b/validate/gst/validate/gst-validate-pad-monitor.c @@ -28,6 +28,7 @@ #include "gst-validate-internal.h" #include "gst-validate-pad-monitor.h" #include "gst-validate-element-monitor.h" +#include "gst-validate-pipeline-monitor.h" #include "gst-validate-reporter.h" #include #include @@ -1985,6 +1986,34 @@ gst_validate_pad_monitor_check_right_buffer (GstValidatePadMonitor * return ret; } +static void +gst_validate_pad_monitor_check_return (GstValidatePadMonitor * pad_monitor, + GstFlowReturn ret) +{ + GstValidateMonitor *parent = GST_VALIDATE_MONITOR (pad_monitor); + + if (ret != GST_FLOW_ERROR) + return; + + while (GST_VALIDATE_MONITOR_GET_PARENT (parent)) + parent = GST_VALIDATE_MONITOR_GET_PARENT (parent); + + if (GST_IS_VALIDATE_PIPELINE_MONITOR (parent)) { + GstValidatePipelineMonitor *m = GST_VALIDATE_PIPELINE_MONITOR (parent); + + GST_VALIDATE_MONITOR_LOCK (m); + if (m->got_error == FALSE) { + GST_VALIDATE_REPORT (pad_monitor, FLOW_ERROR_WITHOUT_ERROR_MESSAGE, + "Pad return GST_FLOW_ERROR but no GST_MESSAGE_ERROR was received on" + " the bus"); + + /* Only report it the first time */ + m->got_error = TRUE; + } + GST_VALIDATE_MONITOR_UNLOCK (m); + } +} + static GstFlowReturn gst_validate_pad_monitor_chain_func (GstPad * pad, GstObject * parent, GstBuffer * buffer) @@ -2008,6 +2037,8 @@ gst_validate_pad_monitor_chain_func (GstPad * pad, GstObject * parent, ret = pad_monitor->chain_func (pad, parent, buffer); + gst_validate_pad_monitor_check_return (pad_monitor, ret); + GST_VALIDATE_PAD_MONITOR_PARENT_LOCK (pad_monitor); GST_VALIDATE_MONITOR_LOCK (pad_monitor); diff --git a/validate/gst/validate/gst-validate-pipeline-monitor.c b/validate/gst/validate/gst-validate-pipeline-monitor.c index 4211785f40..2a2dbdf4b6 100644 --- a/validate/gst/validate/gst-validate-pipeline-monitor.c +++ b/validate/gst/validate/gst-validate-pipeline-monitor.c @@ -113,6 +113,9 @@ _bus_handler (GstBus * bus, GstMessage * message, GST_VALIDATE_REPORT (monitor, ERROR_ON_BUS, "Got error: %s -- Debug message: %s", err->message, debug); } + GST_VALIDATE_MONITOR_LOCK (monitor); + monitor->got_error = TRUE; + GST_VALIDATE_MONITOR_UNLOCK (monitor); g_error_free (err); g_free (debug); break; @@ -139,7 +142,7 @@ _bus_handler (GstBus * bus, GstMessage * message, if (monitor->print_pos_srcid && g_source_remove (monitor->print_pos_srcid)) monitor->print_pos_srcid = 0; - + monitor->got_error = FALSE; } } diff --git a/validate/gst/validate/gst-validate-pipeline-monitor.h b/validate/gst/validate/gst-validate-pipeline-monitor.h index da6128b2d5..6e50961960 100644 --- a/validate/gst/validate/gst-validate-pipeline-monitor.h +++ b/validate/gst/validate/gst-validate-pipeline-monitor.h @@ -57,6 +57,7 @@ struct _GstValidatePipelineMonitor { gulong element_added_id; guint print_pos_srcid; gboolean buffering; + gboolean got_error; }; /** diff --git a/validate/gst/validate/gst-validate-report.c b/validate/gst/validate/gst-validate-report.c index de3ef4e098..e234a720a5 100644 --- a/validate/gst/validate/gst-validate-report.c +++ b/validate/gst/validate/gst-validate-report.c @@ -195,6 +195,10 @@ gst_validate_report_load_issues (void) REGISTER_VALIDATE_ISSUE (ISSUE, BUFFER_AFTER_EOS, _("buffer was received after EOS"), _("a pad shouldn't receive any more buffers after it gets EOS")); + REGISTER_VALIDATE_ISSUE (WARNING, FLOW_ERROR_WITHOUT_ERROR_MESSAGE, + _("GST_FLOW_ERROR returned without posting an ERROR on the bus"), + _("Element MUST post a GST_MESSAGE_ERROR with GST_ELEMENT_ERROR before" + " returning GST_FLOW_ERROR")); REGISTER_VALIDATE_ISSUE (ISSUE, CAPS_IS_MISSING_FIELD, _("caps is missing a required field for its type"), diff --git a/validate/gst/validate/gst-validate-report.h b/validate/gst/validate/gst-validate-report.h index d9cfeafa81..807a217258 100644 --- a/validate/gst/validate/gst-validate-report.h +++ b/validate/gst/validate/gst-validate-report.h @@ -63,6 +63,7 @@ typedef enum { #define WRONG_FLOW_RETURN _QUARK("buffer::wrong-flow-return") #define BUFFER_AFTER_EOS _QUARK("buffer::after-eos") #define WRONG_BUFFER _QUARK("buffer::not-expected-one") +#define FLOW_ERROR_WITHOUT_ERROR_MESSAGE _QUARK("buffer::flow-error-without-error-message") #define CAPS_IS_MISSING_FIELD _QUARK("caps::is-missing-field") #define CAPS_FIELD_HAS_BAD_TYPE _QUARK("caps::field-has-bad-type") diff --git a/validate/tests/check/validate/padmonitor.c b/validate/tests/check/validate/padmonitor.c index e0bef46ccf..675a10d1b3 100644 --- a/validate/tests/check/validate/padmonitor.c +++ b/validate/tests/check/validate/padmonitor.c @@ -17,6 +17,7 @@ * Boston, MA 02110-1301, USA. */ +#include #include #include #include @@ -975,6 +976,99 @@ GST_START_TEST (buffer_timestamp_out_of_received_range) GST_END_TEST; + +GST_START_TEST (flow_error_without_message) +{ + GstElement *decoder = fake_decoder_new (); + GstElement *src = gst_element_factory_make ("fakesrc", NULL); + GstElement *sink = gst_element_factory_make ("fakesink", NULL); + GstBin *pipeline = GST_BIN (gst_pipeline_new ("validate-pipeline")); + GList *reports; + GstValidateRunner *runner; + GstValidateReport *report; + + fail_unless (g_setenv ("GST_VALIDATE_REPORTING_DETAILS", "all", TRUE)); + runner = _start_monitoring_bin (pipeline); + + gst_bin_add_many (pipeline, src, decoder, sink, NULL); + fail_unless (gst_element_link_many (src, decoder, sink, NULL)); + + + FAKE_DECODER (decoder)->return_value = GST_FLOW_ERROR; + ASSERT_SET_STATE (GST_ELEMENT (pipeline), GST_STATE_PLAYING, + GST_STATE_CHANGE_ASYNC); + + gst_element_get_state (GST_ELEMENT (pipeline), NULL, NULL, + GST_CLOCK_TIME_NONE); + + reports = gst_validate_runner_get_reports (runner); + + fail_unless (g_list_length (reports) >= 1); + report = reports->data; + fail_unless_equals_int (report->level, GST_VALIDATE_REPORT_LEVEL_WARNING); + fail_unless_equals_int (report->issue->issue_id, + FLOW_ERROR_WITHOUT_ERROR_MESSAGE); + g_list_free_full (reports, (GDestroyNotify) gst_validate_report_unref); + + gst_element_set_state (GST_ELEMENT (pipeline), GST_STATE_NULL); + + _stop_monitoring_bin (pipeline, runner); +} + +GST_END_TEST; + + +GST_START_TEST (flow_error_with_message) +{ + GstElement *decoder = fake_decoder_new (); + GstElement *src = gst_element_factory_make ("fakesrc", NULL); + GstElement *sink = gst_element_factory_make ("fakesink", NULL); + GstBin *pipeline = GST_BIN (gst_pipeline_new ("validate-pipeline")); + GList *reports; + GstValidateRunner *runner; + GstValidateReport *report; + const GError gerror = + { G_IO_ERROR, G_IO_ERROR_FAILED, (gchar *) "fake error" }; + + fail_unless (g_setenv ("GST_VALIDATE_REPORTING_DETAILS", "all", TRUE)); + runner = _start_monitoring_bin (pipeline); + + gst_bin_add_many (pipeline, src, decoder, sink, NULL); + fail_unless (gst_element_link_many (src, decoder, sink, NULL)); + + g_object_set (src, "is-live", TRUE, NULL); + + FAKE_DECODER (decoder)->return_value = GST_FLOW_ERROR; + + ASSERT_SET_STATE (GST_ELEMENT (pipeline), GST_STATE_PAUSED, + GST_STATE_CHANGE_NO_PREROLL); + + gst_element_post_message (decoder, + gst_message_new_error (GST_OBJECT (decoder), + (GError *) & gerror, "This is a fake error")); + + ASSERT_SET_STATE (GST_ELEMENT (pipeline), GST_STATE_PLAYING, + GST_STATE_CHANGE_ASYNC); + + gst_element_get_state (GST_ELEMENT (pipeline), NULL, NULL, + GST_CLOCK_TIME_NONE); + + reports = gst_validate_runner_get_reports (runner); + + assert_equals_int (g_list_length (reports), 1); + report = reports->data; + fail_unless_equals_int (report->issue->issue_id, ERROR_ON_BUS); + g_list_free_full (reports, (GDestroyNotify) gst_validate_report_unref); + + gst_element_set_state (GST_ELEMENT (pipeline), GST_STATE_NULL); + + _stop_monitoring_bin (pipeline, runner); +} + +GST_END_TEST; + + + static Suite * gst_validate_suite (void) { @@ -992,6 +1086,8 @@ gst_validate_suite (void) tcase_add_test (tc_chain, check_media_info); tcase_add_test (tc_chain, eos_without_segment); tcase_add_test (tc_chain, caps_events); + tcase_add_test (tc_chain, flow_error_without_message); + tcase_add_test (tc_chain, flow_error_with_message); gst_validate_deinit (); return s;