[vlc-devel] [PATCH]GSoC 09 qualification task :Add Truran VOD access module
zhigang wang
pop.felix at gmail.com
Thu Apr 2 17:23:55 CEST 2009
Hi,
Thanks for your kindly code review. I indeed had debugged and tested this
small access module. It can be compiled and work fine for me. But I trim
some trailing whitespace before commit and make patch, I think that
operation make the source code uncompilable. I am sorry.
By the way, I wrote this module by using http access module as template, the
source code is not clean enough :) So I resubmit my patch base on Rémi
Denis-Courmont's comments.
2009/4/2 Rémi Denis-Courmont <rem at videolan.org>
> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20090402/421082e2/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-GSoC-09-qualification-task-Add-Truran-VOD-access-mo.patch
Type: application/octet-stream
Size: 21319 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20090402/421082e2/attachment.obj>
More information about the vlc-devel
mailing list