[vlc-devel] [PATCH 1/2] Added a simple module heavily based upon the udp access module It decoms MPEG TS over iNet-X which is used in the FTI networks

Rémi Denis-Courmont remi at remlab.net
Tue Apr 30 17:01:56 CEST 2013


	Hello,

Comments inline...

Le lundi 29 avril 2013 17:41:07, Diarmuid Collins a écrit :
> From: Diarmuid Collins <dcollins at curtisswright.com>
> 
> ---
>  modules/access/Modules.am |    6 ++
>  modules/access/inetx.c    |  242
> +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 248
> insertions(+)
>  mode change 100644 => 100755 modules/access/Modules.am
>  create mode 100755 modules/access/inetx.c
> 
> diff --git a/modules/access/Modules.am b/modules/access/Modules.am
> old mode 100644
> new mode 100755
> index 8a571e7..ce67c87
> --- a/modules/access/Modules.am
> +++ b/modules/access/Modules.am
> @@ -322,3 +322,9 @@ libaccess_realrtsp_plugin_la_CFLAGS = $(AM_CFLAGS)
>  libaccess_realrtsp_plugin_la_LIBADD = $(AM_LIBADD) $(SOCKET_LIBS)
>  libvlc_LTLIBRARIES += $(LTLIBaccess_realrtsp)
>  EXTRA_LTLIBRARIES += libaccess_realrtsp_plugin.la
> +
> +libaccess_inetx_plugin_la_SOURCES = \
> +	inetx.c
> +libaccess_inetx_plugin_la_CFLAGS = $(AM_CFLAGS)
> +libaccess_inetx_plugin_la_LIBADD = $(AM_LIBADD) $(SOCKET_LIBS)
> +libvlc_LTLIBRARIES += libaccess_inetx_plugin.la
> \ No newline at end of file
> diff --git a/modules/access/inetx.c b/modules/access/inetx.c
> new file mode 100755
> index 0000000..f3211c9
> --- /dev/null
> +++ b/modules/access/inetx.c
> @@ -0,0 +1,242 @@
> +/*************************************************************************
> **** + * inetx.c: raw iNetX input module
> +
> **************************************************************************
> *** + * Copyright (C) 2001-2005 VLC authors and VideoLAN
> + * Copyright (C) 2007 Remi Denis-Courmont
> + * Copyright (C) 2013 Diarmuid Collins
> + * $Id$
> + *

> + * Authors: Christophe Massiot <massiot at via.ecp.fr>
> + *          Tristan Leteurtre <tooney at via.ecp.fr>
> + *          Laurent Aimar <fenrir at via.ecp.fr>
> + *          Jean-Paul Saman <jpsaman #_at_# m2x dot nl>
> + *          Remi Denis-Courmont

Please drop those. I would rather not be contacted as an author of this plugin 
since I have no clue about the underlying protocol.

> + *          Diarmuid Collins <dcollins at curtisswright.com>
> + *
> + * Reviewed: 23 October 2003, Jean-Paul Saman <jpsaman _at_ videolan _dot_
> org>

Drop that too. Your plugin did not exist in 2003 and JPS never reviewed it.

> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU Lesser General Public License as published
> by + * the Free Software Foundation; either version 2.1 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> License + * along with this program; if not, write to the Free Software
> Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston MA
> 02110-1301, USA. +
> **************************************************************************
> ***/ +
> +
> /*************************************************************************
> ****
> + * This module is very heavily based upon the UDP module as the
> starting point
> + * iNETX is a protocol for use in networked data
> acquisition systems. MPEG can
> + * be carried over this protocol
> + * http://en.wikipedia.org/wiki/Bus_monitoring#Packetizer_bus_monitor
> +
> **************************************************************************
> ***/
> +
> +/*************************************************************************
> **** + * Preamble
> +
> **************************************************************************
> ***/ +
> +#ifdef HAVE_CONFIG_H
> +# include "config.h"
> +#endif
> +
> +#include <vlc_common.h>
> +#include <vlc_plugin.h>
> +#include <vlc_access.h>
> +#include <vlc_network.h>
> +
> +#define MTU 65535

