[vlc-devel] [PATCH]GSoC 09 qualification task :Add Truran VOD access module

Rémi Denis-Courmont rem at videolan.org
Wed Apr 1 22:14:42 CEST 2009


	Hello,

Le mercredi 1 avril 2009 17:31:00 zhigang wang, vous avez écrit :
> The module I write is for the GSoC qualification task. This task is not
> list on " VLC selection tasks".
> Truran VoD system is used by our school VoD system. There is only a windows
> client support this VoD system. I wrote this small access module of VLC
> these days so that I can play videos of this VoD system on linux.
>
> Please give me some commit. If it's not enough for qualifuction task, I
> will choose another selection task.

As JB said, this is enough as far as GSoC qualification is concerned.

Nevertheless, some comments inline:

diff --git a/modules/access/truran_vod.c b/modules/access/truran_vod.c
new file mode 100644
index 0000000..4ae21cc
--- /dev/null
+++ b/modules/access/truran_vod.c
@@ -0,0 +1,599 @@
+/*****************************************************************************
+ * truran_vod.c: Truran VOD input module
+ 
*****************************************************************************
+ * Copyright (C) 2009 the VideoLAN team

You need the license boilerplate here.

+ * $Id$\movie\test.avi
        ^^^^^^^^^^^^^^^
What's this?

+#define VOD_DEBUG
+*/
+#define MTU (32*1024)

Is there a specific reason for 32kb read segmentation?

+#ifdef VOD_DEBUG
+#include <stdio.h>
+#endif
+
+#undef ATTRIBUTE_PACKED
+
+#if defined(__GNUC__)
+#if __GNUC__ > 2 || (__GNUC__ == 2 && __GNUC_MINOR__ >= 95)
+#define ATTRIBUTE_PACKED __attribute__ ((packed))
+#define PRAGMA_PACK 0
+#endif
+#endif

We have HAVE_ATTRIBUTE_PACKED in <config.h> for this.

+#if !defined(ATTRIBUTE_PACKED)
+#define ATTRIBUTE_PACKED
+#define PRAGMA_PACK 1
+#endif
+
+#if PRAGMA_PACK
+#pragma pack(1)
+#endif
+
+/*****************************************************************************
+ * Module descriptor
+ 
*****************************************************************************/
+#define CACHING_TEXT N_("Caching value in ms")
+#define CACHING_LONGTEXT N_( \
+    "Caching value for Truran VOD streams. This " \
+    "value should be set in milliseconds." )
+
+#define RECONNECT_TEXT N_("Auto re-connect")
+#define RECONNECT_LONGTEXT N_( \
+    "Automatically try to reconnect to the stream in case of a sudden " \
+    "disconnect." )
+
+static int  Open ( vlc_object_t * );
+static void Close( vlc_object_t * );

Where are the #include lines? Where did you define vlc_object_t? Can you even 
even compile this?

+vlc_module_begin();
+    set_shortname( N_("VOD" ) );
+    set_description( N_("Truran VOD input") );
+    set_category( CAT_INPUT );
+    set_subcategory( SUBCAT_INPUT_ACCESS );
+
+    add_integer( "vod-caching", DEFAULT_PTS_DELAY / 1000, NULL, CACHING_TEXT,
+                 CACHING_LONGTEXT, true );
+
+     add_bool( "vod-reconnect", 0, NULL, RECONNECT_TEXT,
+              RECONNECT_LONGTEXT, true );
+
+    set_capability( "access", 0 );
+    add_shortcut( "vod" );
+
+    set_callbacks( Open, Close );
+vlc_module_end();
+
+/*****************************************************************************
+ * Local prototypes
+ 
*****************************************************************************/
+#if PRAGMA_PACK
+#pragma pack(1)
+#endif
+
+typedef struct {
+    char magic[4];
+    unsigned int ver;		/* always 00 02 00 00 */
+    unsigned int type;		/* ok = 00 00 00 00, err = 0c 00 00 00 */
+    unsigned int unknown1;	/* always 00 00 00 00 */
+    unsigned int len_lo;		/* lower 32 bits of length */
+    unsigned int len_hi;		/* higher 32 bits of length */
+    unsigned int pos_lo;		/* lower 32 bits of position */
+    unsigned int pos_hi;		/* higher 32 bits of position */
+    unsigned int unknown2;	/* always 00 00 01 00 */
+    unsigned int unknown3;	/* always 00 00 00 00 */
+    unsigned int unknown4;	/* ok = 64 00 00 00, err = 00 00 00 00 */
+    char errmsg[100];
+    char name[260];
+    char date[108];
+} ATTRIBUTE_PACKED response_t;

