[vlc-devel] [PATCH] Fix for SAP mishandling of duplicates via different addresses
alex at digriz.org.uk
Thu Oct 20 17:54:12 CEST 2011
Rémi Denis-Courmont <remi at remlab.net> wrote:
> On Wed, 19 Oct 2011 14:21:56 +0100, Alexander Clouter <alex at digriz.org.uk>
>> We broadcast a number of IPTV channels at my organisation and want the
>> channels available to internal and nearby external users. As a result
>> we announce these channels via SAP on *both* 126.96.36.199
>> (SAP_V4_LOCAL_ADDRESS) and 188.8.131.52 (SAP_V4_ORG_ADDRESS)...
> I really don't see the point, when you could use the largest scope for
We have some streams that must stay private and thus should only be
broadcasted on 184.108.40.206, the rest goes out on 220.127.116.11
(the group addresses used for the streams are completely different too).
We have a bunch of hardware set-top boxes (Exterity iDapters) that by
default only tune into 18.104.22.168; if we were to enable them to
tune into 22.214.171.124 too they see announcements from the whole
.ac.uk multicast community.
>> The function modules/services_discovery/sap.c::ParseSAP() has an
>> IsSameSession() check that overlooks the possibility that the same
>> session could arrive via different SAP listener sockets and as a
>> result rewrites the entire SAP playlist once every 30 seconds
> If the two announces are actually identical and VLC should match them and
> not rewrite anything. This is not what your patch does.
> Or they are different and IsSameSession() should not match them,
> irrelevant of the multicast group they were received from. Your patch
> does not do that either.
They are identical, byte for byte, I just send them out over both
126.96.36.199 and 188.8.131.52.
>> (we announce at a period of five seconds and your 'trust' cycle of
>> six leads to 30 seconds).
> This sounds more like a bug in the time/trust logic, that I never really
Looking at things more closely, it looks like it's the "Check for items
that need deletion" bit that gets upset back in the main run loop. The
duplicates mangle the calculation of average period; as the duplicates
arrive ~1ms apart and then the cycle repeats at 5000ms intervals.
>> Once rewritten, any channel being viewed is terminated.
> This is a known bug in the playlist code. Feel free to fix it.
That has a DeKok ring to it...
Besides, that particular bug does not directly bother me.
>> This repeats every thirty seconds, getting annoying, real quickly. :)
>> The solution is to additionally track the SAP listener socket that
>> the packet arrived on, and only go through the 'trust' codepath if it
>> comes in on the same socket; ignoring duplicates (packets that match
>> the conditions of IsSameSession) arriving via different sockets.
>> I am pretty sure this is a safe fix as nothing in RFC2974 (section 5,
>> 'Session Modification') implies that broadcasting the same SDP
>> payload on different SAP addresses is incorrect; hopefully I have not
>> missed any clauses ;)
> Sure, but your patch seems to contradict that by actively fighting this.
This is because I mis-diagnosed the underlying problem :)
The patch fixes the problem by preventing the value of i_last/i_period
being tinkered with by the duplicate SAP inside the for loop lurking at
.sigmonster says: What I tell you three times is true.
-- Lewis Carroll
More information about the vlc-devel