After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 746436 - tee: Add property that allows having all source pads unlinked
tee: Add property that allows having all source pads unlinked
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal enhancement
: 1.5.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-03-19 09:41 UTC by Jose Antonio Santos Cadenas
Modified: 2015-03-26 09:47 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
tee: Add allow-not-linked property (6.75 KB, patch)
2015-03-19 09:41 UTC, Jose Antonio Santos Cadenas
none Details | Review
tee: Add allow-not-linked property (7.07 KB, patch)
2015-03-26 09:16 UTC, Jose Antonio Santos Cadenas
committed Details | Review

Description Jose Antonio Santos Cadenas 2015-03-19 09:41:25 UTC
Created attachment 299791 [details] [review]
tee: Add allow-not-linked property

Attaches is a patch that implements that property
Comment 1 Wim Taymans 2015-03-19 09:50:11 UTC
It is not a good idea in general, things like:

  videotestsrc ! tee 

Would just consume 100% cpu for nothing. The idea of the not-linked
error is to make sure the pipeline doesn't keep on going when there
is nothing to do.
Comment 2 Jose Antonio Santos Cadenas 2015-03-19 09:58:21 UTC
Yes, I know that. This is why I added a property, for this to be optional.

We are developing applications that are linking and unlinking elements to tee while in PLAYING state and it's hard otherwise to take care of not letting the tee completely disconnected for a while. As we are working with live pipelines this won't consume 100% of CPU.

Nevertheless, what I'm forced to do now is this:

videotestsrc ! tee ! fakesink

That could also take 100% of CPU and requires more elements.
Comment 3 Sebastian Dröge (slomo) 2015-03-19 09:59:52 UTC
And the fakesink is also going to cause additional overhead that is generally not needed. Of course one can add/remove the fakesink dynamically, but that complicates the code very much.
Comment 4 Sebastian Dröge (slomo) 2015-03-19 10:01:23 UTC
Review of attachment 299791 [details] [review]:

::: tests/check/elements/tee.c
@@ +670,3 @@
+  src2 = gst_element_get_request_pad (tee, "src_%u");
+
+  fail_unless (gst_pad_push (srcpad, gst_buffer_ref (buffer)) == GST_FLOW_OK);

Shouldn't it return GST_FLOW_OK only if there is no source pad at all, but if there is at least one and it returns GST_FLOW_NOT_LINKED that should be returned instead?
Comment 5 Jose Antonio Santos Cadenas 2015-03-19 10:05:50 UTC
Review of attachment 299791 [details] [review]:

::: tests/check/elements/tee.c
@@ +670,3 @@
+  src2 = gst_element_get_request_pad (tee, "src_%u");
+
+  gst_pad_set_active (srcpad, TRUE);

This behavior is intentional, because if you created a pad and before you link it a buffer arrives you will also have an error and that's exactly what I try to avoid with this property.

The documentation of the property is this:
"Return GTS_FLOW_OK even if there are not source pads or all are unlinked"
Comment 6 Sebastian Dröge (slomo) 2015-03-19 10:11:47 UTC
Makes sense, as you have no way to block the srcpad before you requested it without first blocking the sinkpad (and the complete tee). So there's always some time when the srcpad can return NOT_LINKED.
Comment 7 Tim-Philipp Müller 2015-03-19 10:17:17 UTC
I had a use case for that as well at some point, e.g. if one wants to be able to suppress a GST_FLOW_ERROR from one branch (and remove/restart the branch without affecting the rest of the pipeline).

I'm wondering if it wouldn't be better to do this on a per-srcpad basis rather than a global property though?
Comment 8 Tim-Philipp Müller 2015-03-19 10:18:00 UTC
Well, I guess that's a different use case than yours then, because you can't set anything on the pads if there are not pads of course :)
Comment 9 Sebastian Dröge (slomo) 2015-03-20 08:34:49 UTC
Also you want to ignore *errors*, this here is only about ignoring tee not being linked downstream which can easily happen in dynamic pipelines :) I think those two things are complimentary.

What do people think about getting this merged for now, and then if someone has a need for it again to add a "always-ok" property to the pads that will allow to ignore errors? I can see how that would also be interesting and useful, i.e. when you don't want a branch of a tee to shut down the whole thing.
Comment 10 Jose Antonio Santos Cadenas 2015-03-25 14:14:56 UTC
Any more comments on this? Should I modify something?
Comment 11 Tim-Philipp Müller 2015-03-26 00:03:35 UTC
> What do people think about getting this merged for now

I'm fine with that. Only quibble I have is that there's no gtk-doc blurb documenting that the property is only 'Since: 1.6'.
Comment 12 Jose Antonio Santos Cadenas 2015-03-26 09:16:47 UTC
Created attachment 300339 [details] [review]
tee: Add allow-not-linked property

Attached documentation to notice that the property is only available since 1.6
Comment 13 Sebastian Dröge (slomo) 2015-03-26 09:47:42 UTC
commit 3e8e0a7065410cd161b05133ac79b57118dd14c2
Author: Jose Antonio Santos Cadenas <santoscadenas@gmail.com>
Date:   Thu Mar 19 10:36:11 2015 +0100

    tee: Add allow-not-linked property
    
    This property avoids not linked error when all the pads are unlinked
    or when there are no source pads. This is useful in dynamic pipelines
    where it can happen that for a short time there are no pads at all or
    all downstream pads are not linked yet.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=746436