[Bug 725187] Add new multiappsrc or dynappsrc element with multiple output streams

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=725187

--- Comment #53 from Seungha Yang <sh.yang at lge.com> ---
Thanks for quick but detailed review. 

(In reply to Julien Isorce from comment #49)
> Review of attachment 335415 [details] [review]:
> 
> ::: gst-libs/gst/multiapp/gstmultiappsrc.c
> @@ +236,3 @@
> +typedef struct _GhostPadInfo GhostPadInfo;
> +
> +struct _GstAppSrcGroup
> 
> This name is confusing, it sounds like a group of GstAppSrc whereas it is a
> hash map entry.
> Maybe do not start it with GstAppSrc, ex: SrcHolder or SrcEntry or ...
> 

I changed naming of GstAppSrcGroup to ChildAppSrcInfo like how urisourcebin
did.
And GhostPadInfo to OutputSlot.


> @@ +258,3 @@
> +                                 * Don't modify this group data by
> GstPadInfo */
> +  GstPad *target;               /* target (or to be linked) srcpad of
> appsrc */
> +};
> 
> Can't just "GstPad *target" be part of GstAppSrcGroup ?

variable for the output ghostpad was moved from GstAppSrcGroup to OutputSlot.
OutputSlot is mainly used for forwarding query or event.

> @@ +1483,3 @@
> +  gst_multi_appsrc_set_format_internal (multiappsrc, group);
> +
> +  group->multiappsrc = gst_object_ref (multiappsrc);
> 
> Is this one really necessary ? Since it is added to the multiappsrc bin
> anyway.

Without this, it's ok to access multiappsrc since a pad or an appsrc is a child
of the bin. But, I thought it might burden that calling gst_pad_get_parent() or
gst_element_get_parent () whenever we needed. How about your opinion? 

> @@ +2108,3 @@
> +  expose_group (multiappsrc, group, TRUE);
> +
> +  ret = gst_app_src_end_of_stream (GST_APP_SRC (group->appsrc));
> 
> why ?

push-discont-buffer/sample needs draining appsrc's data and reconfiguration of
appsrc. Please review attached design diagram.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.