I guess you are assuming that unsigned int is 32-bits. You should really use 
uint32_t instead.

+
+#if PRAGMA_PACK
+#pragma pack()
+#endif
+
+struct access_sys_t {
+    int fd;
+
+    /* From uri */
+    vlc_url_t url;
+
+    int i_code; /*must be 200*/
+    char       *psz_mime;
+    response_t resp;

Do you really need to keep the response buffer in the plugin state?

+    bool b_seekable;

This seems to be always true. Then you should remove it.

+    bool b_reconnect;
+#ifdef VOD_DEBUG
+    FILE* out_fd;
+#endif
+};
+
+/* */
+static ssize_t Read( access_t *, uint8_t *, size_t );
+static block_t *Block( access_t *p_access );
+static int Seek( access_t *, int64_t );
+static int Control( access_t *, int, va_list );
+
+/* */
+static int Connect( access_t *, int64_t );
+static int Request( access_t *p_access, int64_t i_tell );
+static void Disconnect( access_t * );
+
+/*****************************************************************************
+ * Open:
+ 
*****************************************************************************/
+static int Open( vlc_object_t *p_this )
+{
+    access_t     *p_access = (access_t*)p_this;
+    access_sys_t *p_sys;
+    char         *psz, *p;
+
+    /* Set up p_access */
+#if 1
+    STANDARD_READ_ACCESS_INIT;
+#else
+    STANDARD_BLOCK_ACCESS_INIT;
+#endif

If block mode is not needed, then you should remove it completely.

+
+    p_sys->fd = -1;
+    p_sys->b_seekable = true;
+    p_sys->psz_mime = NULL;
+
+    p_access->info.i_size = -1;
+    p_access->info.i_pos  = 0;
+    p_access->info.b_eof  = false;
+
+    /* Parse URI - remove spaces */
+    p = psz = strdup( p_access->psz_path );
+
+    vlc_UrlParse( &p_sys->url, psz, 0 );
+    free( psz );

There is no point in strdup+free here, is there?
And p does not seem to be ever used (GCC should warn you about that).

+    if( p_sys->url.psz_host == NULL || *p_sys->url.psz_host == '\0' )
+    {
+        msg_Warn( p_access, "invalid host" );
+        goto error;
+    }
+
+    if( p_sys->url.i_port <= 0 )  p_sys->url.i_port = 1680;
+
+    p_sys->b_reconnect = var_CreateGetBool( p_access, "vod-reconnect" );
+
+    if (Connect( p_access, 0 ) != VLC_SUCCESS) goto error;
+
+#ifdef VOD_DEBUG
+    p_sys->out_fd = fopen("dump", "wb");
+#endif

You probably don't need this. You can run VLC with
`--demux dump --demuxdump-file dump` if you want to dump content.

+    return VLC_SUCCESS;
+
+error:
+    vlc_UrlClean( &p_sys->url );
+    free( p_sys->psz_mime );
+
+    Disconnect( p_access );
+    free( p_sys );
+
+    return VLC_EGENERIC;
+}
+
+/*****************************************************************************
+ * Close:
+ 
*****************************************************************************/
+static void Close( vlc_object_t *p_this )
+{
+    access_t     *p_access = (access_t*)p_this;
+    access_sys_t *p_sys = p_access->p_sys;
+
+    vlc_UrlClean( &p_sys->url );
+    free( p_sys->psz_mime );
+
+    Disconnect( p_access );
+
+#ifdef VOD_DEBUG
+   fclose( p_sys->out_fd);
+#endif
+
+    free( p_sys );
+}
+
+/*****************************************************************************
+ * Read: Read up to i_len bytes from the vod sockect connection and place in
+ * p_buffer. Return the actual number of bytes read
+ 
*****************************************************************************/
+static ssize_t Read( access_t *p_access, uint8_t *p_buffer, size_t i_len )
+{
+    access_sys_t *p_sys = p_access->p_sys;
+    int i_read;
+
+    if( p_sys->fd < 0 )
+    {
+        p_access->info.b_eof = true;
+        return 0;
+    }
+
+    if( p_access->info.i_size >= 0 &&
+        i_len + p_access->info.i_pos > p_access->info.i_size )
+    {
+        if( ( i_len = p_access->info.i_size - p_access->info.i_pos ) == 0 )
+        {
+            p_access->info.b_eof = true;
+            return 0;
+        }
+    }
+
+    i_read = net_Read( p_access, p_sys->fd, NULL, p_buffer, i_len, false );
+
+#ifdef VOD_DEBUG
+    msg_Dbg(p_access, ">%d--->%d", i_read, i_len - i_read);
+#endif
+
+    if( i_read > 0 )
+    {
+        p_access->info.i_pos += i_read;
+    }
+    else if( i_read == 0 )
+    {
+        /*
+         * I very much doubt that this will work.
+         * If i_read == 0, the connection *IS* dead, so the only
+         * sensible thing to do is Disconnect() and then retry.
+         * Otherwise, I got recv() completely wrong. -- Courmisch
+         */

This comment (that I wrote a long time ago) does not belong here.

+        Disconnect( p_access );
+        if( p_sys->b_reconnect )
+        {
+            msg_Dbg( p_access, "got disconnected, trying to reconnect" );
+            if( Connect( p_access, p_access->info.i_pos ) )
+            {
+                msg_Dbg( p_access, "reconnection failed" );
+            }
+            else
+            {
+                p_sys->b_reconnect = false;
+                i_read = Read( p_access, p_buffer, i_len );
+                p_sys->b_reconnect = true;
+            }
+        }
+
+        if( i_read == 0 ) p_access->info.b_eof = true;
+    }
+#ifdef VOD_DEBUG
+    fwrite( p_buffer, 1, i_read, p_sys->out_fd);
+#endif
+
+    return i_read;
+}
+
+/*****************************************************************************
+ * Block
+ 
*****************************************************************************/
+static block_t *Block( access_t *p_access )

