From cbe7b52a70b5a17def7a86ea92a47ca74e193896 Mon Sep 17 00:00:00 2001 From: Wim Taymans Date: Fri, 15 May 2015 13:34:32 +0200 Subject: [PATCH] Improve error reporting Pass GError around for things that can fail and report the errors back to the client. Improve shutdown of pipeline when no clients are consuming. Make GStreamer elements handle all kinds of data and not just video because we can. --- src/gst/gstpvsink.c | 2 +- src/gst/gstpvsink.h | 2 - src/gst/gstpvsrc.c | 4 +- src/gst/gstpvsrc.h | 2 - src/modules/v4l2/pv-v4l2-source.c | 84 ++++++++++++++++++++++--------- src/server/pv-client-source.c | 26 ++++++++-- src/server/pv-client-source.h | 3 +- src/server/pv-client.c | 32 ++++++++---- src/server/pv-daemon.c | 13 +++-- src/server/pv-daemon.h | 3 +- src/server/pv-source-output.c | 13 ++++- src/server/pv-source.c | 16 ++++-- src/server/pv-source.h | 6 ++- 13 files changed, 145 insertions(+), 61 deletions(-) diff --git a/src/gst/gstpvsink.c b/src/gst/gstpvsink.c index 3ca79a20a..1a2cc6414 100644 --- a/src/gst/gstpvsink.c +++ b/src/gst/gstpvsink.c @@ -61,7 +61,7 @@ static GstStaticPadTemplate gst_pulsevideo_sink_template = GST_STATIC_PAD_TEMPLATE ("sink", GST_PAD_SINK, GST_PAD_ALWAYS, - GST_STATIC_CAPS (PVS_VIDEO_CAPS) + GST_STATIC_CAPS_ANY ); #define gst_pulsevideo_sink_parent_class parent_class diff --git a/src/gst/gstpvsink.h b/src/gst/gstpvsink.h index 189d1c263..955b5c6fe 100644 --- a/src/gst/gstpvsink.h +++ b/src/gst/gstpvsink.h @@ -23,8 +23,6 @@ #include #include -#include - #include #include #include diff --git a/src/gst/gstpvsrc.c b/src/gst/gstpvsrc.c index d208634e5..578127bdf 100644 --- a/src/gst/gstpvsrc.c +++ b/src/gst/gstpvsrc.c @@ -60,7 +60,7 @@ static GstStaticPadTemplate gst_pulsevideo_src_template = GST_STATIC_PAD_TEMPLATE ("src", GST_PAD_SRC, GST_PAD_ALWAYS, - GST_STATIC_CAPS (PVS_VIDEO_CAPS) + GST_STATIC_CAPS_ANY ); #define gst_pulsevideo_src_parent_class parent_class @@ -238,7 +238,7 @@ gst_pulsevideo_src_negotiate (GstBaseSrc * basesrc) thiscaps = gst_pad_query_caps (GST_BASE_SRC_PAD (basesrc), NULL); GST_DEBUG_OBJECT (basesrc, "caps of src: %" GST_PTR_FORMAT, thiscaps); /* nothing or anything is allowed, we're done */ - if (thiscaps == NULL || gst_caps_is_any (thiscaps)) + if (thiscaps == NULL) goto no_nego_needed; if (G_UNLIKELY (gst_caps_is_empty (thiscaps))) diff --git a/src/gst/gstpvsrc.h b/src/gst/gstpvsrc.h index c35942f85..0fe7344c2 100644 --- a/src/gst/gstpvsrc.h +++ b/src/gst/gstpvsrc.h @@ -23,8 +23,6 @@ #include #include -#include - #include #include #include diff --git a/src/modules/v4l2/pv-v4l2-source.c b/src/modules/v4l2/pv-v4l2-source.c index 825ae7d8b..1d6fb3175 100644 --- a/src/modules/v4l2/pv-v4l2-source.c +++ b/src/modules/v4l2/pv-v4l2-source.c @@ -34,8 +34,6 @@ struct _PvV4l2SourcePrivate GstElement *sink; GstCaps *possible_formats; - - GSocket *socket; }; G_DEFINE_TYPE (PvV4l2Source, pv_v4l2_source, PV_TYPE_SOURCE); @@ -91,6 +89,8 @@ setup_pipeline (PvV4l2Source *source) bus = gst_pipeline_get_bus (GST_PIPELINE (priv->pipeline)); gst_bus_add_watch (bus, bus_handler, source); gst_object_unref (bus); + + gst_element_set_state (priv->pipeline, GST_STATE_READY); } static GstCaps * @@ -101,7 +101,7 @@ collect_caps (PvSource * source, GstCaps *filter) GstQuery *query; query = gst_query_new_caps (filter); - gst_element_query (priv->src, query); + gst_element_query (priv->filter, query); gst_query_parse_caps_result (query, &res); gst_caps_ref (res); gst_query_unref (query); @@ -154,36 +154,57 @@ on_socket_notify (GObject *gobject, GSocket *socket; guint num_handles; GstCaps *caps; - GBytes *requested_format; + GBytes *requested_format, *format; g_object_get (gobject, "socket", &socket, NULL); g_print ("source socket %p\n", socket); if (socket == NULL) { - if (priv->socket) - g_signal_emit_by_name (priv->sink, "remove", priv->socket); + GSocket *prev_socket = g_object_get_data (gobject, "last-socket"); + if (prev_socket) { + g_signal_emit_by_name (priv->sink, "remove", prev_socket); + } } else { g_signal_emit_by_name (priv->sink, "add", socket); } - priv->socket = socket; - - g_object_get (gobject, "requested-format", &requested_format, NULL); - g_assert (requested_format != NULL); - g_object_set (gobject, "format", requested_format, NULL); - - g_print ("final format %s\n", g_bytes_get_data (requested_format, NULL)); - - caps = gst_caps_from_string (g_bytes_get_data (requested_format, NULL)); - g_assert (caps != NULL); - g_object_set (priv->filter, "caps", caps, NULL); - gst_caps_unref (caps); - g_bytes_unref (requested_format); + g_object_set_data (gobject, "last-socket", socket); g_object_get (priv->sink, "num-handles", &num_handles, NULL); + g_print ("num handles %d\n", num_handles); if (num_handles == 0) { gst_element_set_state (priv->pipeline, GST_STATE_READY); - } else { + g_object_set (priv->filter, "caps", NULL, NULL); + } else if (socket) { + /* what client requested */ + g_object_get (gobject, "requested-format", &requested_format, NULL); + g_assert (requested_format != NULL); + + if (num_handles == 1) { + /* first client, we set the requested format as the format */ + format = requested_format; + + /* set on the filter */ + caps = gst_caps_from_string (g_bytes_get_data (format, NULL)); + g_assert (caps != NULL); + g_object_set (priv->filter, "caps", caps, NULL); + gst_caps_unref (caps); + } else { + gchar *str; + + /* we already have a client, format is whatever is configured already */ + g_bytes_unref (requested_format); + + g_object_get (priv->filter, "caps", &caps, NULL); + str = gst_caps_to_string (caps); + format = g_bytes_new (str, strlen (str) + 1); + gst_caps_unref (caps); + } + /* this is what we use as the final format for the output */ + g_print ("final format %s\n", (gchar *) g_bytes_get_data (format, NULL)); + g_object_set (gobject, "format", format, NULL); + g_bytes_unref (format); + gst_element_set_state (priv->pipeline, GST_STATE_PLAYING); } } @@ -192,9 +213,9 @@ static PvSourceOutput * v4l2_create_source_output (PvSource *source, const gchar *client_path, GBytes *format_filter, - const gchar *prefix) + const gchar *prefix, + GError **error) { - PvV4l2SourcePrivate *priv = PV_V4L2_SOURCE (source)->priv; PvSourceOutput *output; GstCaps *caps, *filtered; gchar *str; @@ -205,8 +226,6 @@ v4l2_create_source_output (PvSource *source, if (caps == NULL) goto invalid_caps; - gst_element_set_state (priv->pipeline, GST_STATE_READY); - filtered = collect_caps (source, caps); if (filtered == NULL || gst_caps_is_empty (filtered)) goto no_format; @@ -215,7 +234,14 @@ v4l2_create_source_output (PvSource *source, g_print ("output filter %s\n", str); format_filter = g_bytes_new_take (str, strlen (str) + 1); - output = PV_SOURCE_CLASS (pv_v4l2_source_parent_class)->create_source_output (source, client_path, format_filter, prefix); + output = PV_SOURCE_CLASS (pv_v4l2_source_parent_class) + ->create_source_output (source, + client_path, + format_filter, + prefix, + error); + if (error == NULL) + return NULL; g_signal_connect (output, "notify::socket", (GCallback) on_socket_notify, source); @@ -224,10 +250,18 @@ v4l2_create_source_output (PvSource *source, /* ERRORS */ invalid_caps: { + if (error) + *error = g_error_new (G_IO_ERROR, + G_IO_ERROR_INVALID_DATA, + "Input filter data invalid"); return NULL; } no_format: { + if (error) + *error = g_error_new (G_IO_ERROR, + G_IO_ERROR_NOT_FOUND, + "No format available that matches input filter"); return NULL; } } diff --git a/src/server/pv-client-source.c b/src/server/pv-client-source.c index fc05fd5d5..f6bb74bfe 100644 --- a/src/server/pv-client-source.c +++ b/src/server/pv-client-source.c @@ -184,7 +184,8 @@ static PvSourceOutput * client_create_source_output (PvSource *source, const gchar *client_path, GBytes *format_filter, - const gchar *prefix) + const gchar *prefix, + GError **error) { PvClientSourcePrivate *priv = PV_CLIENT_SOURCE (source)->priv; PvSourceOutput *output; @@ -192,7 +193,15 @@ client_create_source_output (PvSource *source, /* propose format of input */ g_object_get (priv->input, "format", &format_filter, NULL); - output = PV_SOURCE_CLASS (pv_client_source_parent_class)->create_source_output (source, client_path, format_filter, prefix); + output = PV_SOURCE_CLASS (pv_client_source_parent_class) + ->create_source_output (source, + client_path, + format_filter, + prefix, + error); + + if (output == NULL) + return NULL; gst_element_set_state (priv->pipeline, GST_STATE_READY); @@ -246,7 +255,8 @@ PvSourceOutput * pv_client_source_get_source_input (PvClientSource *source, const gchar *client_path, GBytes *format_filter, - const gchar *prefix) + const gchar *prefix, + GError **error) { PvClientSourcePrivate *priv; @@ -254,7 +264,15 @@ pv_client_source_get_source_input (PvClientSource *source, priv = source->priv; if (priv->input == NULL) { - priv->input = PV_SOURCE_CLASS (pv_client_source_parent_class)->create_source_output (PV_SOURCE (source), client_path, format_filter, prefix); + priv->input = PV_SOURCE_CLASS (pv_client_source_parent_class) + ->create_source_output (PV_SOURCE (source), + client_path, + format_filter, + prefix, + error); + if (priv->input == NULL) + return NULL; + g_signal_connect (priv->input, "notify::socket", (GCallback) on_input_socket_notify, source); } return priv->input; diff --git a/src/server/pv-client-source.h b/src/server/pv-client-source.h index 377f19f2d..e0471bb5b 100644 --- a/src/server/pv-client-source.h +++ b/src/server/pv-client-source.h @@ -68,7 +68,8 @@ PvSource * pv_client_source_new (PvDaemon *daemon); PvSourceOutput * pv_client_source_get_source_input (PvClientSource *source, const gchar *client_path, GBytes *format_filter, - const gchar *prefix); + const gchar *prefix, + GError **error); G_END_DECLS diff --git a/src/server/pv-client.c b/src/server/pv-client.c index f29e1331b..d72d87390 100644 --- a/src/server/pv-client.c +++ b/src/server/pv-client.c @@ -130,14 +130,23 @@ handle_create_source_output (PvClient1 *interface, PvSourceOutput *output; const gchar *object_path, *sender; GBytes *formats; + GError *error = NULL; formats = g_bytes_new (arg_accepted_formats, strlen (arg_accepted_formats) + 1); - source = pv_daemon_find_source (priv->daemon, arg_source, priv->properties, formats); + source = pv_daemon_find_source (priv->daemon, + arg_source, + priv->properties, + formats, + &error); if (source == NULL) goto no_source; - output = pv_source_create_source_output (source, priv->object_path, formats, priv->object_path); + output = pv_source_create_source_output (source, + priv->object_path, + formats, + priv->object_path, + &error); if (output == NULL) goto no_output; @@ -154,14 +163,16 @@ handle_create_source_output (PvClient1 *interface, /* ERRORS */ no_source: { - g_dbus_method_invocation_return_dbus_error (invocation, - "org.pulsevideo.Error", "Can't create source"); + g_dbus_method_invocation_return_gerror (invocation, error); + g_clear_error (&error); + g_bytes_unref (formats); return TRUE; } no_output: { - g_dbus_method_invocation_return_dbus_error (invocation, - "org.pulsevideo.Error", "Can't create output"); + g_dbus_method_invocation_return_gerror (invocation, error); + g_clear_error (&error); + g_bytes_unref (formats); return TRUE; } } @@ -178,6 +189,7 @@ handle_create_source_input (PvClient1 *interface, PvSourceOutput *input; const gchar *source_input_path, *sender; GBytes *formats; + GError *error = NULL; source = pv_client_source_new (priv->daemon); if (source == NULL) @@ -192,7 +204,8 @@ handle_create_source_input (PvClient1 *interface, input = pv_client_source_get_source_input (PV_CLIENT_SOURCE (source), priv->object_path, formats, - priv->object_path); + priv->object_path, + &error); if (input == NULL) goto no_input; @@ -214,8 +227,9 @@ no_source: } no_input: { - g_dbus_method_invocation_return_dbus_error (invocation, - "org.pulsevideo.Error", "Can't create input"); + g_dbus_method_invocation_return_gerror (invocation, error); + g_clear_error (&error); + g_bytes_unref (formats); return TRUE; } } diff --git a/src/server/pv-daemon.c b/src/server/pv-daemon.c index c9ae45108..bfcd00994 100644 --- a/src/server/pv-daemon.c +++ b/src/server/pv-daemon.c @@ -78,8 +78,6 @@ client_name_appeared_handler (GDBusConnection *connection, SenderData *data = user_data; PvDaemonPrivate *priv = data->daemon->priv; - g_print ("client name appeared def: %p\n", g_main_context_get_thread_default ()); - g_hash_table_insert (priv->senders, data->sender, data); if (!g_strcmp0 (name, g_dbus_connection_get_unique_name (connection))) @@ -121,7 +119,6 @@ sender_data_new (PvDaemon *daemon, const gchar *sender) data->daemon = daemon; data->sender = g_strdup (sender); - g_print ("watch name def: %p\n", g_main_context_get_thread_default ()); g_print ("watch name %s %p\n", sender, data); data->id = g_bus_watch_name_on_connection (priv->connection, @@ -371,15 +368,21 @@ PvSource * pv_daemon_find_source (PvDaemon *daemon, const gchar *name, GVariant *props, - GBytes *format_filter) + GBytes *format_filter, + GError **error) { PvDaemonPrivate *priv; g_return_val_if_fail (PV_IS_DAEMON (daemon), NULL); priv = daemon->priv; - if (priv->sources == NULL) + if (priv->sources == NULL) { + if (error) + *error = g_error_new (G_IO_ERROR, + G_IO_ERROR_NOT_FOUND, + "No sources registered"); return NULL; + } return priv->sources->data; } diff --git a/src/server/pv-daemon.h b/src/server/pv-daemon.h index 6dacb4fc4..396a2dd6e 100644 --- a/src/server/pv-daemon.h +++ b/src/server/pv-daemon.h @@ -78,7 +78,8 @@ void pv_daemon_remove_source (PvDaemon *daemon, PvSource *source PvSource * pv_daemon_find_source (PvDaemon *daemon, const gchar *name, GVariant *props, - GBytes *format_filter); + GBytes *format_filter, + GError **error); G_END_DECLS diff --git a/src/server/pv-source-output.c b/src/server/pv-source-output.c index fe98cd01d..09f4b2c06 100644 --- a/src/server/pv-source-output.c +++ b/src/server/pv-source-output.c @@ -271,14 +271,22 @@ output_unregister_object (PvSourceOutput *output) pv_daemon_unexport (priv->daemon, priv->object_path); } +static void +pv_source_output_dispose (GObject * object) +{ + PvSourceOutput *output = PV_SOURCE_OUTPUT (object); + + output_unregister_object (output); + + G_OBJECT_CLASS (pv_source_output_parent_class)->dispose (object); +} + static void pv_source_output_finalize (GObject * object) { PvSourceOutput *output = PV_SOURCE_OUTPUT (object); PvSourceOutputPrivate *priv = output->priv; - output_unregister_object (output); - g_object_unref (priv->daemon); g_object_unref (priv->iface); g_free (priv->client_path); @@ -306,6 +314,7 @@ pv_source_output_class_init (PvSourceOutputClass * klass) g_type_class_add_private (klass, sizeof (PvSourceOutputPrivate)); + gobject_class->dispose = pv_source_output_dispose; gobject_class->finalize = pv_source_output_finalize; gobject_class->set_property = pv_source_output_set_property; gobject_class->get_property = pv_source_output_get_property; diff --git a/src/server/pv-source.c b/src/server/pv-source.c index bcc527f8a..756b7e0bf 100644 --- a/src/server/pv-source.c +++ b/src/server/pv-source.c @@ -210,7 +210,6 @@ default_release_source_output (PvSource *source, PvSourceOutput *output) return TRUE; } - static void pv_source_class_init (PvSourceClass * klass) { @@ -354,7 +353,8 @@ PvSourceOutput * pv_source_create_source_output (PvSource *source, const gchar *client_path, GBytes *format_filter, - const gchar *prefix) + const gchar *prefix, + GError **error) { PvSourceClass *klass; PvSourceOutput *res; @@ -363,10 +363,16 @@ pv_source_create_source_output (PvSource *source, klass = PV_SOURCE_GET_CLASS (source); - if (klass->create_source_output) - res = klass->create_source_output (source, client_path, format_filter, prefix); - else + if (klass->create_source_output) { + res = klass->create_source_output (source, client_path, format_filter, prefix, error); + } else { + if (error) { + *error = g_error_new (G_IO_ERROR, + G_IO_ERROR_NOT_SUPPORTED, + "Create SourceOutput not implemented"); + } res = NULL; + } return res; } diff --git a/src/server/pv-source.h b/src/server/pv-source.h index 78ae3e9e4..34d020090 100644 --- a/src/server/pv-source.h +++ b/src/server/pv-source.h @@ -71,7 +71,8 @@ struct _PvSourceClass { PvSourceOutput * (*create_source_output) (PvSource *source, const gchar *client_path, GBytes *format_filter, - const gchar *prefix); + const gchar *prefix, + GError **error); gboolean (*release_source_output) (PvSource *source, PvSourceOutput *output); }; @@ -90,7 +91,8 @@ void pv_source_report_error (PvSource *source, GError *err PvSourceOutput * pv_source_create_source_output (PvSource *source, const gchar *client_path, GBytes *format_filter, - const gchar *prefix); + const gchar *prefix, + GError **error); gboolean pv_source_release_source_output (PvSource *source, PvSourceOutput *output); G_END_DECLS