[Bug 725187] Add new multiappsrc or dynappsrc element with multiple output streams
GStreamer (bugzilla.gnome.org)
bugzilla at gnome.org
Tue Nov 25 13:34:54 PST 2014
https://bugzilla.gnome.org/show_bug.cgi?id=725187
GStreamer | gst-plugins-bad | git
Reynaldo H. Verdejo Pinochet <reynaldo> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #290675|none |needs-work
status| |
--- Comment #26 from Reynaldo H. Verdejo Pinochet <reynaldo at opendot.cl> 2014-11-25 21:34:48 UTC ---
(From update of attachment 290675)
Hi
> [..]
>+
>+/**
>+ * SECTION:element-multiappsrc
>+ *
>+ * MultiAppsrc provides multiple appsrc elements inside a single source bin
>+ * for handling separated audio, video and text stream.
stream/streams
>+ *
>+ * <refsect2>
>+ * <title>Usage</title>
>+ * <para>
>+ * A multiappsrc element is created by pipeline based on "multiappsrc://" protocal URI.
protocal/protocol
> [..]
>+/* must be called with OBJECT_LOCK */
>+static gboolean
>+setup_sources (GstMultiAppsrc * bin)
>+{
>+ GList *item;
>+ GstPadTemplate *pad_tmpl;
>+ gchar *padname;
>+ gboolean ret = FALSE;
>+
>+ pad_tmpl = gst_static_pad_template_get (&src_template);
>+
>+ for (item = bin->appsrc_list; item; item = g_list_next (item)) {
>+ GstAppSourceGroup *appsrc_group = (GstAppSourceGroup *) item->data;
>+ GstPad *srcpad = NULL;
>+
>+ gst_bin_add (GST_BIN_CAST (bin), appsrc_group->appsrc);
>+
>+ srcpad = gst_element_get_static_pad (appsrc_group->appsrc, "src");
>+ padname =
>+ g_strdup_printf ("src_%u", g_list_position (bin->appsrc_list, item));
>+ appsrc_group->srcpad =
>+ gst_ghost_pad_new_from_template (padname, srcpad, pad_tmpl);
>+ gst_pad_set_event_function (appsrc_group->srcpad,
>+ gst_multi_appsrc_handle_src_event);
>+ gst_pad_set_query_function (appsrc_group->srcpad,
>+ gst_multi_appsrc_handle_src_query);
>+
>+ gst_pad_set_active (appsrc_group->srcpad, TRUE);
>+ gst_element_add_pad (GST_ELEMENT_CAST (bin), appsrc_group->srcpad);
>+
>+ gst_object_unref (srcpad);
>+ g_free (padname);
>+
>+ ret = TRUE;
>+ }
Whats the point of unconditionally setting ret = TRUE on each
iteration? shouldn't you be checking some of the while loop's
functions return values like the one from gst_bin_add()?
>+ gst_object_unref (pad_tmpl);
>+
>+ if (ret) {
>+ GST_DEBUG_OBJECT (bin, "all appsrc elements are added");
>+ gst_element_no_more_pads (GST_ELEMENT_CAST (bin));
>+ }
Coming from the above, the if(ret) is superfluous unless something
can break the for() loop above with ret = FALSE, Unless all you wanted
to check was (bin->appsrc_list) at the beginning, in which case,
possibly a simpler if(!list){ warn(""); return FALSE } do{}while(next)
would do.
>+
>+ return ret;
>+}
>+
>+/* must be called with OBJECT_LOCK */
>+static void
>+remove_sources (GstMultiAppsrc * bin)
>+{
>+ GList *item;
>+
>+ for (item = bin->appsrc_list; item; item = g_list_next (item)) {
>+ GstAppSourceGroup *appsrc_group = (GstAppSourceGroup *) item->data;
>+
>+ GST_DEBUG_OBJECT (bin, "removing appsrc element and ghostpad");
>+
>+ gst_ghost_pad_set_target (GST_GHOST_PAD_CAST (appsrc_group->srcpad), NULL);
>+ gst_element_remove_pad (GST_ELEMENT_CAST (bin), appsrc_group->srcpad);
>+ appsrc_group->srcpad = NULL;
>+
>+ gst_element_set_state (appsrc_group->appsrc, GST_STATE_NULL);
>+ gst_bin_remove (GST_BIN_CAST (bin), appsrc_group->appsrc);
>+ appsrc_group->appsrc = NULL;
>+ }
>+
>+ g_list_free_full (bin->appsrc_list, g_free);
>+ bin->appsrc_list = NULL;
>+
>+ bin->n_source = 0;
>+}
Is there any guarantee funcs like _pad_set_target() and
_element_remove_pad() will never fail in this scenario?
Would the state at function exit be sane if they do?
Otherwise you might want to add some return checking and
make this one return a gboolean instead.
> [..]
>+static GstPadProbeReturn
>+_appsrc_event_probe (GstPad * pad, GstPadProbeInfo * info, gpointer udata)
>+{
>+ GstEvent *event = GST_PAD_PROBE_INFO_DATA (info);
>+ GstElement *appsrc = udata;
>+ const GstStructure *s;
>+ App *app = &s_app;
>+
>+ GST_DEBUG ("element:%s got an event:%s", GST_ELEMENT_NAME (appsrc),
>+ GST_EVENT_TYPE_NAME (event));
>+
>+ if (GST_EVENT_TYPE (event) == GST_EVENT_CUSTOM_UPSTREAM) {
>+ s = gst_event_get_structure (event);
>+ if (g_str_equal (gst_structure_get_name (s), "custom-upstream"))
>+ app->nb_received_event++;
>+ } else if (GST_EVENT_TYPE (event) == GST_EVENT_SEEK) {
>+ app->nb_received_event++;
>+ }
Would probably look cleaner as a switch() but it's
OK anyway.
> [..]
>+GST_START_TEST (test_appsrc_creation)
>+{
>+ GstElement *multiappsrc;
>+ guint pad_added_id = 0;
>+ gint n_added = 0;
>+ GstStateChangeReturn ret;
>+ GstElement *appsrc1 = NULL;
>+ GstElement *appsrc2 = NULL;
>+ GstIterator *iter;
>+ GValue item = { 0, };
>+ gboolean done = FALSE;
>+ gboolean exist_srcpad = FALSE;
>+ gint n_source = 0;
>+
>+ multiappsrc =
>+ gst_element_make_from_uri (GST_URI_SRC, "multiappsrc://", "source", NULL);
>+
>+ fail_unless (multiappsrc != NULL,
>+ "fail to create multiappsrc element by uri");
>+
>+ pad_added_id =
>+ g_signal_connect (multiappsrc, "pad-added",
>+ G_CALLBACK (pad_added_cb), &n_added);
>+
>+ g_signal_emit_by_name (multiappsrc, "new-appsrc", NULL, &appsrc1);
>+ g_signal_emit_by_name (multiappsrc, "new-appsrc", "thisisappsrc", &appsrc2);
>+
>+ /* user should do ref appsrc elements before using it */
>+ gst_object_ref (appsrc1);
>+ gst_object_ref (appsrc2);
>+
>+ fail_unless (appsrc1 && appsrc2, "fail to create appsrc element");
>+ fail_unless (GST_OBJECT_NAME (appsrc2)
>+ && "thisisappsrc", "fail to set user-defined name");
>+
>+ iter = gst_element_iterate_src_pads (multiappsrc);
>+ while (!done) {
>+ switch (gst_iterator_next (iter, &item)) {
>+ case GST_ITERATOR_OK:
>+ exist_srcpad = TRUE;
>+ done = TRUE;
>+ break;
>+ case GST_ITERATOR_RESYNC:
>+ gst_iterator_resync (iter);
>+ break;
>+ case GST_ITERATOR_ERROR:
>+ case GST_ITERATOR_DONE:
>+ done = TRUE;
>+ break;
>+ }
>+ }
>+
>+ fail_unless (!exist_srcpad,
>+ "srcpad is added too earlier, it should be added during state change");
>+
>+ g_value_unset (&item);
>+ gst_iterator_free (iter);
>+
>+ g_object_get (multiappsrc, "n-source", &n_source, NULL);
>+ fail_unless (n_source == 2, "the number of source element is not matched");
>+
>+ ret = gst_element_set_state (multiappsrc, GST_STATE_PAUSED);
>+ fail_unless (ret == GST_STATE_CHANGE_SUCCESS,
>+ "fail to state change to PAUSED");
>+ fail_unless (n_added == 2, "srcpad of multiappsrc does not added");
>+
>+ /* user should do unref appsrc elements before destroy pipeline */
>+ gst_object_unref (appsrc1);
>+ gst_object_unref (appsrc2);
>+
>+ /*
>+ In general, we don't need to call set_state(NULL) before unref element
>+ because if ref_count is zero by gst_object_unref, the state of the element
>+ automatically becomes NULL.
>+ However, in here, set_state(NULL) is called in order to clarify
automatically becomes NULL. However, ...
>+ how to handle appsrc from multiappsrc. Without internal appsrcs de-referencing,
>+ the appsrcs are remained as detached elements from multiappsrc.
are remained/remain
>+ This means that the upper layer cannot re-use appsrcs after calling set_state(NULL).
Breaking your lines at 80 characters wouldn't hurt.
> [..]
>+GST_START_TEST (test_appsrc_create_when_paused)
>+{
>+ GstElement *multiappsrc;
>+ guint pad_added_id = 0;
>+ gint n_added = 0;
>+ GstStateChangeReturn ret;
>+ GstElement *appsrc1 = NULL;
>+ GstElement *appsrc2 = NULL;
>+ gint n_source = 0;
>+
>+ multiappsrc =
>+ gst_element_make_from_uri (GST_URI_SRC, "multiappsrc://", "source", NULL);
>+
>+ pad_added_id =
>+ g_signal_connect (multiappsrc, "pad-added",
>+ G_CALLBACK (pad_added_cb), &n_added);
>+
>+ g_signal_emit_by_name (multiappsrc, "new-appsrc", "appsrc", &appsrc1);
>+
>+ /* user should do ref appsrc elements before using it */
>+ gst_object_ref (appsrc1);
>+
>+ fail_unless (appsrc1 != NULL, "failed to create appsrc element");
>+
>+ g_object_get (multiappsrc, "n-source", &n_source, NULL);
>+ fail_unless (n_source == 1, "the number of source element is not matched");
>+
>+ ret = gst_element_set_state (multiappsrc, GST_STATE_PAUSED);
>+ fail_unless (ret == GST_STATE_CHANGE_SUCCESS,
>+ "fail to state change to PAUSED");
>+
>+ /* Try to generate a appsrc element when state is paused.
>+ * If it is genetated, it is not make sence.
>+ * Because, in this time, configuratoin of all appsrc elements in multiappsrc bin is already done.
>+ * Thus, you should call "new-appsrc" singal action before paused state.
>+ */
Ditto.
> [...]
>+GST_START_TEST (test_appsrc_upstream_event)
>+{
>+ App *app = &s_app;
>+ guint pad_added_id = 0;
>+ gint n_added = 0;
>+ GstStateChangeReturn ret;
>+ gint n_source = 0;
>+ gint count = 0;
>+ GstEvent *event;
>+
>+ GST_DEBUG ("Creating pipeline");
>+ app->pipeline = gst_pipeline_new ("pipeline");
>+ fail_if (app->pipeline == NULL);
>+
>+ GST_DEBUG ("Creating multiappsrc");
>+ app->multiappsrc =
>+ gst_element_make_from_uri (GST_URI_SRC, "multiappsrc://", "source", NULL);
>+ fail_unless (app->multiappsrc != NULL,
>+ "fail to create multiappsrc element by uri");
>+ gst_bin_add (GST_BIN (app->pipeline), app->multiappsrc);
>+
>+ GST_DEBUG ("Creating fakesink");
>+ for (count = 0; count < NUM_APPSRC; count++) {
>+ app->fakesink[count] = gst_element_factory_make ("fakesink", NULL);
>+ fail_if (app->fakesink[count] == NULL);
>+ g_object_set (G_OBJECT (app->fakesink[count]), "sync", TRUE, NULL);
>+ gst_bin_add (GST_BIN (app->pipeline), app->fakesink[count]);
>+ }
>+
>+ pad_added_id =
>+ g_signal_connect (app->multiappsrc, "pad-added",
>+ G_CALLBACK (pad_added_cb), &n_added);
>+
>+ GST_DEBUG ("Creating appsrc");
>+ for (count = 0; count < NUM_APPSRC; count++) {
>+ gchar *appsrc_name;
>+ appsrc_name = g_strdup_printf ("foot%d", count);
>+ g_signal_emit_by_name (app->multiappsrc, "new-appsrc", appsrc_name,
>+ &app->appsrc[count]);
>+ /* user should do ref appsrc elements before using it */
>+ gst_object_ref (app->appsrc[count]);
>+
>+ fail_unless (app->appsrc[count] != NULL, "failed to create appsrc element");
>+ g_free (appsrc_name);
>+ }
>+
>+ g_object_get (app->multiappsrc, "n-source", &n_source, NULL);
>+ fail_unless (n_source == NUM_APPSRC,
>+ "the number of source element is not matched");
>+
>+ ret = gst_element_set_state (app->pipeline, GST_STATE_PAUSED);
>+ fail_unless (ret == GST_STATE_CHANGE_ASYNC);
>+
>+ fail_unless (n_added == NUM_APPSRC, "srcpad of multiappsrc does not added");
>+
>+ ret = gst_element_set_state (app->pipeline, GST_STATE_PLAYING);
>+ fail_unless (ret == GST_STATE_CHANGE_ASYNC);
>+
>+ GST_DEBUG ("Sending upstream custom event");
>+ app->nb_received_event = 0;
>+ event = gst_event_new_custom (GST_EVENT_CUSTOM_UPSTREAM,
>+ gst_structure_new_empty ("custom-upstream"));
>+
>+ gst_element_send_event (app->pipeline, event);
>+ fail_unless (app->nb_received_event == NUM_APPSRC,
>+ "the number of received events are not matched");
>+
>+ /*
>+ * First of all, multiappsrc handle a seek-event that it send all of appsrc elements.
>+ * Try to send seek-event in a fakesink.
>+ * Received seek-events in all of appsrc elements should be 10.
>+ */
Same. Check elsewhere too please.
>+ GST_DEBUG ("Sending flush seek event to fakesink");
>+ for (count = 0; count < NUM_APPSRC; count++) {
>+ app->nb_received_event = 0;
>+ gst_element_seek (app->fakesink[count], 1.0, GST_FORMAT_TIME,
>+ GST_SEEK_FLAG_FLUSH, GST_SEEK_TYPE_SET, 0, GST_SEEK_TYPE_SET, -1);
>+
>+ fail_unless (app->nb_received_event == NUM_APPSRC,
>+ "the number of received events are not matched");
>+ }
>+
>+ /*
>+ * Try to send seek-event in pipeline.
>+ * In this case, received seek-event in all of appsrc elements should be 100.
>+ */
>+ GST_DEBUG ("Sending flush seek event to pipeline");
Please see bellow
>+ app->nb_received_event = 0;
>+ gst_element_seek (app->pipeline, 1.0, GST_FORMAT_TIME,
>+ GST_SEEK_FLAG_FLUSH, GST_SEEK_TYPE_SET, 0, GST_SEEK_TYPE_SET, -1);
>+
>+ fail_unless (app->nb_received_event == (NUM_APPSRC * 10),
>+ "the number of received events are not matched");
>+
>+ GST_DEBUG ("Release pipeline");
Please see bellow
>+ /* user should do unref appsrc elements before destroy pipeline */
>+ for (count = 0; count < NUM_APPSRC; count++)
>+ gst_object_unref (app->appsrc[count]);
>+
>+ gst_element_set_state (app->pipeline, GST_STATE_NULL);
>+ g_signal_handler_disconnect (app->multiappsrc, pad_added_id);
>+ gst_object_unref (app->pipeline);
>+}
>+
>+GST_END_TEST;
>+
>+static gboolean
>+bus_message (GstBus * bus, GstMessage * message, App * app)
>+{
>+ GST_DEBUG ("got message %s \n",
>+ gst_message_type_get_name (GST_MESSAGE_TYPE (message)));
Please see bellow
> [..]
>+GST_START_TEST (test_appsrc_eos)
>+{
>+ App *app = &s_app;
>+ guint pad_added_id = 0;
>+ gint n_added = 0;
>+ GstStateChangeReturn ret;
>+ gint n_source = 0;
>+ gint count = 0;
>+ GstBus *bus;
>+
>+ GST_DEBUG ("Creating pipeline");
>+ app->pipeline = gst_pipeline_new ("pipeline");
>+ fail_if (app->pipeline == NULL);
>+
>+ app->loop = g_main_loop_new (NULL, FALSE);
>+
>+ bus = gst_pipeline_get_bus (GST_PIPELINE (app->pipeline));
>+
>+ /* add watch for messages */
>+ gst_bus_add_watch (bus, (GstBusFunc) bus_message, app);
>+ gst_object_unref (bus);
>+
>+ GST_DEBUG ("Creating multiappsrc");
>+ app->multiappsrc =
>+ gst_element_make_from_uri (GST_URI_SRC, "multiappsrc://", "source", NULL);
>+ fail_unless (app->multiappsrc != NULL,
>+ "fail to create multiappsrc element by uri");
>+ gst_bin_add (GST_BIN (app->pipeline), app->multiappsrc);
>+
>+ GST_DEBUG ("Creating fakesink");
>+ for (count = 0; count < NUM_APPSRC; count++) {
>+ app->fakesink[count] = gst_element_factory_make ("fakesink", NULL);
>+ fail_if (app->fakesink[count] == NULL);
>+ g_object_set (G_OBJECT (app->fakesink[count]), "sync", TRUE, NULL);
>+ gst_bin_add (GST_BIN (app->pipeline), app->fakesink[count]);
>+ }
>+
>+ pad_added_id =
>+ g_signal_connect (app->multiappsrc, "pad-added",
>+ G_CALLBACK (pad_added_cb), &n_added);
>+
>+ GST_DEBUG ("Creating appsrc");
This and the above ones fail the "is not the expected default behavior"
so should probably be _INFO () instead
Also, please add bug number to your commit message in your next
iteration.
Thanks a lot
--
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.
More information about the gstreamer-bugs
mailing list