This function seems to be unused, and should be removed. GCC should have 
warned about this issue.

+{
+    access_sys_t *p_sys = p_access->p_sys;
+    block_t      *p_block;
+    ssize_t i_read;
+
+    if( p_access->info.b_eof )
+        return NULL;
+
+    /* Read data */
+    p_block = block_New( p_access, MTU );
+    i_read = net_Read( p_access, p_sys->fd, NULL, p_block->p_buffer, MTU, 
false );
+
+#ifdef VOD_DEBUG
+     msg_Dbg(p_access, "<%d--->%d", i_read, MTU - i_read);
+#endif
+
+    if( i_read < 0 )
+    {
+        block_Release( p_block );
+        return NULL;
+    }
+
+#ifdef VOD_DEBUG
+    fwrite( p_block->p_buffer, 1, i_read, p_sys->out_fd);
+#endif
+
+    return block_Realloc( p_block, 0, p_block->i_buffer = i_read );
+}
+#if 0
+static char* get_comment(char* fn)
+{
+    char* pos;
+
+    pos = strrchr(fn,'\\');
+    if (pos) fn = pos+1;
+    pos = strrchr(fn,'/');
+    if (pos) fn = pos+1;
+    return fn;
+    //  return "comic.sjtu.edu.cn";
+}
+#endif
+static unsigned int get_ip_from_fd(int fd)

