GNOME Bugzilla – Bug 652711
udp: add support for IGMPv3 SSM (Source Specific Multicast RFC 4604)
Last modified: 2018-11-03 14:44:16 UTC
Created attachment 190026 [details] [review] when joining a multicast stream, users can select the source they want to receive the stream from * Overview: for now when joining a multicast stream, users cannot select the source they want to receive the stream from. * Additional Information: The kernel configuration determines which IGMP version to use. On linux you can select the minimum IGMP version to use. On windows, you can only select the maximum IGMP version to use. IGMPv3 + SSM is avaible from winXP_SP1. On winXP (don't know on vista and seven) when a network interface sees a IGMPv1 or IGMPv2 it becomes impossible to send a IGMPv3 packet without restarting the network interface (http://support.microsoft.com/kb/815752)
* Additional information on the attached patch (id=190026): This is done by using IP_ADD_SOURCE_MEMBERSHIP socket option instead of IP_ADD_MEMBERSHIP. This patch was compiled on both Windows and Linux, but only Windows tests were performed.
I forgot to tell that the source has to be set using the following format 'udp://172.22.200.54@239.130.110.108:10108' through the 'uri' property. We have not added a property 'source-ip' as it's done for 'multicast-group'
Created attachment 205171 [details] [review] also add source-ip property
Not sure how useful this is now, the plan would be to port the UDP (and all network plugins for that matter) to GIO in 0.11. Adding support for multicast stuff to GIO would be very useful though, see bug #667857
Anybody planning to port this to GIO and add support in the UDP plugin?
Julien, are you planning to port this? If not, we should probably just close this bug.
So I tried to port my patch to 1.x and gio but I found that even if "g_socket_join_multicast_group" has a "gboolean source-specific" parameter, there is no way to setup the source address (see ip_mreq_source / imr_sourceaddr in my original patch) The "g_socket_join_multicast_group" implementation can be found here: https://git.gnome.org/browse/glib/tree/gio/gsocket.c#n2108 I missed something ? Otherwise gio needs to be improved first.
Entirely possible that gio might need fixing up first.
gio bug reported here https://bugzilla.gnome.org/show_bug.cgi?id=740791
Created attachment 291706 [details] [review] udpsrc: add support for IGMPv3 SSM
Comment on attachment 291706 [details] [review] udpsrc: add support for IGMPv3 SSM As this requires a changed GIO API it's not good to merge obviously ;) Otherwise it looks good though. I think this just needs a new function in GLib with different parameter types.
Created attachment 345045 [details] [review] udpsrc: add support for IGMPv3 SSM I rebased Julien's patch onto 1.6.4, which is old but newer than the original patch. It uses the newly added functions from Bugzilla #740791. It doesn't apply cleanly on master, though.
Created attachment 360769 [details] [review] udpsrc: add support for IGMPv3 SSM Rebase on top of latest mater.
There is a regression in gst_udp_parse_uri, causing access to uninitialized memory after having parsed a VLC-style URI without source address, e.g. "@239.35.10.4:10000". You can find a fixed patch here (for 1.6.4 again), for your reference: https://github.com/opendreambox/opendreambox/blob/krogoth/meta-opendreambox/recipes-multimedia/gstreamer/gstreamer1.0-plugins-good/0001-udpsrc-add-support-for-IGMPv3-SSM.patch
Created attachment 361661 [details] [review] udpsrc: add support for IGMPv3 SSM Handle the case when no multicast-source is provided
Hi Andreas, thx for reporting this, indeed there was a bug. I fixed one but could you make a diff if still needed, would be easier for me to see what your change fixes. Thx a lot
At a second glance, I realized that the access to uninitialized memory was a problem caused by an earlier modification made to your patch by myself. However, the problem I still see is here: if (source != NULL) { *source = g_strndup (location_start, location - location_start); GST_DEBUG ("source ip address set to '%s'", *source); } *source becomes non-NULL, even if the string before '@' is empty. I therefore added location - location_start > 0 to the condition and set *source to NULL otherwise. I currently can't download your raw patch from bugzilla, probably due to a problem with my local resolver and DNSSEC, so it's hard to apply and modify right now.
Created attachment 361789 [details] [review] udpsrc: add support for IGMPv3 SSM Done, thx for reporting that.
Created attachment 361790 [details] [review] udpsrc: add support for IGMPv3 SSM Just spotted a miror issue with previous patch related to iface, so this is fixed now.
Created attachment 361791 [details] [review] udpsrc: add support for IGMPv3 SSM And another one.
Created attachment 362158 [details] [review] udpsrc: add support for IGMPv3 SSM Set *source to NULL to fix uninitialized memory issue.
Sebastian, Tim, the gio patch has landed 2 months ago in upstream glib. Do you want to review the attached patch for gstudpsrc and is it ok to land it before 1.14 ? Maybe the element should output a GST_WARNING is one is trying to use the new property with glib < 2.56. In the patch it is currently silently ignored.
I would prefer to have the property implemented on our side in #ifdefs for older GLib versions. Conditional properties are a usability nightmare.
Sure good idea, I can try to do it in the next 2 or 3 weeks if the 1.14 will not happen before. But could you review current patch so that I will integrate remarks while integrating your suggestion. Thx!
Would be great to have this in 1.14 since glib 2.56 too is released now :)
(In reply to Baby octopus from comment #25) > Would be great to have this in 1.14 since glib 2.56 too is released now :) Thx for letting us know. But I plan that for later. In the end I am not really keen to maintain a local copy the new glib functions g_socket_join_multicast_group_ssm / g_socket_leave_multicast_group_ssm . Is there any precedence case like this ? i.e. where a new gst element property that depends on a newer glib ? Also note that in the attached patch the new property is added even for older glib versions (no ifdef)
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/issues/46.