From 08aae8336b2807595d18686a930cdd8ddde4b6e3 Mon Sep 17 00:00:00 2001
From: Thiago Santos <thiago.sousa.santos@collabora.com>
Date: Wed, 7 Aug 2013 16:10:57 -0300
Subject: [PATCH] qa-runner: simplify runner to not hold refs to
 monitor/pipeline

The GstQaRunner is now a simple aggregator of reports that it receives
from monitors and filechecker. This allows it to be used in both
scenarios without  APIs that expect GstElement or Monitors, that are
only used on the pipeline monitoring QA tests.
---
 validate/gst/qa/gst-qa-monitor-preload.c | 15 +++----
 validate/gst/qa/gst-qa-runner.c          | 56 ++----------------------
 validate/gst/qa/gst-qa-runner.h          |  5 +--
 validate/gst/qa/gst-qa-scenario.c        |  8 ++--
 validate/gst/qa/gst-qa-scenario.h        |  1 +
 validate/gst/qa/gst-qa-transcoding.c     |  6 ++-
 validate/gst/qa/gst-qa.c                 |  6 ++-
 validate/gst/qa/qa.h                     |  2 +
 8 files changed, 28 insertions(+), 71 deletions(-)

diff --git a/validate/gst/qa/gst-qa-monitor-preload.c b/validate/gst/qa/gst-qa-monitor-preload.c
index 18faa82f9f7..bd8e7740397 100644
--- a/validate/gst/qa/gst-qa-monitor-preload.c
+++ b/validate/gst/qa/gst-qa-monitor-preload.c
@@ -22,10 +22,13 @@
 #include <gst/gst.h>
 #include <string.h>
 #include "gst-qa-runner.h"
+#include "gst-qa-monitor-factory.h"
 
 #define __USE_GNU
 #include <dlfcn.h>
 
+static GstQaRunner *runner = NULL;
+
 /*
  * Functions that wrap object creation so gst-qa can be used
  * to monitor 'standard' applications
@@ -34,15 +37,11 @@
 static void
 gst_qa_preload_wrap (GstElement * element)
 {
-  GstQaRunner *runner;
+  if (runner == NULL)
+    runner = gst_qa_runner_new ();
 
-  runner = gst_qa_runner_new (element);
-
-  /* TODO this will actually never unref the runner as it holds a ref
-   * to the element */
-  if (runner)
-    g_object_set_data_full ((GObject *) element, "qa-runner", runner,
-        g_object_unref);
+  /* the reference to the monitor is lost */
+  gst_qa_monitor_factory_create (GST_OBJECT_CAST (element), runner, NULL);
 }
 
 GstElement *
diff --git a/validate/gst/qa/gst-qa-runner.c b/validate/gst/qa/gst-qa-runner.c
index b82821863b1..6ef4aa4e508 100644
--- a/validate/gst/qa/gst-qa-runner.c
+++ b/validate/gst/qa/gst-qa-runner.c
@@ -22,7 +22,6 @@
 #include "gst-qa-runner.h"
 #include "gst-qa-report.h"
 #include "gst-qa-monitor-factory.h"
-#include "gst-qa-element-monitor.h"
 #include "gst-qa-override-registry.h"
 #include "gst-qa-scenario.h"
 
@@ -51,20 +50,13 @@ enum
 
 static guint _signals[LAST_SIGNAL] = { 0 };
 
