GNOME Bugzilla – Bug 757653
pad: add flag to disable reconfigure event on pad linking
Last modified: 2018-05-14 22:42:52 UTC
Linking pads lead to a reconfigure event being sent upstream. This is required since the linking of a new element can create a scenario where new caps would be optimal, or perhaps a bufferpool could be used, so this helps the pipeline adjust. If it is not needed it is harmless as elements would end up noticing the same caps/bufferpool is still good to be used. Except that we know in some cases that renegotiation wouldn't be needed. This can introduce some overhead, specially on autoplugging scenarios. It would be interesting to have a linking flag to enable/disable reconfigure so that applications could disable reconfigure when they are sure it isn't needed. A possible solution is to add a new GstPadLinkCheck flag but this isn't really a check and would cause code already using GST_PAD_LINK_CHECK_NOTHING to automatically disable reconfigure and that can break stuff. Do we want a new gst_pad_link variant? Abuse GstPadLinkCheck in some other way?
Created attachment 315013 [details] [review] pad: add no-reconfigure link check Enable it to prevent sending reconfigure when linking elements. Useful for autoplugging when we know caps or bufferpools shouldn't change to save doing caps renegotiation to end up with the same final scenario. The no-reconfigure is not a proper check, it is a flag. It is implemented as a GstPadLinkCheck to avoid creating another gst_pad_link variant.
Review of attachment 315013 [details] [review]: Makes sense and arguably it actually is a check. It triggers a check at other elements to see if they need to renegotiate or can't work anymore (but it's not a check that might fail linking). ::: gst/gstpad.h @@ +198,3 @@ * caps returned by gst_pad_query_caps(). + * @GST_PAD_LINK_CHECK_NO_RECONFIGURE: Disables pushing a reconfigure event when pads are + * linked. Since: 1.8
Created attachment 317171 [details] [review] playback: use gst_pad_link_full to disable unneeded checks With this patch it avoids decoders from negotiating a 2nd bufferpool of the same configuration as the previous one because of the reconfigure event. It would be nice if people could test if this doesn't break stuff in android or other platforms. Also makes me think if decoders couldn't detect that they don't need to actually start a renegotiation at all if the format is the same and the current bufferpool is suitable.
+1 Sounds a much better solution than what I badly attempted in bug #720597 Have you counted how many reconfiguration it avoids in common pipelines like: gst-launch-1.0 playbin uri=http://clips.vorwaerts-gmbh.de/big_buck_bunny.mp4 ? (At the time I reported these bugs #720597 and #721400, there were 2 unnecessary renegotiations) (In reply to Thiago Sousa Santos from comment #3) > It would be nice if people could test if this doesn't break stuff in android > or > other platforms. I will give a try with hardware decoders + zecopy-decoding on desktop with https://github.com/Samsung/ChromiumGStreamerBackend but rebasing glimagesink+dmabuf will take some times so do not wait for me. I'll just notify what happen afterward.
Review of attachment 317171 [details] [review]: Do you have numbers how much it improves the situation? Looks good otherwise except for the playsink part. In playsink this code can be dynamically called while the pipeline is already running, and reconfiguration of upstream (e.g. the decoders) might be useful then.
Thiago?
Comment on attachment 315013 [details] [review] pad: add no-reconfigure link check Attachment 315013 [details] pushed as 83cac0f - pad: add no-reconfigure link check
Thiago ping again? :)
Hi, updated and re-run my scripts. It reduces the queries from 102 to 94 and the gains on a desktop are marginal. The other thing this patch used to do was avoid an extra bufferpool being re-negotiated but without the playsink bits this isn't working. Mostly the renegotiation is caused by linking the convertbin elements and playsink, this happens after the decoder has started its first bufferpool, so it ends up asking for a new one with the exact same configurations. Because of the way bufferpools need to be activated and deactivated it is hard to have the allocation query return the same bufferpool again.
For what it's worth, I believe in playbin3 things work differently so that the decoder gets through to the sink right away.
Do we still want this ? Considering the gains are marginal and decodebin3/playbin3 solves the original reason why this is wanted..
I agree we can close this one.