GNOME Bugzilla – Bug 739546
New socketsrc element
Last modified: 2015-04-23 10:10:56 UTC
Created attachment 289861 [details] [review] gst-plugins-base: patch: tcp: Add element socketsrc `socketsrc` can be considered a source counterpart to `multisocketsink`. It can be considered a generalization of `tcpclientsrc` and `tcpserversrc`: it contains all the logic required to communicate over the socket but none of the logic for creating the sockets/establishing the connection in the first place. The user is responsible for providing the already connected socket and they may accomplish this in whatever manner they wish. In particular this makes it applicable to other types of sockets besides TCP such as UNIX domain sockets. There are also patches to refactor tcpclientsrc and tcpserversrc in terms of socketsrc to reduce code-duplication. These patches build on-top of the tests introduced in #739544. This will be useful to have a src to add socket-specific functionality to, specifically the ability to populate the metadata from the GstNet package. This was written to support my aims of zero-copy video IPC discussed in #737316, but I believe the patches stand on their own.
Created attachment 289862 [details] [review] patch: gst-plugins-base: tcpclientsrc: Refactor to derive from socketsrc
Created attachment 289863 [details] [review] patch: gst-plugins-base: tcpserversrc: Refactor to derive from socketsrc
Note, this new plugin is basically the same code as removed from tcpclientsrc and tcpserversrc. There are a few issues with it including: * It ignores GstBaseSrc's blocksize property * Reading the number of bytes available before reading seems unnecessary * Downstream allocators are not used. but I'd like to fix those issues in later commits where they can be individually reviewed.
Created attachment 299324 [details] [review] Add element socketsrc
Created attachment 299325 [details] [review] socketsrc: Refactor to simplify
Created attachment 299326 [details] [review] socketsrc: Tidy up usage of `g_object_unref`/`g_clear_object` and locking
Created attachment 299327 [details] [review] socketsrc: Add `on-socket-eos` signal
I consider 1. Add element socketsrc 2. socketsrc: Refactor to simplify 3. socketsrc: Tidy up usage of `g_object_unref`/`g_clear_object` and locking 4. socketsrc: Add `on-socket-eos` signal Ready to merge. The end-result is that there is a new element "socketsrc", but I've split these patches up. The intention is that the patches are easier to review as the first one contains pretty much an exact copy of the code from `tcpserversrc`, and the other 3 are changes I've made on top of that. The other two patches (refactoring tcpclientsrc and tcpserversrc) probably need some more work as I've not looked at them so recently.
Created attachment 299333 [details] [review] Add element socketsrc
Created attachment 299334 [details] [review] socketsrc: Refactor to simplify
Created attachment 299335 [details] [review] socketsrc: Tidy up usage of `g_object_unref`/`g_clear_object` and locking
Created attachment 299336 [details] [review] socketsrc: Add `on-socket-eos` signal
pushed now with some minor fixes. commit a297b0545f664c82351bbae1fd3f943d068b5031 Author: William Manley <will@williammanley.net> Date: Fri Mar 13 13:56:13 2015 +0000 socketsrc: Add `connection-closed-by-peer` signal This provides notification that the socket in use was closed by the peer and gives an opportunity to replace it with a new one which is not closed, allowing reading from many sockets in order. I use this in pulsevideo to implement reconnection logic to handle the pulsevideo service dieing, such that is can be restarted without disrupting downstream. Fixes https://bugzilla.gnome.org/show_bug.cgi?id=739546 commit a19ac4b85c9dbd930bf68058e8581b7e09a640ea Author: William Manley <will@williammanley.net> Date: Fri Mar 13 13:43:59 2015 +0000 socketsrc: Tidy up usage of `g_object_unref`/`g_clear_object` and locking This is clearer, and should make future changes safer. No functional change intended. See https://bugzilla.gnome.org/show_bug.cgi?id=739546 commit 0c054aa00d99f02a73e83261ccb08f237c5e7b70 Author: William Manley <will@williammanley.net> Date: Fri Mar 13 13:30:48 2015 +0000 socketsrc: Refactor to simplify * Don't bother polling, just do a blocking read, the `GCancellable` will take care of unlocking. This should also be faster on MS Windows where the GIO documentation for `g_socket_get_available_bytes` states: "Note that on Windows, this function is rather inefficient in the UDP case". * Implement `GstPushSrc.fill` rather than `GstPushSrc.create`. This means that we will be using the downstream allocator which may be more efficient. It also means that socketsrc is likely to respect its "blocksize" property (assuming that there is enough data available). See https://bugzilla.gnome.org/show_bug.cgi?id=739546 commit 7c10499ecdfb3743e893eac8439c03d58895dd46 Author: William Manley <will@williammanley.net> Date: Mon Nov 3 02:47:14 2014 +0000 tcp: Add element socketsrc `socketsrc` can be considered a source counterpart to `multisocketsink`. It can be considered a generalization of `tcpclientsrc` and `tcpserversrc`: it contains all the logic required to communicate over the socket but none of the logic for creating the sockets/establishing the connection in the first place, allowing the user to accomplish this externally in whatever manner they wish making it applicable to other types of sockets besides TCP. This commit essentially copies the implementation directly from tcpserversrc. Later patches will tidy the implementation up and re-implement `tcpclientsrc` and `tcpserversrc` in terms of `socketsrc`. See https://bugzilla.gnome.org/show_bug.cgi?id=739546
deriving tcpclientsrc and tcpserversrc from socketsrc is not nice. It would expose a useless GSocket on the elements. It is probably better to either make an abstract socketsrc and make subclasses for each case, only exposing the properties that make sense.
Anything left to be done here? Avoiding the code duplication?
> Anything left to be done here? Avoiding the code duplication? That would be nice, but can be done on a separate bug. Resolving.