-static gboolean gst_qa_runner_setup (GstQaRunner * runner);
-
 static void
 gst_qa_runner_dispose (GObject * object)
 {
   GstQaRunner *runner = GST_QA_RUNNER_CAST (object);
-  if (runner->pipeline)
-    gst_object_unref (runner->pipeline);
 
   g_slist_free_full (runner->reports, (GDestroyNotify) gst_qa_report_unref);
 
-  if (runner->monitor)
-    g_object_unref (runner->monitor);
-
   if (runner->scenario)
     g_object_unref (runner->scenario);
 
@@ -100,54 +92,11 @@ gst_qa_runner_init (GstQaRunner * runner)
 
 /**
  * gst_qa_runner_new:
- * @pipeline: (transfer-none): a #GstElement to run QA on
  */
 GstQaRunner *
-gst_qa_runner_new (GstElement * pipeline)
+gst_qa_runner_new (void)
 {
-  const gchar *scenario_name;
-  GstQaRunner *runner;
-
-  g_return_val_if_fail (pipeline != NULL, NULL);
-
-  runner = g_object_get_data ((GObject *) pipeline, "qa-runner");
-  if (runner) {
-    GST_WARNING_OBJECT (pipeline,
-        "Pipeline already has a qa-runner associated, returning it");
-
-    return gst_object_ref (runner);
-  }
-
-  runner = g_object_new (GST_TYPE_QA_RUNNER, NULL);
-  runner->pipeline = gst_object_ref (pipeline);
-
-  if ((scenario_name = g_getenv ("GST_QA_SCENARIO")))
-    runner->scenario = gst_qa_scenario_factory_create (runner, scenario_name);
-
-  g_object_set_data ((GObject *) pipeline, "qa-runner", runner);
-
-  if (!gst_qa_runner_setup (runner)) {
-    gst_object_unref (runner);
-    runner = NULL;
-  }
-
-  return runner;
-}
-
-static gboolean
-gst_qa_runner_setup (GstQaRunner * runner)
-{
-  GST_INFO_OBJECT (runner, "Starting QA Runner setup");
-  runner->monitor =
-      gst_qa_monitor_factory_create (GST_OBJECT_CAST (runner->pipeline), runner,
-      NULL);
-  if (runner->monitor == NULL) {
-    GST_WARNING_OBJECT (runner, "Setup failed");
-    return FALSE;
-  }
-
-  GST_DEBUG_OBJECT (runner, "Setup successful");
-  return TRUE;
+  return g_object_new (GST_TYPE_QA_RUNNER, NULL);
 }
 
 void
@@ -161,6 +110,7 @@ gst_qa_runner_add_report (GstQaRunner * runner, GstQaReport * report)
 guint
 gst_qa_runner_get_reports_count (GstQaRunner * runner)
 {
+  g_return_val_if_fail (runner != NULL, 0);
   return g_slist_length (runner->reports);
 }
 
diff --git a/validate/gst/qa/gst-qa-runner.h b/validate/gst/qa/gst-qa-runner.h
index 6d6c483b46b..14f11c62cdd 100644
--- a/validate/gst/qa/gst-qa-runner.h
+++ b/validate/gst/qa/gst-qa-runner.h
@@ -30,7 +30,6 @@
 G_BEGIN_DECLS
 
 /* forward declaration */
-typedef struct _GstQaMonitor GstQaMonitor;
 typedef struct _GstQaScenario GstQaScenario;
 
 #define GST_TYPE_QA_RUNNER			(gst_qa_runner_get_type ())
@@ -59,8 +58,6 @@ struct _GstQaRunner {
   gboolean       setup;
 
   /*< private >*/
-  GstElement    *pipeline;
-  GstQaMonitor *monitor;
   GstQaScenario *scenario;
 
   GSList *reports;
@@ -79,7 +76,7 @@ struct _GstQaRunnerClass {
 /* normal GObject stuff */
 GType		gst_qa_runner_get_type		(void);
 
-GstQaRunner *   gst_qa_runner_new               (GstElement * pipeline);
+GstQaRunner *   gst_qa_runner_new               (void);
 
 void            gst_qa_runner_add_report  (GstQaRunner * runner, GstQaReport * report);
 
diff --git a/validate/gst/qa/gst-qa-scenario.c b/validate/gst/qa/gst-qa-scenario.c
index 618d9869407..673d2e9490f 100644
--- a/validate/gst/qa/gst-qa-scenario.c
+++ b/validate/gst/qa/gst-qa-scenario.c
@@ -695,7 +695,7 @@ gst_qa_scenario_finalize (GObject * object)
 }
 
 GstQaScenario *
-gst_qa_scenario_factory_create (GstQaRunner * runner,
+gst_qa_scenario_factory_create (GstQaRunner * runner, GstElement * pipeline,
     const gchar * scenario_name)
 {
   GstBus *bus;
@@ -709,11 +709,11 @@ gst_qa_scenario_factory_create (GstQaRunner * runner,
     return NULL;
   }
 
-  scenario->priv->pipeline = gst_object_ref (runner->pipeline);
+  scenario->priv->pipeline = gst_object_ref (pipeline);
   gst_qa_reporter_set_name (GST_QA_REPORTER (scenario),
       g_strdup (scenario_name));
 
-  bus = gst_element_get_bus (runner->pipeline);
+  bus = gst_element_get_bus (pipeline);
   gst_bus_add_signal_watch (bus);
   g_signal_connect (bus, "message::async-done", (GCallback) async_done_cb,
       scenario);
@@ -724,7 +724,7 @@ gst_qa_scenario_factory_create (GstQaRunner * runner,
   g_print ("\n=========================================\n"
       "Running scenario %s on pipeline %s"
       "\n=========================================\n", scenario_name,
-      GST_OBJECT_NAME (runner->pipeline));
+      GST_OBJECT_NAME (pipeline));
 
   return scenario;
 }
diff --git a/validate/gst/qa/gst-qa-scenario.h b/validate/gst/qa/gst-qa-scenario.h
index 12337739f57..46c790a13bf 100644
--- a/validate/gst/qa/gst-qa-scenario.h
+++ b/validate/gst/qa/gst-qa-scenario.h
@@ -57,6 +57,7 @@ struct _GstQaScenario
 GType gst_qa_scenario_get_type (void);
 
 GstQaScenario * gst_qa_scenario_factory_create (GstQaRunner *runner,
+                                                GstElement *pipeline,
                                                 const gchar *scenario_name);
 
 G_END_DECLS
diff --git a/validate/gst/qa/gst-qa-transcoding.c b/validate/gst/qa/gst-qa-transcoding.c
index db7e0d30515..8edbaf92e7c 100644
--- a/validate/gst/qa/gst-qa-transcoding.c
+++ b/validate/gst/qa/gst-qa-transcoding.c
@@ -244,6 +244,7 @@ main (int argc, gchar ** argv)
 {
   GstBus *bus;
   GstQaRunner *runner;
+  GstQaMonitor *monitor;
   GOptionContext *ctx;
 
   GError *err = NULL;
@@ -304,7 +305,9 @@ main (int argc, gchar ** argv)
   /* Create the pipeline */
   create_transcoding_pipeline (argv[1], argv[2]);
 
-  runner = gst_qa_runner_new (pipeline);
+  runner = gst_qa_runner_new ();
+  monitor =
+      gst_qa_monitor_factory_create (GST_OBJECT_CAST (pipeline), runner, NULL);
   mainloop = g_main_loop_new (NULL, FALSE);
 
   if (!runner) {
@@ -328,6 +331,7 @@ main (int argc, gchar ** argv)
 exit:
   gst_element_set_state (pipeline, GST_STATE_NULL);
   g_main_loop_unref (mainloop);
+  g_object_unref (monitor);
   g_object_unref (runner);
   g_object_unref (pipeline);
 
diff --git a/validate/gst/qa/gst-qa.c b/validate/gst/qa/gst-qa.c
index 2d46912a5be..a05324fd552 100644
--- a/validate/gst/qa/gst-qa.c
+++ b/validate/gst/qa/gst-qa.c
@@ -57,6 +57,7 @@ main (int argc, gchar ** argv)
   GOptionContext *ctx;
   gchar **argvn;
   GstQaRunner *runner;
+  GstQaMonitor *monitor;
   GstBus *bus;
 
   ctx = g_option_context_new ("- runs QA tests for a pipeline.");
@@ -87,7 +88,9 @@ main (int argc, gchar ** argv)
   pipeline = (GstElement *) gst_parse_launchv ((const gchar **) argvn, &err);
   g_free (argvn);
 
-  runner = gst_qa_runner_new (pipeline);
+  runner = gst_qa_runner_new ();
+  monitor =
+      gst_qa_monitor_factory_create (GST_OBJECT_CAST (pipeline), runner, NULL);
   mainloop = g_main_loop_new (NULL, FALSE);
 
   if (!runner) {
@@ -111,6 +114,7 @@ main (int argc, gchar ** argv)
 exit:
   gst_element_set_state (pipeline, GST_STATE_NULL);
   g_main_loop_unref (mainloop);
+  g_object_unref (monitor);
   g_object_unref (runner);
   g_object_unref (pipeline);
   if (count)
diff --git a/validate/gst/qa/qa.h b/validate/gst/qa/qa.h
index a16e2634347..6fb2ba94a0d 100644
--- a/validate/gst/qa/qa.h
+++ b/validate/gst/qa/qa.h
@@ -3,4 +3,6 @@
  */
 
 #include <gst/qa/gst-qa-runner.h>
+#include <gst/qa/gst-qa-file-checker.h>
+#include <gst/qa/gst-qa-monitor-factory.h>