Return uint32_t here.

+{
+    struct sockaddr_in saddr;
+    socklen_t len = sizeof(saddr);
+
+    getsockname(fd,(struct sockaddr*)(&saddr), &len);
+    return saddr.sin_addr.s_addr;
+}
+
+static int Connect( access_t *p_access, int64_t i_tell )
+{
+    access_sys_t   *p_sys = p_access->p_sys;
+
+    /* Open connection */
+    p_sys->fd = net_ConnectTCP( p_access,  p_sys->url.psz_host,  
p_sys->url.i_port );
+    if( p_sys->fd == -1 )
+    {
+        msg_Err( p_access, "cannot connect to %s:%d",  p_sys->url.psz_host,  
p_sys->url.i_port );
+        return VLC_EGENERIC;
+    }
+    return Request( p_access, i_tell );
+}
+
+static int Request( access_t *p_access, int64_t i_tell )
+{
+    access_sys_t   *p_sys = p_access->p_sys;
+    char buffer[4096];
+    int pos = 0;
+    int size = sizeof(buffer);
+    response_t *resp = &p_sys->resp;
+    char           *psz;
+    char hostname[256];
+
+    gethostname (hostname, sizeof (hostname));
+    #define myprintf(fmt, args...) do { int len = snprintf(buffer+pos, size, 
fmt, ## args); pos+=len; size-=len; } while (0)

size may suffer an integer underflow here, as snprintf() expects an unsigned.

+
+    myprintf("POST /XMMS/%s%ld.xms HTTP/1.0\r\n", hostname, time(NULL) % 
10000 + 10000);
+    myprintf("Host: %s\r\n",   hostname);

Normally, the HTTP Host field conveys the remote host name from the URL, not 
the local host name. I don't know what's correct here, because your protocol 
seems to not Unless there is a weird deviation from standard HTTP.

+    myprintf("Accept: %s\r\n", "*/*");
+    myprintf("User-Agent: %s\r\n", "XMPLAYER");
+    myprintf("UGT:%s\r\n",     "XMPLAYER");
+    myprintf("UIP:%u\r\n",     get_ip_from_fd(p_sys->fd));

Should be "PRIu32" instead of u, I guess. You should also check that the 
socket address is in the AF_INET family first.

+    myprintf("USN:%d\r\n",     12345678);
+    myprintf("PRI:%d\r\n",     0);
+    myprintf("UPR:%d\r\n",     0);
+    myprintf("SPL:%"PRId64"\r\n",     i_tell);
+    myprintf("SPH:%d\r\n",     0);
+    myprintf("URN:%s\r\n",     "sjtu");
+    myprintf("PWD:%s\r\n",     "sjtu");

Is this used anywhere except in SJTU university? 

+    myprintf("FPT:%d\r\n",     (i_tell==0)?1:0);
+    myprintf("FIL:%s\r\n",     (p_sys->url.psz_path[0] == '/')? 
p_sys->url.psz_path+1:p_sys->url.psz_path);
+    myprintf("ULG:%s\r\n",     "CN");

I assume this is only used by Chinese users.

+    myprintf("STT:%s\r\n",     "YYYY/MM/DD HH:MM:SS");

Is this the date, or the date format?

+    myprintf("HST:%s\r\n",     hostname);
+    myprintf("PNM:%s\r\n",     "comic.sjtu.edu.cn");

Same question as above.

+    myprintf("PID:%d\r\n",     0);
+    myprintf("IDX:%d\r\n",     0);
+    myprintf("\r\n");
+
+    msg_Dbg( p_access, "sending request: \n%s", buffer);
+
+    if ( net_Write( p_access, p_sys->fd, NULL, buffer, strlen(buffer) ) <= 0)
+    {
+        msg_Err( p_access, "failed to send request" );
+        Disconnect( p_access );
+        return VLC_EGENERIC;
+    }
+
+    /* Read Answer */
+    if( ( psz = net_Gets( VLC_OBJECT(p_access), p_sys->fd, NULL ) ) == NULL )
+    {
+        msg_Err( p_access, "failed to read answer" );
+        goto error;
+    }
+    if( !strncmp( psz, "HTTP/1.", 7 ) )
+    {
+          p_sys->i_code = atoi( &psz[9] );
+    }
+    else {
+        msg_Err( p_access, "invalid HTTP reply '%s'", psz );
+        free( psz );
+        goto error;
+    }
+
+    msg_Dbg( p_access, "answer code %d",  p_sys->i_code );
+
+    for( ;; )
+    {
+        char *psz = net_Gets( VLC_OBJECT(p_access), p_sys->fd, NULL );
+        char *p;
+
+        if( psz == NULL )
+        {
+            msg_Err( p_access, "failed to read answer" );
+            goto error;
+        }
+
+        if( !vlc_object_alive (p_access) || p_access->b_error )
+        {
+            free( psz );
+            goto error;
+        }
+
+        msg_Dbg( p_access, "Line=%s", psz );
+        if( *psz == '\0' )
+        {
+            free( psz );
+            break;
+        }
+
+        if( ( p = strchr( psz, ':' ) ) == NULL )
+        {
+            msg_Err( p_access, "malformed header line: %s", psz );
+            free( psz );
+            goto error;
+        }
+        *p++ = '\0';
+        while( *p == ' ' ) p++;
+
+        if( !strcasecmp( psz, "Content-Type" ) )
+        {
+            free( p_sys->psz_mime );
+            p_sys->psz_mime = strdup( p );
+            msg_Dbg( p_access, "Content-Type: %s", p_sys->psz_mime );
+            if (!strcmp(p_sys->psz_mime, "errmsg/*"))
+            {
+                msg_Err( p_access, "Error: %s", p );
+                free( psz );
+                goto error;
+            }
+        }
+
+        free( psz );
+    }
+
+    if (net_Read( p_access, p_sys->fd, NULL, resp, sizeof(response_t), true ) 
< (int)sizeof(response_t))
+    {
+        msg_Err( p_access, "failed to read Response" );
+        goto error;
+    }
+
+    if (strncmp( resp->magic, "XMMS", 4 ) != 0)
+    {     // CHECK MAGIC
+        msg_Err(p_access, " magic mismatch");
+        goto error;
+    }
+
+    if ((resp->type != 0) && (resp->unknown4 == 0))
+    { // maybe error
+        msg_Err(p_access, "Server error %s", resp->errmsg);
+        goto error;
+    }
+
+    msg_Dbg(p_access, "Opening %s\n", resp->name);
+    msg_Dbg(p_access, "Date %s\n", resp->date);
+    msg_Dbg(p_access, "Position %u/%u\n", resp->pos_lo, resp->len_lo);

Assuming you are using a little-endian system, that will fail on big-endian 
systems.

+
+    if (resp->len_lo == 0) {
+        Disconnect(p_access);
+        msg_Err( p_access, "Open failed, may be too many users." );
+        return VLC_EGENERIC;
+    }
+
+    p_access->info.i_size = resp->len_lo;
+    p_access->info.i_pos  = resp->pos_lo;

Same (worse) problem here.

+    p_access->info.b_eof  = false;
+
+    if (resp->pos_lo >= resp->len_lo)

And here.

+        p_access->info.b_eof  = true;
+
+    return VLC_SUCCESS;
+
+error:
+    Disconnect( p_access );
+    return VLC_EGENERIC;
+}
+
+/*****************************************************************************
+ * Disconnect:
+ 
*****************************************************************************/
+static void Disconnect( access_t *p_access )
+{
+    access_sys_t *p_sys = p_access->p_sys;
+
+    if( p_sys->fd != -1)
+    {
+        net_Close(p_sys->fd);
+        p_sys->fd = -1;
+    }
+}
+
+/*****************************************************************************
+ * Seek: close and re-open a connection at the right place
+ 
*****************************************************************************/
+static int Seek( access_t *p_access, int64_t i_pos )
+{
+    msg_Dbg( p_access, "trying to seek to %"PRId64, i_pos );
+
+    Disconnect( p_access );
+
+    if( p_access->info.i_size
+     && (uint64_t)i_pos >= (uint64_t)p_access->info.i_size ) {
+        msg_Err( p_access, "seek to far" );
+        int retval = Seek( p_access, p_access->info.i_size - 1 );
+        if( retval == VLC_SUCCESS ) {
+            uint8_t p_buffer[2];
+            Read( p_access, p_buffer, 1);
+            p_access->info.b_eof  = false;
+        }
+        return retval;
+    }
+    if( Connect( p_access, i_pos ) != VLC_SUCCESS)
+    {
+        msg_Err( p_access, "seek failed" );
+        p_access->info.b_eof = true;
+        return VLC_EGENERIC;
+    }
+    return VLC_SUCCESS;
+}
+
+/*****************************************************************************
+ * Control:
+ 
*****************************************************************************/
+static int Control( access_t *p_access, int i_query, va_list args )
+{
+    access_sys_t *p_sys = p_access->p_sys;
+    bool   *pb_bool;
+    int64_t      *pi_64;
+
+    switch( i_query )
+    {
+        /* */
+        case ACCESS_CAN_SEEK:
+            pb_bool = (bool*)va_arg( args, bool* );
+            *pb_bool = p_sys->b_seekable;
+            break;
+        case ACCESS_CAN_FASTSEEK:
+            pb_bool = (bool*)va_arg( args, bool* );
+            *pb_bool = false;
+            break;
+        case ACCESS_CAN_PAUSE:
+        case ACCESS_CAN_CONTROL_PACE:
+            pb_bool = (bool*)va_arg( args, bool* );
+
+            *pb_bool = true;
+            break;
+
+        case ACCESS_GET_PTS_DELAY:
+            pi_64 = (int64_t*)va_arg( args, int64_t * );
+            *pi_64 = (int64_t)var_GetInteger( p_access, "vod-caching" ) * 
INT64_C(1000);
+            break;
+
+        /* */
+        case ACCESS_SET_PAUSE_STATE:
+            break;
+
+        case ACCESS_GET_META:
+/*
+            p_meta = (vlc_meta_t*)va_arg( args, vlc_meta_t* );
+
+            if( p_sys->psz_icy_name )
+                vlc_meta_Set( p_meta, vlc_meta_Title, p_sys->psz_icy_name );
+            if( p_sys->psz_icy_genre )
+                vlc_meta_Set( p_meta, vlc_meta_Genre, p_sys->psz_icy_genre );
+            if( p_sys->psz_icy_title )
+                vlc_meta_Set( p_meta, vlc_meta_NowPlaying, 
p_sys->psz_icy_title );
+*/


Remove this.

+            break;
+
+        case ACCESS_GET_CONTENT_TYPE:
+            *va_arg( args, char ** ) =
+                p_sys->psz_mime ? strdup( p_sys->psz_mime ) : NULL;
+            break;
+
+        case ACCESS_GET_TITLE_INFO:
+        case ACCESS_SET_TITLE:
+        case ACCESS_SET_SEEKPOINT:
+        case ACCESS_SET_PRIVATE_ID_STATE:
+            return VLC_EGENERIC;
+
+        default:
+            msg_Warn( p_access, "unimplemented query in control" );
+            return VLC_EGENERIC;
+
+    }
+    return VLC_SUCCESS;
+}
+
-- 
1.5.4.3

-- 
Rémi Denis-Courmont
http://git.remlab.net/cgi-bin/gitweb.cgi?p=vlc-courmisch.git;a=summary



More information about the vlc-devel mailing list