GNOME Bugzilla – Bug 788200
New element: gstproxy that proxies buffers, events, and caps to separate pipeline
Last modified: 2018-02-07 17:25:09 UTC
Created attachment 360468 [details] [review] New element "proxy" This plugin is useful when you want to pipe arbitrary data to a different pipeline within the same process. Some advantages over appsink/appsrc or the inter elements: * Ease of use. Buffers, events, and caps are transmitted as-is without copying or serialization. * Enables zerocopy (especially DMABUF) * Enables usage with sinks or elements that are unreliable and may throw errors and need re-initialization, such as a network sink, a USB device sink (v4l2), etc * Transmits arbitrary data, not just audio/video/subs * Can implement 1 producer pipeline -> N dynamic consumer pipelines within a single process when combined with the `tee` element
So, what's the difference from appsrc/sink ? Everything in this list can be achieved with appsrc/sink, maybe Ease of use ?
Review of attachment 360468 [details] [review]: Whys is this mixed BSD and LGPLv2 (gstproxy.c) lisenced ?
Review of attachment 360468 [details] [review]: ::: gst/proxy/gstproxysrc.c @@ +35,3 @@ + * + * The element queues buffers from the matching proxysink to an internal queue, + * so everything downstream is properly decoupled from the upstream pipeline. Ho do we use this element ?
Review of attachment 360468 [details] [review]: ::: gst/proxy/gstproxysrc.h @@ +1,2 @@ +/* + * Copyright (C) 2015 Centricular Ltd. Did you really wrote this in 2015, or is this copy paste error ?
Review of attachment 360468 [details] [review]: How does this work with clocks? Timing? Latency queries? Seeks ? State changes? You need to start the sink side before the src side ? It feels like something is missing to make it a generic element. You should try to document a lot of caveats if we want to merge this.
(In reply to Nicolas Dufresne (stormer) from comment #2) > Review of attachment 360468 [details] [review] [review]: > > Whys is this mixed BSD and LGPLv2 (gstproxy.c) lisenced ? It's based on the owr_inter_sink/src elements written by slomo for OpenWebRTC: https://github.com/EricssonResearch/openwebrtc/blob/master/owr/owr_inter_sink.c https://github.com/EricssonResearch/openwebrtc/blob/master/owr/owr_inter_src.c These were BSD clause-2 licensed. I can relicense the whole as LGPL-2 if it is appropriate. (In reply to Nicolas Dufresne (stormer) from comment #3) > Review of attachment 360468 [details] [review] [review]: > > ::: gst/proxy/gstproxysrc.c > @@ +35,3 @@ > + * > + * The element queues buffers from the matching proxysink to an internal > queue, > + * so everything downstream is properly decoupled from the upstream > pipeline. > > Ho do we use this element ? Sorry, I'll update the comment with documentation on how to use it. Will also ensure that a test accompanies it before it goes in-tree. (In reply to Nicolas Dufresne (stormer) from comment #4) > Review of attachment 360468 [details] [review] [review]: > > ::: gst/proxy/gstproxysrc.h > @@ +1,2 @@ > +/* > + * Copyright (C) 2015 Centricular Ltd. > > Did you really wrote this in 2015, or is this copy paste error ? I modified the code in 2015, and haven't had to make any changes since. I've been meaning to post it for a while, but I forgot. I posted it now because dv_ (CCed) needed it for his use-case of 1 producer feeding N pipelines which he is currently hacking with rtsp-server. (In reply to Olivier Crête from comment #5) > Review of attachment 360468 [details] [review] [review]: > > How does this work with clocks? Timing? Latency queries? Seeks ? State > changes? You need to start the sink side before the src side ? > > It feels like something is missing to make it a generic element. You should > try to document a lot of caveats if we want to merge this. All queries, events, buffers, and buffer lists are proxied. State changes are independent since the upstream and downstreams are different pipelines. You should ideally start the producer side before the consumer side, and there is a queue on the consumer end to buffer in-band data if needed. If you start downstream first, the state change will wait till the first buffer gets proxied, but there shouldn't be any issues. :) I'm not sure what you mean by clocks, are you worried that the upstream and downstream might have, for instance, an audio clock and a system clock and go out of sync? I think that would have to be handled manually with get_clock and set_clock. Will document that. Is there anything else you can see that might need to be documented? My approach to this is that it has worked swimmingly for OWRTC and a few other projects I've worked on, and I think it is ready for general consumption. I'm sure people will find things to improve in this, and that's what -bad is for ;)
(In reply to Nirbheek Chauhan from comment #6) > I'm not sure what you mean by clocks, are you worried that the upstream and > downstream might have, for instance, an audio clock and a system clock and > go out of sync? I think that would have to be handled manually with > get_clock and set_clock. Will document that. > > Is there anything else you can see that might need to be documented? I though some events/queries would also include the running time or other things that depend on the base time, but a quick search didn't find any. I'm worried that the simple approach taken here may not work in some cases and that people will see hard to debug issues. The workings of the latency query and latency event will be a bit strange if the computation is done on both pipelines too.
I would say we should get this in at this point. The elements were already used by various people for different use-cases and have proven to be useful. And this is going to -bad, so any problems can be solved or additional use-cases can be implemented as needed.
I agree. I know by experience that the proxy plugin is much easier to use than app plugin for inter-pipeline communication.
Created attachment 367972 [details] [review] Add new 'proxy' element to stream data between pipelines This keep-it-simple plugin is useful when you want to pipe arbitrary data to a different pipeline within the same process. Some advantages over appsink/appsrc, the inter elements, etc: * Ease of use. Buffers, events, and caps are transmitted as-is without copying or serialization. * Enables zerocopy (especially DMABUF) transparently without any special-casing. * Enables usage with sinks or elements that are unreliable and may throw errors and need re-initialization, such as a network sink, a USB device sink (v4l2), etc. * Transmits arbitrary data, not just audio/video/subs * Can easily implement 1 producer pipeline -> N dynamic consumer pipelines within a single process when combined with the `tee` element. All queries, events, buffers, and buffer lists are proxied. State changes, clocks, and base times for the two pipelines are independent since the upstream and downstreams continue to be different pipelines.
Created attachment 367973 [details] [review] Add new 'proxy' element to stream data between pipelines This keep-it-simple plugin is useful when you want to pipe arbitrary data to a different pipeline within the same process. Some advantages over appsink/appsrc, the inter elements, etc: * Ease of use. Buffers, events, and caps are transmitted as-is without copying or serialization. * Enables zerocopy (especially DMABUF) transparently without any special-casing. * Enables usage with sinks or elements that are unreliable and may throw errors and need re-initialization, such as a network sink, a USB device sink (v4l2), etc. * Transmits arbitrary data, not just audio/video/subs * Can easily implement 1 producer pipeline -> N dynamic consumer pipelines within a single process when combined with the `tee` element. All queries, events, buffers, and buffer lists are proxied. State changes, clocks, and base times for the two pipelines are independent since the upstream and downstreams continue to be different pipelines.
This latest patch now contains example usage, relicenses to LGPL-2.1, and updates the build files. Should be ready to go in.
Review of attachment 367973 [details] [review]: The example looks useful to me. IMHO all good to go to -bad for now
Attachment 367973 [details] pushed as 3f7e29d - Add new 'proxy' element to stream data between pipelines