[vlc-devel] [PATCH] Add support for AMT multicast tunneling over non-multicast enabled networks

Wayne Brassem wbrassem at rogers.com
Tue Apr 30 15:58:51 CEST 2019


Thank you Ilkka for the idea.  I think I get the principle of what you are saying, but let me explain the issue I am trying to address just to make sure the suggestion will work.  The basic issue is that this module needs to be able to handle both native IP multicast and AMT tunneled IP multicast.

For native multicast we just need to return the UDP payload.

For AMT tunneled multicast we will receive the AMT tunnel header within the UDP payload - but this cannot be returned to main.  So two buffers are used for the AMT tunnel case and then we copy the AMT payload into the pkt buffer and then free the AMT buffer in this module.  Main is responsible for freeing the returned AMT decapsulated content.

If I used just one buffer and returned an AMT tunnel header adjusted pointer to the payload beginning I was concerned that it might leak memory (equal to sizeof( AMT header) ) in main.
Wayne.

> On Apr 30, 2019, at 6:02 AM, Ilkka Ollakka <ileoo at videolan.org> wrote:
> 
> On Mon, Apr 29, 2019 at 12:42:25PM -0400, Wayne Brassem wrote:
>> Updated submission.
>> 
>> Notes on this version based on previous code reviews:
> 
> Hi,
> 
> 
>> +
>> +/*****************************************************************************
>> + * BlockUDP: Responsible for returning the multicast payload
>> + * BlockUDP: Several sections based upon the BlockUDP code in udp.c
>> + * 
>> + * Default MTU based on number of MPEG-2 transports carried in a 1500 byte Ethernet frame
>> + * however the code is able to receive jumbo Ethernet frames and then adjusts the MTU
>> + *****************************************************************************/
>> +static block_t *BlockUDP(stream_t *p_access, bool *restrict eof)
>> +{
>> +    access_sys_t *sys = p_access->p_sys;
>> +    ssize_t len, shift = 0, tunnel = IP_HDR_LEN + UDP_HDR_LEN + AMT_HDR_LEN;
>> +
>> +    /* Allocate a packet buffer based on anticipated MTU */
>> +    block_t *pkt = block_Alloc(sys->mtu);
> 
> I didn't checkin full details of the code, but I don't think you actually need
> pkt and memcpy to it?
> 
> Why not just do "recvpkt->i_buffer -= shift" and "recvpkt->p_buffer += shift"
> and return that? It is not that readable code, but saves one malloc and one
> memcpy per call.
> 
>> +    {
>> +        memcpy( pkt->p_buffer, &recvpkt[shift], len );
>> +    }
>> +
>> +    pkt->i_buffer = len;
>> +
>> +    free( recvpkt );
>> +    return pkt;
> 
> -- 
> Ilkka Ollakka
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel



More information about the vlc-devel mailing list