Do you really need it this big? The UDP plugin needs to deal with buggy 
senders doing fragmentation. For Ethernet the maximum should be 1472 or 8972 
bytes.

> +
> +/*************************************************************************
> ****
> + * Module descriptor
> +
> **************************************************************************
> ***/
> +static int  Open ( vlc_object_t * );
> +static void Close( vlc_object_t * );
> +
> +vlc_module_begin ()
> +    set_shortname( N_("INETX" ) )
> +    set_description( N_("iNet-X input") )
> +    set_category( CAT_INPUT )
> +    set_subcategory( SUBCAT_INPUT_ACCESS )
> +
> +    add_obsolete_integer( "server-port" ) /* since 2.0.0 */

I don't think this ever was part of your plugin, so don't declare it.

> +
> +    set_capability( "access", 0 )
> +    add_shortcut( "inetx", "inetxstream" )
> +
> +    set_callbacks( Open, Close )
> +vlc_module_end ()
> +
> +/*************************************************************************
> ****
> + * Local prototypes
> +
> **************************************************************************
> ***/
> +static block_t *BlockINETX( access_t * );
> +static int Control( access_t *, int, va_list );
> +
> +/*************************************************************************
> ****
> + * Open: open the socket
> +
> **************************************************************************
> ***/
> +static int Open( vlc_object_t *p_this )
> +{
> +    access_t     *p_access = (access_t*)p_this;
> +
> +    char *psz_name = strdup( p_access->psz_location );
> +    char *psz_parser;
> +    const char *psz_server_addr, *psz_bind_addr = "";
> +    int  i_bind_port = 1234, i_server_port = 0;
> +    int fd;
> +
> +    /* Set up p_access */
> +    access_InitFields( p_access );
> +    ACCESS_SET_CALLBACKS( NULL, BlockINETX, Control, NULL );
> +
> +    /* Parse psz_name syntax :
> +     * [serveraddr[:serverport]][@[bindaddr]:[bindport]] */

If you don't use the full server at bind syntax, you don't need most of this.

> +    psz_parser = strchr( psz_name, '@' );
> +    if( psz_parser != NULL )
> +    {
> +        /* Found bind address and/or bind port */
> +        *psz_parser++ = '\0';
> +        psz_bind_addr = psz_parser;
> +
> +        if( psz_bind_addr[0] == '[' )
> +            /* skips bracket'd IPv6 address */
> +            psz_parser = strchr( psz_parser, ']' );
> +
> +        if( psz_parser != NULL )
> +        {
> +            psz_parser = strchr( psz_parser, ':' );
> +            if( psz_parser != NULL )
> +            {
> +                *psz_parser++ = '\0';
> +                i_bind_port = atoi( psz_parser );
> +            }
> +        }
> +    }
> +
> +    psz_server_addr = psz_name;
> +    psz_parser = ( psz_server_addr[0] == '[' )
> +        ? strchr( psz_name, ']' ) /* skips bracket'd IPv6 address */
> +        : psz_name;
> +
> +    if( psz_parser != NULL )
> +    {
> +        psz_parser = strchr( psz_parser, ':' );
> +        if( psz_parser != NULL )
> +        {
> +            *psz_parser++ = '\0';
> +            i_server_port = atoi( psz_parser );
> +        }
> +    }
> +
> +    msg_Dbg( p_access, "opening server=%s:%d local=%s:%d",
> +             psz_server_addr, i_server_port, psz_bind_addr, i_bind_port );
> +
> +    fd = net_OpenDgram( p_access, psz_bind_addr, i_bind_port,
> +                        psz_server_addr, i_server_port, IPPROTO_UDP );
> +    free (psz_name);
> +    if( fd == -1 )
> +    {
> +        msg_Err( p_access, "cannot open socket" );
> +        return VLC_EGENERIC;
> +    }
> +    p_access->p_sys = (void *)(intptr_t)fd;
> +
> +    return VLC_SUCCESS;
> +}
> +
> +/*************************************************************************
> ****
> + * Close: free unused data structures
> +
> **************************************************************************
> ***/
> +static void Close( vlc_object_t *p_this )
> +{
> +    access_t     *p_access = (access_t*)p_this;
> +
> +    net_Close( (intptr_t)p_access->p_sys );
> +}
> +
> +/*************************************************************************
> ****
> + * Control:
> +
> **************************************************************************
> ***/
> +static int Control( access_t *p_access, int i_query, va_list args )
> +{
> +    bool    *pb_bool;
> +    int64_t *pi_64;
> +
> +    switch( i_query )
> +    {
> +        /* */
> +        case ACCESS_CAN_SEEK:
> +        case ACCESS_CAN_FASTSEEK:
> +        case ACCESS_CAN_PAUSE:
> +        case ACCESS_CAN_CONTROL_PACE:
> +            pb_bool = (bool*)va_arg( args, bool* );
> +            *pb_bool = false;
> +            break;
> +        /* */
> +        case ACCESS_GET_PTS_DELAY:
> +            pi_64 = (int64_t*)va_arg( args, int64_t * );
> +            *pi_64 = INT64_C(1000)
> +                   * var_InheritInteger(p_access, "network-caching");
> +            break;
> +
> +        /* */
> +        case ACCESS_SET_PAUSE_STATE:
> +        case ACCESS_GET_TITLE_INFO:
> +        case ACCESS_SET_TITLE:
> +        case ACCESS_SET_SEEKPOINT:
> +        case ACCESS_SET_PRIVATE_ID_STATE:
> +        case ACCESS_GET_CONTENT_TYPE:
> +            return VLC_EGENERIC;
> +
> +        default:
> +            msg_Warn( p_access, "unimplemented query in control" );
> +            return VLC_EGENERIC;
> +
> +    }
> +    return VLC_SUCCESS;
> +}
> +
> +/*************************************************************************
> ****
> + * BlockINETX:
> +
> **************************************************************************
> ***/
> +static block_t *BlockINETX( access_t *p_access )

