From 8650c47cd31001bb2afdc533da4b02aadf0652e2 Mon Sep 17 00:00:00 2001 From: Thibault Saunier Date: Mon, 15 Jun 2020 16:17:55 -0400 Subject: [PATCH] validate: Move action finalization to _set_done where it belongs gst_validate_action_set_done is the place where we should finalize the action, not in `execute_next`, this way we better handle printing interlaced action finalization too. Part-of: --- validate/gst/validate/gst-validate-scenario.c | 113 +++++++++--------- validate/gst/validate/gst-validate-scenario.h | 5 +- .../simple_interlaced_action.validatetest | 12 ++ 3 files changed, 71 insertions(+), 59 deletions(-) create mode 100644 validate/tests/launcher_tests/simple_interlaced_action.validatetest diff --git a/validate/gst/validate/gst-validate-scenario.c b/validate/gst/validate/gst-validate-scenario.c index 8218b1b095..c760f39854 100644 --- a/validate/gst/validate/gst-validate-scenario.c +++ b/validate/gst/validate/gst-validate-scenario.c @@ -477,8 +477,8 @@ gst_validate_action_return_get_name (GstValidateActionReturn r) return "IN_PROGRESS"; case GST_VALIDATE_EXECUTE_ACTION_NONE: return "NONE"; - case GST_VALIDATE_EXECUTE_ACTION_SKIP: - return "SKIP"; + case GST_VALIDATE_EXECUTE_ACTION_DONE: + return "DONE"; } g_assert_not_reached (); return "???"; @@ -2443,7 +2443,7 @@ gst_validate_execute_action (GstValidateActionType * action_type, if (action_type->prepare) { res = action_type->prepare (action); - if (res == GST_VALIDATE_EXECUTE_ACTION_SKIP) { + if (res == GST_VALIDATE_EXECUTE_ACTION_DONE) { gst_validate_print_action (action, NULL); return GST_VALIDATE_EXECUTE_ACTION_OK; } @@ -2636,55 +2636,10 @@ execute_next_action_full (GstValidateScenario * scenario, GstMessage * message) switch (act->priv->state) { case GST_VALIDATE_EXECUTE_ACTION_NONE: + case GST_VALIDATE_EXECUTE_ACTION_INTERLACED: break; case GST_VALIDATE_EXECUTE_ACTION_IN_PROGRESS: - GST_INFO_OBJECT (scenario, "Action %s:%d still running", - GST_VALIDATE_ACTION_FILENAME (act), GST_VALIDATE_ACTION_LINENO (act)); return G_SOURCE_CONTINUE; - case GST_VALIDATE_EXECUTE_ACTION_ERROR: - GST_VALIDATE_REPORT_ACTION (scenario, act, - SCENARIO_ACTION_EXECUTION_ERROR, "Action %s failed", act->type); - case GST_VALIDATE_EXECUTE_ACTION_ERROR_REPORTED: - case GST_VALIDATE_EXECUTE_ACTION_OK: - { - gchar *repeat = NULL; - - if (GST_VALIDATE_ACTION_N_REPEATS (act)) - repeat = - g_strdup_printf ("[%d/%d]", act->repeat, - GST_VALIDATE_ACTION_N_REPEATS (act)); - - gst_validate_printf (NULL, - "%*c⇨ Action %s '%s' %s (duration: %" GST_TIME_FORMAT ")\n\n", - (act->priv->subaction_level * 2) - 1, ' ', - gst_structure_get_name (act->priv->main_structure), - gst_validate_action_return_get_name (act->priv->state), - repeat ? repeat : "", GST_TIME_ARGS (act->priv->execution_duration)); - g_free (repeat); - - priv->actions = g_list_remove (priv->actions, act); - gst_validate_action_unref (act); - - if (!gst_validate_parse_next_action_playback_time (scenario)) { - gst_validate_error_structure (priv->actions ? priv-> - actions->data : NULL, - "Could not determine next action playback time!"); - - return G_SOURCE_REMOVE; - } - - - GST_INFO_OBJECT (scenario, "Action %" GST_PTR_FORMAT " is DONE now" - " executing next", act->structure); - - if (scenario->priv->actions) { - act = scenario->priv->actions->data; - } else { - _check_scenario_is_done (scenario); - act = NULL; - } - break; - } case GST_VALIDATE_EXECUTE_ACTION_ASYNC: if (GST_CLOCK_TIME_IS_VALID (act->priv->timeout)) { GstClockTime etime = @@ -2706,6 +2661,7 @@ execute_next_action_full (GstValidateScenario * scenario, GstMessage * message) return G_SOURCE_CONTINUE; default: + GST_ERROR ("State is %d", act->priv->state); g_assert_not_reached (); } @@ -2745,10 +2701,13 @@ execute_next_action_full (GstValidateScenario * scenario, GstMessage * message) priv->execute_actions_source_id = 0; SCENARIO_UNLOCK (scenario); + return G_SOURCE_CONTINUE; + case GST_VALIDATE_EXECUTE_ACTION_IN_PROGRESS: return G_SOURCE_CONTINUE; case GST_VALIDATE_EXECUTE_ACTION_INTERLACED: SCENARIO_LOCK (scenario); priv->interlaced_actions = g_list_append (priv->interlaced_actions, act); + priv->actions = g_list_remove (priv->actions, act); SCENARIO_UNLOCK (scenario); return gst_validate_scenario_execute_next_or_restart_looping (scenario); default: @@ -3485,7 +3444,7 @@ _execute_appsrc_push (GstValidateScenario * scenario, res = GST_VALIDATE_EXECUTE_ACTION_ASYNC; } else { gst_validate_printf (NULL, - "Pipeline is not ready to push buffers, interlacing appsrc-push action..."); + "Pipeline is not ready to push buffers, interlacing appsrc-push action...\n"); res = GST_VALIDATE_EXECUTE_ACTION_INTERLACED; } done: @@ -3640,7 +3599,6 @@ done: action->repeat = 0; GST_VALIDATE_ACTION_N_REPEATS (action) = repeat; - SCENARIO_LOCK (scenario); position = g_list_index (scenario->priv->actions, action); g_assert (position >= 0); for (i = 1; i < repeat; i++) { @@ -3650,7 +3608,7 @@ done: scenario->priv->actions = g_list_insert (scenario->priv->actions, copy, position + i); } - SCENARIO_UNLOCK (scenario); + return TRUE; } @@ -3821,6 +3779,8 @@ gst_validate_foreach_prepare (GstValidateAction * action) GST_VALIDATE_ACTION_N_REPEATS (subaction) = max; scenario->priv->actions = g_list_insert (scenario->priv->actions, subaction, i++); + + gst_structure_free (nstruct); } } g_list_free_full (actions, (GDestroyNotify) gst_structure_free); @@ -3829,7 +3789,7 @@ gst_validate_foreach_prepare (GstValidateAction * action) gst_structure_remove_field (action->structure, "actions"); gst_object_unref (scenario); - return GST_VALIDATE_EXECUTE_ACTION_SKIP; + return GST_VALIDATE_EXECUTE_ACTION_DONE; } static void @@ -5756,6 +5716,7 @@ _execute_stop (GstValidateScenario * scenario, GstValidateAction * action) static gboolean _action_set_done (GstValidateAction * action) { + gchar *repeat_message = NULL; JsonBuilder *jbuild; GstValidateScenario *scenario = gst_validate_action_get_scenario (action); @@ -5781,17 +5742,53 @@ _action_set_done (GstValidateAction * action) action->priv->pending_set_done = FALSE; switch (action->priv->state) { + case GST_VALIDATE_EXECUTE_ACTION_ERROR: + GST_VALIDATE_REPORT_ACTION (scenario, action, + SCENARIO_ACTION_EXECUTION_ERROR, "Action %s failed", action->type); case GST_VALIDATE_EXECUTE_ACTION_ASYNC: - case GST_VALIDATE_EXECUTE_ACTION_INTERLACED: case GST_VALIDATE_EXECUTE_ACTION_IN_PROGRESS: case GST_VALIDATE_EXECUTE_ACTION_NONE: - action->priv->state = GST_VALIDATE_EXECUTE_ACTION_OK; + case GST_VALIDATE_EXECUTE_ACTION_ERROR_REPORTED: + case GST_VALIDATE_EXECUTE_ACTION_OK: + { + scenario->priv->actions = g_list_remove (scenario->priv->actions, action); + + _check_scenario_is_done (scenario); + + if (!gst_validate_parse_next_action_playback_time (scenario)) { + gst_validate_error_structure (scenario->priv->actions ? scenario-> + priv->actions->data : NULL, + "Could not determine next action playback time!"); + } + + GST_INFO_OBJECT (scenario, "Action %" GST_PTR_FORMAT " is DONE now" + " executing next", action->structure); + break; - default: + } + case GST_VALIDATE_EXECUTE_ACTION_INTERLACED: break; } - gst_structure_free (action->structure); - action->structure = gst_structure_copy (action->priv->main_structure); + + if (GST_VALIDATE_ACTION_N_REPEATS (action)) + repeat_message = + g_strdup_printf ("[%d/%d]", action->repeat, + GST_VALIDATE_ACTION_N_REPEATS (action)); + + gst_validate_printf (NULL, + "%*c⇨ Action %s done '%s' %s (duration: %" GST_TIME_FORMAT ")\n\n", + (action->priv->subaction_level * 2) - 1, ' ', + gst_structure_get_name (action->priv->main_structure), + gst_validate_action_return_get_name (action->priv->state), + repeat_message ? repeat_message : "", + GST_TIME_ARGS (action->priv->execution_duration)); + g_free (repeat_message); + + if (action->priv->state != GST_VALIDATE_EXECUTE_ACTION_INTERLACED) + /* We took the 'scenario' reference... unreffing it now */ + gst_validate_action_unref (action); + + action->priv->state = GST_VALIDATE_EXECUTE_ACTION_DONE; gst_validate_scenario_execute_next_or_restart_looping (scenario); gst_object_unref (scenario); return G_SOURCE_REMOVE; diff --git a/validate/gst/validate/gst-validate-scenario.h b/validate/gst/validate/gst-validate-scenario.h index aeec38de3e..6a1238f760 100644 --- a/validate/gst/validate/gst-validate-scenario.h +++ b/validate/gst/validate/gst-validate-scenario.h @@ -47,6 +47,9 @@ typedef struct _GstValidateActionParameter GstValidateActionParameter; * GST_VALIDATE_EXECUTE_ACTION_ASYNC: * GST_VALIDATE_EXECUTE_ACTION_INTERLACED: * GST_VALIDATE_EXECUTE_ACTION_ERROR_REPORTED: + * GST_VALIDATE_EXECUTE_ACTION_IN_PROGRESS: + * GST_VALIDATE_EXECUTE_ACTION_NONE: + * GST_VALIDATE_EXECUTE_ACTION_DONE: */ typedef enum { @@ -57,7 +60,7 @@ typedef enum GST_VALIDATE_EXECUTE_ACTION_ERROR_REPORTED, GST_VALIDATE_EXECUTE_ACTION_IN_PROGRESS, GST_VALIDATE_EXECUTE_ACTION_NONE, - GST_VALIDATE_EXECUTE_ACTION_SKIP, + GST_VALIDATE_EXECUTE_ACTION_DONE, } GstValidateActionReturn; const gchar *gst_validate_action_return_get_name (GstValidateActionReturn r); diff --git a/validate/tests/launcher_tests/simple_interlaced_action.validatetest b/validate/tests/launcher_tests/simple_interlaced_action.validatetest new file mode 100644 index 0000000000..6ae4dae65e --- /dev/null +++ b/validate/tests/launcher_tests/simple_interlaced_action.validatetest @@ -0,0 +1,12 @@ +meta, + handles-states=true, + args = { + "appsrc name=src ! typefind ! fakesink", + } + +# Let push ourself into the pipeline :-) +appsrc-push, file-name="$(test_dir)/$(test_name).validatetest", target-element-name=src +priv_check-action-type-calls, type=appsrc-push, n=1 +appsrc-eos, target-element-name=src +pause; +stop