GNOME Bugzilla – Bug 745197
pad: Don't fail latency query on unlinked pads
Last modified: 2015-02-26 19:37:42 UTC
Created attachment 297949 [details] [review] pad: Don't fail latency query on unlinked pads This is needed for elements like srtpdec where you might have an unlinked srcpad (RTCP srcpad in my case), but don't want the latency query to fail pipeline-wide. Perhaps we also need a mechanism to fire a gst_message_new_latency() if an element has one of its srcpads linked to in the PLAYING state.
Review of attachment 297949 [details] [review]: Makes sense, also the same bug in a few other places. At least input-selector, aggregator, adder, videomixer. Please go over all our code and grep for gst_query_set_latency() :) ::: gst/gstpad.c @@ +3102,3 @@ + /* No peer, so let's return some default */ + gst_query_set_latency (query, FALSE, 0, -1); + res = TRUE; Just doing nothing here is enough. The two lines above have no effect as the latency query "query" is just unreffed below, and res is never used again. The only way how things failed before was that they went into the branch above and caused g_value_set_boolean(ret, FALSE).
Created attachment 297950 [details] [review] pad: Don't fail latency query on unlinked pads A single unlinked pad can make the latency query fail across the pipeline, which is probably not desirable. Instead, we return a default anything goes value. Perhaps we should also be emitting a gst_message_new_latency() when a PLAYING element has one of its pads linked.
Comment on attachment 297950 [details] [review] pad: Don't fail latency query on unlinked pads Looks good, now we just need to do the same in the other places :)
For srtpdec, it should follow the internal links, so are you linking the srcpad, but not the matching sink pad? For this use-case, I don't think this change should be required. For the other n->1 elements (aggregator, input-selector, etc), they should probably post latency messages on the bus when the first buffer arrives on a new pad. Possibly toplevel GstBin should drop latency messages when they're not in the playing state if we start posting more of them. I've had issues recently with pipelines including audiomixer where the latency didn't get re-computed when modifying a playing pipeline.
(In reply to Olivier Crête from comment #4) > For srtpdec, it should follow the internal links, so are you linking the > srcpad, but not the matching sink pad? For this use-case, I don't think this > change should be required. Erf, yes, this is what I meant. It is required for this case, because the default logic runs a gst_pad_peer_query on the sinkpad, fails to find a peer, and returns FALSE. And yes, the other elements need fixing too. I'm looking at that.
(In reply to Olivier Crête from comment #4) > For the other n->1 elements (aggregator, input-selector, etc), they should > probably post latency messages on the bus when the first buffer arrives on a > new pad. Possibly toplevel GstBin should drop latency messages when they're > not in the playing state if we start posting more of them. > > I've had issues recently with pipelines including audiomixer where the > latency didn't get re-computed when modifying a playing pipeline. Same here, I worked around it by posting the messages myself when things were changed. We need to fix quite a few elements to post latency messages :) But that should be tracked in a different bug, can you open one?
input-selector, adder, videomixer, interleave, liveadder and aggregator now use the fixed latency handling.