You can all it Block(). It is called BlockUDP() for historical reasons (when 
the RTP plugin was in the same file).

> +{
> +    access_sys_t *p_sys = p_access->p_sys;
> +    block_t      *p_block;
> +    ssize_t len;
> +
> +    /* The iNET-X header contains stream ids, sequence numbers and PTP
> timestamps*
> +    *  For the moment these headers will be ignore and we
> pick out the raw MPEG ts
> +    *  which is offset INETX_HDR_LEN bytes into
> the UDP payload
> +    */

Well OK for now. But ignoring sequence numbers is probably not a very good 
idea as MPEG-TS does not deal with packet reordering.

> +    const int INETX_HDR_LEN = 28;
> +
> +    if( p_access->info.b_eof )

This is always false. Remove.

> +        return NULL;
> +
> +    /* Read data */
> +    p_block = block_Alloc( MTU - INETX_HDR_LEN );

block_Alloc() can fail.

> +    char * temp = malloc(MTU);

So can malloc(), but you should not need to call it.

> +    len = net_Read( p_access, (intptr_t)p_sys, NULL,
> +                    temp, MTU, false );
> +
> +    for (int i = INETX_HDR_LEN ; i < MTU ; ++i)
> +    {
> +        // strip out the first INETX_HDR_LEN bytes from the UDP payload
> +        p_block->p_buffer[i - INETX_HDR_LEN] = temp[i];
> +    }

Normally, you would use memcpy() for that.

But you can simply read into p_block and move the block head without copying 
data:
p_block->p_buffer += INETX_HDR_LEN;
p_block->i_buffer -= INETX_HDR_LEN;

> +    free(temp);
> +
> +    if( len < 0 )
> +    {
> +        block_Release( p_block );
> +        return NULL;
> +    }
> +    return block_Realloc( p_block, 0, len-INETX_HDR_LEN );

This is undefined if len < INETX_HDR_LEN.

> +}

-- 
Rémi Denis-Courmont
http://www.remlab.net/



More information about the vlc-devel mailing list