[vlc-devel] Re: [PATCH] New RTSP client module
Jean-Paul Saman
jean-paul.saman at planet.nl
Fri Oct 6 10:45:22 CEST 2006
Florent,
Thank you and anevia for contributing to the VLC project.
I cannot test the patch, since the Anevia hardware is not on my desk to
test against.
I reviewed your patch and found some issues with it:
1) General remark: name the plugin directory access/anevia iso access/rtsp2.
2) several memory leaks (read on below in the patch).
3) VLC uses 4 spaces per indentation level (several parts in the patch
are not properly idented).
4) new files don't have a copyright and GPL license statement
Florent Audebert wrote:
> Hello,
>
> I'm writing an RTSP client module (UDP/TS) for Anevia which works with
> our Manager/VOD servers since Live555 and RealRTSP modules aren't
> compatible.
>
> I've started it from scratch looking at file, realrtsp and udp
> modules. It allows pausing / seeking and runs almost smoothly. The
> only issue I have is during seeking.
>
> Sometimes, even if VLC is receiving the stream it doesn't display it
> and get stuck on the last displayed frame (progress bar still
> continue). Seeking again or doing a Pause/Play and everything seems to
> go well again.
>
> Here's a patch containing the module (named it rtsp2, don't know if
> that's really appropriate). I didn't add GPL headers yet and Windows
> compiling / testings are coming. I've tried to respect VideoLan
> developpers guidelines as much as possible tell me if something is
> wrong. Any suggestion or comments are welcome.
>
> Thanks
>
>
> ------------------------------------------------------------------------
>
> Index: configure.ac
> ===================================================================
> --- configure.ac (revision 16928)
> +++ configure.ac (working copy)
> @@ -2011,7 +2011,7 @@
> if test "${enable_sout}" != "no"; then
> VLC_ADD_PLUGINS([mux_ts])
> fi
> - VLC_ADD_LDFLAGS([mux_ts ts dvb],[-ldvbpsi]) ],
> + VLC_ADD_LDFLAGS([mux_ts ts dvb access_aneviartsp],[-ldvbpsi]) ],
> [ AC_MSG_WARN([cannot find libdvbpsi headers]) ],
> [#if defined( HAVE_STDINT_H )
> # include <stdint.h>
> @@ -2039,8 +2039,8 @@
> if test "${enable_sout}" != "no"; then
> VLC_ADD_BUILTINS([mux_ts])
> fi
> - VLC_ADD_CPPFLAGS([mux_ts ts dvb],[-I${real_dvbpsi_tree}/src])
> - VLC_ADD_LDFLAGS([mux_ts ts dvb],[${real_dvbpsi_tree}/src/.libs/libdvbpsi.a])
> + VLC_ADD_CPPFLAGS([mux_ts ts dvb access_aneviartsp],[-I${real_dvbpsi_tree}/src])
> + VLC_ADD_LDFLAGS([mux_ts ts dvb access_aneviartsp],[${real_dvbpsi_tree}/src/.libs/libdvbpsi.a])
> else
> dnl The given libdvbpsi wasn't built
> AC_MSG_RESULT(no)
> @@ -2067,8 +2067,8 @@
> if test "${enable_sout}" != "no"; then
> VLC_ADD_PLUGINS([mux_ts])
> fi
> - VLC_ADD_CPPFLAGS([mux_ts ts dvb],[${CPPFLAGS_test}])
> - VLC_ADD_LDFLAGS([mux_ts ts dvb],[${LDFLAGS_test} -ldvbpsi])
> + VLC_ADD_CPPFLAGS([mux_ts ts dvb access_aneviartsp],[${CPPFLAGS_test}])
> + VLC_ADD_LDFLAGS([mux_ts ts dvb access_aneviartsp],[${LDFLAGS_test} -ldvbpsi])
>
> ],[
> if test -n "${enable_dvbpsi}"
> @@ -3067,6 +3067,15 @@
> fi
>
> dnl
> +dnl Anevia RTSP plugin
> +dnl
> +AC_ARG_ENABLE(aneviartsp,
> + [ --enable-aneviartsp Anevia RTSP module (default disabled)])
> +if test "${enable_aneviartsp}" = "yes"; then
> + VLC_ADD_PLUGINS([access_aneviartsp])
> +fi
> +
> +dnl
> dnl MP4 module
> dnl
> AC_CHECK_HEADERS(zlib.h, [
> @@ -5617,6 +5626,7 @@
> modules/access/mms/Makefile
> modules/access/cdda/Makefile
> modules/access/rtsp/Makefile
> + modules/access/rtsp2/Makefile
> modules/access/vcd/Makefile
> modules/access/vcdx/Makefile
> modules/access/screen/Makefile
> Index: modules/access/rtsp2/access.c
> ===================================================================
> --- modules/access/rtsp2/access.c (revision 0)
> +++ modules/access/rtsp2/access.c (revision 0)
> @@ -0,0 +1,311 @@
> +/*****************************************************************************
> + * Preamble
> + *****************************************************************************/
> +#define _GNU_SOURCE
_GNU_SOURCE is already defined elsewhere in VLC.
Either include <config.h> or put
#ifndef _GNU_SOURCE
#define _GNU_SOURCE
#endif
> +#include <stdio.h>
> +#include <assert.h>
> +
> +#include <vlc/vlc.h>
> +#include <vlc/input.h>
> +#include <vlc_access.h>
> +
> +#include "network.h"
> +
> +#include "rtsp_client.h"
> +#include "ts_tools.h"
> +#include "access.h"
> +
> +
> +/*****************************************************************************
> + * Module descriptor
> + *****************************************************************************/
> +static int Open ( vlc_object_t * );
> +static void Close( vlc_object_t * );
> +
> +static int Seek( access_t *, int64_t );
> +static block_t *Block( access_t * );
> +static int Control( access_t *, int, va_list );
> +
> +#define CACHING_TEXT N_("Caching value (ms)")
> +#define CACHING_LONGTEXT N_( \
> + "Caching value for RTSP streams. This " \
> + "value should be set in milliseconds." )
> +
> +vlc_module_begin();
> + set_description( _("Anevia RTSP") );
> + set_shortname( _("Anevia RTSP") );
> + set_category( CAT_INPUT );
> + set_subcategory( SUBCAT_INPUT_ACCESS );
> + add_integer( "udp-caching", DEFAULT_PTS_DELAY / 1000, NULL, CACHING_TEXT,
> + CACHING_LONGTEXT, VLC_TRUE );
> + set_capability( "access2", 10 );
> + set_callbacks( Open, Close );
> + add_shortcut( "artsp" );
> + add_shortcut( "rtsp" );
> +vlc_module_end();
> +
> +
> +/*****************************************************************************
> + * Open: open the rtsp connection
> + *****************************************************************************/
> +static int Open( vlc_object_t *p_this )
> +{
> + access_t *p_access = (access_t *)p_this;
> + access_sys_t *p_sys;
> + rtsp_client_t *p_rtsp_client;
> + int i_error;
> + int i_sock_buff = 2000000;
> +
> + STANDARD_BLOCK_ACCESS_INIT;
> +
> + MALLOC_ERR( p_sys->p_ts_data, ts_data_t )
> + memset( p_sys->p_ts_data, 0, sizeof( ts_data_t ) );
> + MALLOC_ERR( p_sys->p_rtsp_client, rtsp_client_t );
On error you are leaking memory from p_sys->p_ts_data and
p_sys->p_rtsp_client. It is better to not use the MALLOC_ERR macros in
this situation. Just write it out.
> + memset( p_sys->p_rtsp_client, 0, sizeof( rtsp_client_t ) );
> +
> + /* Spawn keep-alive thread to send PLAY / PAUSE request periodically */
> + if( vlc_thread_create( p_access, "rtsp keep alive thread",
> + RtspKeepAliveThread,
> + VLC_THREAD_PRIORITY_LOW, VLC_FALSE ) )
> + {
Again leaking memory on exit.
> + /* FIXME: Memory not free'ed on error */
> + msg_Err( p_access, "cannot spawn rtsp keep alive thread" );
> + return VLC_EGENERIC;
> + }
> +
> + p_rtsp_client = p_sys->p_rtsp_client;
> + p_rtsp_client->p_access = p_access;
> + if ( ( i_error = RtspParseURL( p_access ) ) )
We write if( .. ) iso if ( .. )
> + {
> + Close( p_this );
> + return i_error;
Please fix indentation (4 spaces per level is the VLC convention)
> + }
> +
> + p_sys->i_mtu = var_CreateGetInteger( p_access, "mtu" );
> + if( p_sys->i_mtu <= 1 )
> + p_sys->i_mtu = 1500; /* Avoid problem */
> +
> + if ( RtspConnect( p_access ) )
> + {
> + Close( p_this );
> + return VLC_EGENERIC;
> + }
see comment above
> +
> + msg_Dbg( p_access, "rtsp connected" );
> +
> + /* SETUP and PLAY stream (using Anevia x-playnow command) */
> + if ( RtspPlay( p_access ) )
> + {
> + Close( p_this );
> + return VLC_EGENERIC;
> + }
> +
see comment above
> + /* Seeking available if we have stream length */
> + p_sys->b_seekable = p_rtsp_client->i_duration ? VLC_TRUE : VLC_FALSE;
> +
> +
> + /* UDP stream */
> + p_sys->i_stream_fd = net_OpenUDP( p_access, "localhost",
> + p_rtsp_client->i_port, 0, 0 );
> + if ( p_sys->i_stream_fd <= 0 )
> + {
> + msg_Err( p_access, "unable to open port %d for stream reception",
> + p_sys->i_stream_fd );
> + Close( p_this );
> + return VLC_EGENERIC;
> + }
see comment above
> +
> + /* Increase socket buffer size */
> + setsockopt( p_sys->i_stream_fd, SOL_SOCKET, SO_RCVBUF,
> + (void *) &i_sock_buff, sizeof( i_sock_buff ) );
> +
> + /* Update default_pts to a suitable value for udp access */
> + var_Create( p_access, "udp-caching", VLC_VAR_INTEGER | VLC_VAR_DOINHERIT );
> +
> + return VLC_SUCCESS;
> +}
> +
> +/*****************************************************************************
> + * Close: close the target
> + *****************************************************************************/
> +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;
> +
> + /* Clean RTSP disconnection */
> + RtspDisconnect( p_access );
> +
> + if( p_sys->i_stream_fd > 0 )
> + net_Close( p_sys->i_stream_fd );
> +
indentation
> + /* Waiting for keep-alive thread */
> + vlc_thread_join( p_access );
> +
> + /* Memory cleanings */
> + RtspDestroyData( p_access );
> + FREENULL( p_sys->p_ts_data );
> + FREENULL( p_sys );
> +}
> +
> +/*****************************************************************************
> + * Block
> + *****************************************************************************/
> +static block_t *Block( access_t *p_access )
> +{
> + access_sys_t *p_sys = p_access->p_sys;
> + rtsp_client_t *p_rtsp_client = p_sys->p_rtsp_client;
> + block_t *p_block;
> +
> + /* Read stream data */
> + p_block = block_New( p_access, p_sys->i_mtu );
> + p_block->i_buffer = 0;
> +
> + while ( p_block->i_buffer <= 0 && !p_access->b_die )
> + {
> + p_block->i_buffer = net_Read( p_access, p_sys->i_stream_fd, NULL,
> + p_block->p_buffer, p_sys->i_mtu,
> + VLC_FALSE );
indentation
> + }
> +
> + if ( p_access->b_die )
> + {
> + block_Release( p_block );
> + return NULL;
> + }
> +
> + /* Block is called once when seeking in pause, we ignore time */
> + if ( p_sys->b_seekable == VLC_TRUE && p_rtsp_client->i_state == PLAY )
> + {
> + /*
> + ** p_block contains several TS packets. We only check if the
> + ** first one is a PCR since we don't need all PCR values
> + */
> + GetPCRData( p_access, p_sys->p_ts_data, p_block->p_buffer );
> +
> + /* Compute new estimated size with PCR clock at 90 KHz */
> + if ( p_sys->p_ts_data->i_elapsed / 90000 != 0 )
> + {
> + p_access->info.i_size = p_rtsp_client->i_duration *
> + p_access->info.i_pos / ( p_sys->p_ts_data->i_elapsed / 90000 );
> + p_access->info.i_update |= INPUT_UPDATE_SIZE;
> + }
> +
> + p_access->info.i_pos += p_block->i_buffer;
> + p_access->info.i_update |= INPUT_UPDATE_SIZE;
> +
> + if ( p_sys->p_ts_data->i_elapsed / 90000 + 1 >=
> + p_rtsp_client->i_duration )
> + {
> + p_access->info.b_eof = VLC_TRUE;
> + p_access->b_die = VLC_TRUE;
> + }
> + }
> +
> + return p_block;
> +}
> +
> +
> +/*****************************************************************************
> + * Seek
> + *****************************************************************************/
> +static int Seek( access_t *p_access, int64_t i_pos )
> +{
> + rtsp_client_t *p_rtsp_client = p_access->p_sys->p_rtsp_client;
> + char *psz_mess = NULL;
> + mtime_t i_time;
> +
> + assert( p_access->info.i_size != 0 );
don't use assert since it will abort VLC (user will experience a crash),
but rather gracefully exit the function instead.
> + i_time = i_pos * p_rtsp_client->i_duration / p_access->info.i_size;
> +
> + if ( i_pos >= 0 && i_pos < p_access->info.i_size &&
> + i_time < p_rtsp_client->i_duration )
> + {
indentation in this block is wrong
> + asprintf( &psz_mess, "PLAY rtsp://%s/%s %s\r\n",
> + p_rtsp_client->psz_rtsp_sv,
> + p_rtsp_client->psz_path, RTSP_VERSION );
> + RtspWrite( p_access, psz_mess );
> + FREENULL( psz_mess );
> +
> + asprintf( &psz_mess, "Session: %s\r\n", p_rtsp_client->psz_session );
> + RtspWrite( p_access, psz_mess );
> + FREENULL( psz_mess );
> +
> + asprintf( &psz_mess, "Range: npt=%lld-\r\n\r\n", i_time );
> + RtspWrite( p_access, psz_mess );
> + FREENULL( psz_mess );
> +
> + p_access->info.i_pos = i_pos;
> + p_access->info.i_update |= INPUT_UPDATE_SIZE;
> +
> + if ( p_rtsp_client->i_state == PAUSE )
> + RtspPause( p_access );
> +
> + return RtspGetAnswers( p_access );
> + }
> +
> + p_access->info.b_eof = VLC_TRUE;
> + p_access->b_die = VLC_TRUE;
> +
> + 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;
> + rtsp_client_t *p_rtsp_client = p_sys->p_rtsp_client;
> + vlc_bool_t *pb_bool;
> + int *pi_int;
> + int64_t *pi_64;
> +
> + switch( i_query )
> + {
> + /* */
Please fix indentation in this block.
> + case ACCESS_CAN_PAUSE:
> + case ACCESS_CAN_CONTROL_PACE:
> + pb_bool = (vlc_bool_t*)va_arg( args, vlc_bool_t* );
> + *pb_bool = VLC_TRUE;
> + break;
> +
> + case ACCESS_CAN_SEEK:
> + pb_bool = (vlc_bool_t*)va_arg( args, vlc_bool_t* );
> + *pb_bool = p_sys->b_seekable;
> + break;
> +
> + case ACCESS_CAN_FASTSEEK:
> + pb_bool = (vlc_bool_t*)va_arg( args, vlc_bool_t* );
> + *pb_bool = VLC_FALSE;
> + break;
> +
> + case ACCESS_GET_MTU:
> + pi_int = (int*)va_arg( args, int * );
> + *pi_int = p_sys->i_mtu;
> + break;
> +
> + case ACCESS_GET_PTS_DELAY:
> + pi_64 = (int64_t*)va_arg( args, int64_t * );
> + *pi_64 = var_GetInteger( p_access, "udp-caching" ) * 1000;
> + break;
> +
> + case ACCESS_SET_PAUSE_STATE:
> + p_rtsp_client->i_state == PAUSE ? RtspPlay( p_access )
> + : RtspPause( p_access );
> + break;
> +
> + case ACCESS_SET_SEEKPOINT:
> + case ACCESS_GET_TITLE_INFO:
> + case ACCESS_SET_TITLE:
> + case ACCESS_SET_PRIVATE_ID_STATE:
> + case ACCESS_GET_META:
> + return VLC_EGENERIC;
> +
> + default:
> + msg_Warn( p_access, "unimplemented query in control" );
> + return VLC_EGENERIC;
> + }
> +
> + return VLC_SUCCESS;
> +}
> Index: modules/access/rtsp2/Modules.am
> ===================================================================
> --- modules/access/rtsp2/Modules.am (revision 0)
> +++ modules/access/rtsp2/Modules.am (revision 0)
> @@ -0,0 +1,8 @@
> +SOURCES_access_aneviartsp = \
> + access.c \
> + access.h \
> + rtsp_client.c \
> + rtsp_client.h \
> + ts_tools.c \
> + ts_tools.h \
> + $(NULL)
again indentation
> Index: modules/access/rtsp2/access.h
> ===================================================================
> --- modules/access/rtsp2/access.h (revision 0)
> +++ modules/access/rtsp2/access.h (revision 0)
> @@ -0,0 +1,14 @@
> +/*****************************************************************************
> + * Module internal structure
> + *****************************************************************************/
> +
> +struct access_sys_t
> +{
> + int i_stream_fd;
> + int i_mtu;
> +
> + ts_data_t *p_ts_data;
> + rtsp_client_t *p_rtsp_client;
> +
> + vlc_bool_t b_seekable;
> +};
> Index: modules/access/rtsp2/ts_tools.c
> ===================================================================
> --- modules/access/rtsp2/ts_tools.c (revision 0)
> +++ modules/access/rtsp2/ts_tools.c (revision 0)
> @@ -0,0 +1,120 @@
> +#include "ts_tools.h"
> +
> +static void DumpPAT( void *data, dvbpsi_pat_t *p_pat )
> +{
> + dvbpsi_pat_program_t *p_program = p_pat->p_first_program;
> + ts_data_t *p_ts_data = (ts_data_t *) data;
> +
> + p_ts_data->i_pmt_pid = p_program->i_pid;
> + p_ts_data->i_program_number = p_program->i_number;
> +
> + dvbpsi_DeletePAT( p_pat );
> +}
> +
> +static void DumpPMT( void *data, dvbpsi_pmt_t *p_pmt )
> +{
> + ts_data_t *p_ts_data = (ts_data_t *) data;
> +
> + p_ts_data->i_pcr_pid = p_pmt->i_pcr_pid;
> +
> + dvbpsi_DeletePMT( p_pmt );
> +}
> +
> +static uint16_t GetPacketPid( void *p_buffer )
> +{
> + uint8_t *p_data = (uint8_t *)p_buffer;
> +
> + return ( (uint16_t) ( p_data[1] & 0x1f) << 8 ) + p_data[2];
> +}
> +
> +static vlc_bool_t IsTSPacket( void *p_buffer )
> +{
> + uint8_t *p_data = (uint8_t *)p_buffer;
> +
> + return p_data[0] == 0x47;
> +}
> +
> +static void GetPMTPid( void *p_buffer, ts_data_t *p_ts_data )
> +{
> + dvbpsi_handle h_dvbpsi;
> +
> + h_dvbpsi = dvbpsi_AttachPAT( DumpPAT, p_ts_data );
> + dvbpsi_PushPacket( h_dvbpsi, p_buffer );
> + dvbpsi_DetachPAT( h_dvbpsi );
> +}
> +
> +static void GetPCRPid( void *p_buffer, ts_data_t *p_ts_data )
> +{
> + dvbpsi_handle h_dvbpsi;
> +
> + h_dvbpsi = dvbpsi_AttachPMT( p_ts_data->i_program_number,
> + DumpPMT, p_ts_data );
> + dvbpsi_PushPacket( h_dvbpsi, p_buffer );
> + dvbpsi_DetachPMT( h_dvbpsi );
> +}
> +
> +static void GetPCR( access_t *p_access, ts_data_t *p_ts_data, void *p_buffer )
> +{
> + const uint8_t *p = (uint8_t *)p_buffer;
> + mtime_t i_delta;
> +
> + if( ( p[3]&0x20 ) && /* adaptation */
> + ( p[5]&0x10 ) &&
> + ( p[4] >= 7 ) )
> + {
indentation
> + p_ts_data->i_prev_pcr = p_ts_data->i_pcr;
> + p_ts_data->i_pcr = ( (mtime_t)p[6] << 25 ) |
> + ( (mtime_t)p[7] << 17 ) |
> + ( (mtime_t)p[8] << 9 ) |
> + ( (mtime_t)p[9] << 1 ) |
> + ( (mtime_t)p[10] >> 7 );
> +
> + /* Waiting for at leat 2 values */
> + if ( p_ts_data->i_prev_pcr != 0 )
> + {
> + i_delta = p_ts_data->i_pcr - p_ts_data->i_prev_pcr;
> +
> + /* Checking time discontinuity */
> + if ( p_ts_data->i_prev_pcr > p_ts_data->i_pcr ||
> + i_delta > MAX_PCR_INTERVAL )
> + {
> + msg_Dbg( p_access, "PCR discontinuity (delta = %lld)", i_delta);
> + /* Adding mean value */
> + p_ts_data->i_elapsed +=
> + p_ts_data->i_elapsed / p_access->info.i_pos;
> + }
> + else
> + p_ts_data->i_elapsed += i_delta;
> + }
> + }
> +}
> +
> +void GetPCRData( access_t *p_access, ts_data_t *p_ts_data, void *p_buffer )
> +{
> + if( IsTSPacket( p_buffer ) )
> + {
indentation
> + /* Getting PAT -> PMT -> PCR */
> + if( p_ts_data->i_pcr_pid == 0 )
> + {
> + if( p_ts_data->i_pmt_pid == 0 && GetPacketPid( p_buffer ) == 0 )
> + {
> + msg_Dbg( p_access,
> + "Seen PAT packet, getting PMT packet pid" );
> + GetPMTPid( p_buffer, p_ts_data );
> + }
> + else
explicitly add braces here it prevents mistakes in maintanance.
{
> + if ( GetPacketPid( p_buffer ) == p_ts_data->i_pmt_pid )
> + {
> + msg_Dbg( p_access,
> + "Seen PMT packet, getting PCR packet pid" );
> + GetPCRPid( p_buffer, p_ts_data );
> + }
}
> + }
> + else
> + {
> + if ( GetPacketPid( p_buffer ) == p_ts_data->i_pcr_pid )
> + GetPCR( p_access, p_ts_data, p_buffer );
> + }
> + }
> +
> +}
> Index: modules/access/rtsp2/rtsp_client.c
> ===================================================================
> --- modules/access/rtsp2/rtsp_client.c (revision 0)
> +++ modules/access/rtsp2/rtsp_client.c (revision 0)
> @@ -0,0 +1,405 @@
> +#define _GNU_SOURCE
> +#include <stdio.h>
> +#include <assert.h>
> +
> +#include <vlc/vlc.h>
> +#include <vlc_interaction.h>
> +
> +#include "network.h"
> +
> +#include "rtsp_client.h"
> +#include "ts_tools.h"
> +#include "access.h"
> +
> +
> +/*****************************************************************************
> + * Network
> + *****************************************************************************/
> +int RtspConnect( access_t *p_access )
> +{
> + rtsp_client_t *p_rtsp_client = p_access->p_sys->p_rtsp_client;
> +
> + p_rtsp_client->i_rtsp_fd = net_ConnectTCP( p_access,
> + p_rtsp_client->psz_rtsp_sv,
> + p_rtsp_client->i_rtsp_port );
> +
> + if( p_rtsp_client->i_rtsp_fd < 0 )
> + {
indentation
> + intf_UserFatal( p_access, VLC_FALSE, _("Connection failed"),
> + _("VLC could not connect to %s:%d."),
> + p_rtsp_client->psz_rtsp_sv, p_rtsp_client->i_rtsp_port );
> + return VLC_EGENERIC;
> + }
> +
> + msg_Dbg( p_access, "connected to %s:%d",
> + p_rtsp_client->psz_rtsp_sv, p_rtsp_client->i_rtsp_port );
> +
> + return VLC_SUCCESS;
> +}
> +
> +int RtspDisconnect( access_t *p_access )
> +{
> + rtsp_client_t *p_rtsp_client = p_access->p_sys->p_rtsp_client;
> +
> + /*
> + ** FIXME: Can't send / receive data if b_die == VLC_TRUE
> + ** There's probably a good reason, but we _must_ send TEARDOWN qtoo
type qtoo -> too
> + */
> + p_access->b_die = VLC_FALSE;
> +
> + if ( p_rtsp_client->i_state != STOP )
> + RtspTeardown( p_access );
> +
indentation
> + if( p_rtsp_client->i_rtsp_fd > 0 )
> + net_Close( p_rtsp_client->i_rtsp_fd );
> +
> + p_access->b_die = VLC_TRUE;
> +
> + return VLC_SUCCESS;
> +}
> +
> +int RtspWrite( access_t *p_access, char *p_buffer )
> +{
> + rtsp_client_t *p_rtsp_client = p_access->p_sys->p_rtsp_client;
> +
> + net_Printf( VLC_OBJECT(p_access), p_rtsp_client->i_rtsp_fd, 0, "%s", p_buffer );
> +
> + msg_Dbg( p_access, ">> %s", p_buffer );
> +
> + return VLC_SUCCESS;
> +}
> +
> +char *RtspReadLine( access_t *p_access )
> +{
> + return net_Gets( VLC_OBJECT( p_access ),
> + p_access->p_sys->p_rtsp_client->i_rtsp_fd, 0 );
> +}
> +
> +int RtspGetAnswers( access_t *p_access )
> +{
> + rtsp_client_t *p_rtsp_client = p_access->p_sys->p_rtsp_client;
> + int i_mess;
> + int i_ret = 0;
> + char *psz_line = NULL;
> + char *psz_mess = NULL;
> +
> + psz_line = RtspReadLine( p_access );
> + while ( psz_line && strlen( psz_line ) != 0 )
> + {
> + msg_Dbg( p_access, "<< %s", psz_line );
> + i_mess = 0;
> + while ( !i_ret && p_messages[i_mess].mess != NULL )
> + {
> + psz_mess = strstr( psz_line, p_messages[i_mess].mess );
> + if ( psz_mess == psz_line )
> + break;
> + ++i_mess;
> + }
> + if ( !i_ret && p_messages[i_mess].handle != NULL )
> + i_ret |= p_messages[i_mess].handle( p_rtsp_client,
> + strchr( psz_mess, ' ' ) + 1 );
> + FREENULL( psz_line );
psz_mess = NULL;
> + psz_line = RtspReadLine( p_access );
> + }
> + return i_ret;
> +}
> +
> +
> +/*****************************************************************************
> + * RTSP User commands
> + *****************************************************************************/
> +int RtspPlay( access_t *p_access )
> +{
> + rtsp_client_t *p_rtsp_client = p_access->p_sys->p_rtsp_client;
> + char *psz_mess = NULL;
> + char *psz_session = NULL;
> +
> + if ( p_rtsp_client->i_state == STOP )
> + {
> + asprintf( &psz_mess, "SETUP rtsp://%s/%s %s\r\n", p_rtsp_client->psz_rtsp_sv,
> + p_rtsp_client->psz_path, RTSP_VERSION );
> +
> + RtspWrite( p_access, psz_mess );
> + RtspWrite( p_access, "x-playNow:\r\n\r\n" );
> + }
> + else /* PLAY or PAUSE */
> + {
> + asprintf( &psz_mess, "PLAY rtsp://%s/%s %s\r\n", p_rtsp_client->psz_rtsp_sv,
> + p_rtsp_client->psz_path, RTSP_VERSION );
> + asprintf( &psz_session, "Session: %s\r\n\r\n", p_rtsp_client->psz_session );
> +
> + RtspWrite( p_access, psz_mess );
> + RtspWrite( p_access, psz_session );
> + }
> + p_rtsp_client->i_state = PLAY;
> +
> + FREENULL( psz_mess );
> + FREENULL( psz_session );
> +
> + return RtspGetAnswers( p_access );
> +}
> +
> +int RtspPause( access_t *p_access )
> +{
> + rtsp_client_t *p_rtsp_client = p_access->p_sys->p_rtsp_client;
> + char *psz_mess = NULL;
> + char *psz_session = NULL;
> +
> + asprintf( &psz_mess, "PAUSE rtsp://%s/%s %s\r\n", p_rtsp_client->psz_rtsp_sv,
> + p_rtsp_client->psz_path, RTSP_VERSION );
> + asprintf( &psz_session, "Session: %s\r\n\r\n", p_rtsp_client->psz_session );
> +
> + RtspWrite( p_access, psz_mess );
> + RtspWrite( p_access, psz_session );
> +
> + p_rtsp_client->i_state = PAUSE;
> +
> + FREENULL( psz_mess );
> + FREENULL( psz_session );
> +
> + return RtspGetAnswers( p_access );
> +}
> +
> +int RtspTeardown( access_t *p_access )
> +{
> + rtsp_client_t *p_rtsp_client = p_access->p_sys->p_rtsp_client;
> + char *psz_msg = NULL;
> +
> + asprintf( &psz_msg, "TEARDOWN rtsp://%s/%s %s\r\n", p_rtsp_client->psz_rtsp_sv,
> + p_rtsp_client->psz_path, RTSP_VERSION );
> + RtspWrite( p_access, psz_msg );
> + FREENULL( psz_msg );
> +
> + asprintf( &psz_msg, "Session: %s\r\n\r\n", p_rtsp_client->psz_session );
> + RtspWrite( p_access, psz_msg );
> + FREENULL( psz_msg );
> +
> + p_rtsp_client->i_state = STOP;
> +
> + return RtspGetAnswers( p_access );
> +}
> +
> +
> +/*****************************************************************************
> + * Anevia Manager messages
> + *****************************************************************************/
> +int MsgOk( rtsp_client_t *p_rtsp_client, char *psz_mess )
> +{
> + psz_mess = psz_mess; /* Well... */
this is bogus ^^^^ remove it or write it to a
msg_Dbg( p_rtsp_client->p_access, "%s:%s\n",
p_rtsp_client->psz_rtsp_sv, psz_mess );
or remove psz_mess as argument from the function.
> + if ( p_rtsp_client->i_state == STOP )
> + p_rtsp_client->i_state = PLAY;
> +
> + return VLC_SUCCESS;
> +}
> +
> +int MsgNotFound( rtsp_client_t *p_rtsp_client, char *psz_mess )
> +{
> + msg_Err( p_rtsp_client->p_access, "%s:%s\n",
> + p_rtsp_client->psz_rtsp_sv, psz_mess );
> +
> + return VLC_EGENERIC;
> +}
> +
> +int MsgNotAv( rtsp_client_t *p_rtsp_client, char *psz_mess )
> +{
> + msg_Err( p_rtsp_client->p_access, "%s:%s\n",
> + p_rtsp_client->psz_rtsp_sv, psz_mess );
> +
> + return VLC_EGENERIC;
> +}
> +
> +int MsgSession( rtsp_client_t *p_rtsp_client, char *psz_mess )
> +{
> + char *psz_egal = strrchr( psz_mess, '=' );
> +
> + /* Session */
> + if ( p_rtsp_client->psz_session == NULL )
> + p_rtsp_client->psz_session = strdup( psz_mess );
> +
> + if ( p_rtsp_client->psz_session != NULL &&
> + strcmp( psz_mess, p_rtsp_client->psz_session ) )
> + {
> + msg_Warn( p_rtsp_client->p_access,
> + "Session mismatch, setting new session" );
> + free( p_rtsp_client->psz_session );
> + p_rtsp_client->psz_session = strdup( psz_mess );
> + }
> +
> + /* Timeout */
> + if ( psz_egal == NULL )
> + p_rtsp_client->i_timeout = DEFAULT_RTSP_TIMEOUT;
> + else
> + {
> + int i_len = strlen( psz_egal + 1 );
> + char psz_timeout[i_len + 1];
> +
> + psz_timeout[i_len] = '\0';
> + strncpy( psz_timeout, psz_egal + 1, i_len );
> + p_rtsp_client->i_timeout = strtol( psz_timeout, NULL, 10 );
> + }
> +
> + return VLC_SUCCESS;
> +}
> +
> +int MsgServerID( rtsp_client_t *p_rtsp_client, char *psz_mess )
> +{
> +
> + if ( p_rtsp_client->psz_sv_id == NULL )
> + {
> + if ( strstr( psz_mess, "Anevia" ) == NULL )
> + {
> + msg_Dbg( p_rtsp_client->p_access,
> + "only Anevia RTSP servers supported" );
> + return VLC_EGENERIC;
> + }
> +
> + p_rtsp_client->psz_sv_id = strdup( psz_mess );
> + msg_Dbg( p_rtsp_client->p_access, "found Anevia RTSP server" );
> + }
> +
> + return VLC_SUCCESS;
> +}
> +
> +
> +int MsgRange( rtsp_client_t *p_rtsp_client, char *psz_mess )
> +{
> + char *psz_egal = strchr( psz_mess, '=' );
> + char *psz_dash = strchr( psz_mess, '-' );
> + int i_len;
> +
> + /* Getting film length */
> + if ( p_rtsp_client->i_duration == 0 )
> + {
> + i_len = psz_dash - psz_mess;
> +
> + char psz_length[i_len + 1];
> +
> + strncpy( psz_length, psz_dash + 1, i_len );
> + psz_length[i_len] = '\0';
> + p_rtsp_client->i_duration = strtol( psz_length, NULL, 10 );
> + }
> +
> + assert( psz_dash != NULL );
> +
> + i_len = psz_dash - psz_egal - 1;
> +
> + char psz_start[i_len + 1];
> + mtime_t i_start;
> +
> + strncpy( psz_start, psz_egal + 1, i_len );
> + psz_start[i_len] = '\0';
> + i_start = strtol( psz_start, NULL, 10 );
> +
> + /* Seek */
> + if ( p_rtsp_client->p_access->p_sys->b_seekable )
> + p_rtsp_client->p_access->p_sys->p_ts_data->i_elapsed = i_start * 90000;
> +
> + return VLC_SUCCESS;
> +}
> +
> +int MsgTransport( rtsp_client_t *p_rtsp_client, char *psz_mess )
> +{
> + char *psz_port = strstr( psz_mess, "client_port=" ) + 12;
> + char *psz_semi_colon = strchr( psz_port, ';' );
> +
> + assert( psz_port != NULL && psz_semi_colon != NULL && psz_semi_colon > psz_port );
> +
> + int i_len = psz_semi_colon - psz_port;
> + char psz_value[i_len + 1];
> +
> + strncpy( psz_value, psz_port, i_len );
> + psz_value[i_len] = '\0';
> + p_rtsp_client->i_port = strtol( psz_value, NULL, 10 );
> +
> + return VLC_SUCCESS;
> +}
> +
> +
> +void RtspDestroyData( access_t *p_access )
> +{
> + rtsp_client_t *p_rtsp_client = p_access->p_sys->p_rtsp_client;
> +
> + FREENULL( p_rtsp_client->psz_rtsp_sv );
> + FREENULL( p_rtsp_client->psz_path );
> + FREENULL( p_rtsp_client->psz_session );
> + FREENULL( p_rtsp_client->psz_sv_id );
> + FREENULL( p_rtsp_client );
> +}
> +
> +int RtspParseURL( access_t *p_access )
> +{
> + rtsp_client_t *p_rtsp_client = p_access->p_sys->p_rtsp_client;
> + char *psz_path = p_access->psz_path;
> + char *psz_slash, *psz_colon, *psz_port, *psz_error[1];
char *psz_slash = NULL;
char *psz_colon = NULL;
char *psz_port = NULL;
char *psz_error[1];
> + uint16_t i_len;
> +
indentation needs fixing in this block
> + if ( psz_path == NULL )
> + return VLC_EGENERIC;
> +
> + psz_slash = strchr( psz_path, '/' );
> + psz_colon = strchr( psz_path, ':' );
> +
> + /* No path specified */
> + if ( psz_slash == NULL )
> + return VLC_EGENERIC;
> +
> + /* Server name */
> + i_len = psz_colon == NULL ? psz_slash - psz_path : psz_colon - psz_path;
> + if ( !( p_rtsp_client->psz_rtsp_sv = malloc( sizeof(char) * ( i_len + 1 ) ) ) )
better separate this if(..) for better reading
p_rtsp_client->psz_rtsp_sv = malloc( sizeof(char)*(i_len + 1) );
if ( !(p_rtsp_client->psz_rtsp_sv) )
> + return VLC_ENOMEM;
> + strncpy( p_rtsp_client->psz_rtsp_sv, psz_path, i_len );
> + p_rtsp_client->psz_rtsp_sv[i_len] = '\0';
> +
> + /* Port */
> + if( psz_colon == NULL || psz_slash < psz_colon )
> + p_rtsp_client->i_rtsp_port = DEFAULT_RTSP_PORT;
> + else
> + {
> + i_len = psz_slash - ( psz_colon + 1 );
> + if ( !( psz_port = malloc( sizeof(char) * ( i_len + 1 ) ) ) )
psz_port = malloc( sizeof(char) * ( i_len + 1 ) );
if ( !psz_port )
> + return VLC_ENOMEM;
> + strncpy( psz_port, psz_colon + 1, i_len );
> + psz_port[i_len] = '\0';
> + p_rtsp_client->i_rtsp_port = strtoul( psz_port, psz_error, 10 );
where is psz_error given a value?? not in this function. So its content
is here actually undefined.
> + if ( psz_port[0] == '\0' || p_rtsp_client->i_rtsp_port > 65535 ||
> + **psz_error != '\0' )
> + p_rtsp_client->i_rtsp_port = DEFAULT_RTSP_PORT;
> + free( psz_port );
> + }
> +
> + /* Path */
> + i_len = &psz_path[strlen( psz_path )] - ( psz_slash + 1 );
> + if ( i_len == 0 ) /* No path specified (trailing slash) */
> + return VLC_EGENERIC;
> + if ( !( p_rtsp_client->psz_path = malloc( sizeof(char) * ( i_len + 1 ) ) ) )
p_rtsp_client->psz_path = malloc( sizeof(char) * ( i_len + 1 ) );
if ( !(p_rtsp_client->psz_path) )
> + return VLC_ENOMEM;
> + strncpy( p_rtsp_client->psz_path, psz_slash + 1, i_len );
> + p_rtsp_client->psz_path[i_len] = '\0';
> +
> + return 0;
> +}
> +
> +void RtspKeepAliveThread( access_t *p_access )
> +{
> + rtsp_client_t *p_rtsp_client = p_access->p_sys->p_rtsp_client;
> + time_t i_current;
> +
> + while ( !p_access->b_die && p_rtsp_client->psz_session == NULL )
> + ;
Isn't it better to return a message eg: DESCRIBE or PING to the anevia
RTSP manager iso busy looping here?
> +
> + while ( !p_access->b_die )
> + {
> + i_current = time( NULL );
> + if ( p_rtsp_client->i_timer_start == 0 )
> + p_rtsp_client->i_timer_start = i_current;
> + else
> + if ( i_current - p_rtsp_client->i_timer_start >
> + (int32_t) ( p_rtsp_client->i_timeout * 0.80 ) )
> + {
> + p_rtsp_client->i_timer_start = i_current;
> + p_rtsp_client->i_state == PAUSE ? RtspPause( p_access )
> + : RtspPlay( p_access );
> + }
> + msleep( 1000 );
> + }
> +}
> Index: modules/access/rtsp2/ts_tools.h
> ===================================================================
> --- modules/access/rtsp2/ts_tools.h (revision 0)
> +++ modules/access/rtsp2/ts_tools.h (revision 0)
> @@ -0,0 +1,44 @@
> +#ifndef HAVE_TS_TOOLS_H
> +# define HAVE_TS_TOOLS_H
> +
> +# include <vlc/vlc.h>
> +# include <vlc_access.h>
> +
> +/* Include dvbpsi headers */
> +# ifdef HAVE_DVBPSI_DR_H
> +# include <dvbpsi/dvbpsi.h>
> +# include <dvbpsi/demux.h>
> +# include <dvbpsi/descriptor.h>
> +# include <dvbpsi/pat.h>
> +# include <dvbpsi/pmt.h>
> +# include <dvbpsi/sdt.h>
> +# include <dvbpsi/dr.h>
> +# include <dvbpsi/psi.h>
> +# else
> +# include "dvbpsi.h"
> +# include "demux.h"
> +# include "descriptor.h"
> +# include "tables/pat.h"
> +# include "tables/pmt.h"
> +# include "tables/sdt.h"
> +# include "descriptors/dr.h"
> +# include "psi.h"
> +# endif
> +
> +typedef struct
> +{
> + uint16_t i_pmt_pid;
> + uint16_t i_pcr_pid;
> + uint16_t i_program_number;
> +
> + mtime_t i_pcr;
> + mtime_t i_prev_pcr;
> + mtime_t i_elapsed;
> +} ts_data_t;
> +
> +static const int TS_PACKET_SIZE = 188;
> +static const int MAX_PCR_INTERVAL = 200000;
> +
> +void GetPCRData( access_t *p_access, ts_data_t *p_ts_data, void *p_buffer );
> +
> +#endif
> Index: modules/access/rtsp2/rtsp_client.h
> ===================================================================
> --- modules/access/rtsp2/rtsp_client.h (revision 0)
> +++ modules/access/rtsp2/rtsp_client.h (revision 0)
> @@ -0,0 +1,86 @@
> +#ifndef HAVE_RTSP_CLIENT_H
> +# define HAVE_RTSP_CLIENT_H 1
> +
> +typedef enum { STOP, PLAY, PAUSE } state_t;
> +
> +typedef struct
> +{
> + int i_rtsp_fd;
> + char *psz_rtsp_sv;
> + uint32_t i_rtsp_port;
> + char *psz_path;
> + char *psz_session;
> + uint16_t i_port;
> + char *psz_sv_id;
> + mtime_t i_duration;
> + mtime_t i_pos;
> + uint16_t i_timeout;
> + time_t i_timer_start;
> + state_t i_state;
> + access_t *p_access;
> +} rtsp_client_t;
> +
> +/*****************************************************************************
> + * Constants
> + *****************************************************************************/
> +static const int DEFAULT_RTSP_PORT = 554;
> +static const int DEFAULT_RTSP_TIMEOUT = 60;
> +static const char RTSP_VERSION[] = "RTSP/1.0";
> +
> +
> +/*****************************************************************************
> + * Network
> + *****************************************************************************/
> +int RtspConnect( access_t * );
> +int RtspDisconnect( access_t * );
> +int RtspWrite( access_t *, char * );
> +char *RtspReadLine( access_t * );
> +int RtspGetAnswers( access_t * );
> +
> +
> +/*****************************************************************************
> + * RTSP User commands
> + *****************************************************************************/
> +int RtspPlay( access_t * );
> +int RtspPause( access_t * );
> +int RtspTeardown( access_t * );
> +
> +
> +/*****************************************************************************
> + * Anevia Manager messages
> + *****************************************************************************/
> +typedef int (*mess_fun_t)( rtsp_client_t *, char * );
> +
> +typedef struct
> +{
> + char *mess;
> + mess_fun_t handle;
> +} rtsp_message_t;
Just write:
typedef struct
{
char *mess;
int (*handle)( rtsp_client_t *, char * );
} rtsp_message_t;
> +
> +int MsgOk( rtsp_client_t *, char * );
> +int MsgNotFound( rtsp_client_t *, char * );
> +int MsgNotAv( rtsp_client_t *, char * );
> +int MsgSession( rtsp_client_t *, char * );
> +int MsgServerID( rtsp_client_t *, char * );
> +int MsgRange( rtsp_client_t *, char * );
> +int MsgTransport( rtsp_client_t *, char * );
> +
> +static const rtsp_message_t p_messages[] =
> + {
> + /* RTSP Answer */
> + { "RTSP/1.0 200", MsgOk },
> + { "RTSP/1.0 404", MsgNotFound },
> + { "RTSP/1.0 503", MsgNotAv },
> + { "Session: ", MsgSession },
> + { "Server: ", MsgServerID },
> + { "Range: ", MsgRange },
> + { "Transport: ", MsgTransport },
> + { NULL, NULL }
> + };
> +
> +
> +int RtspParseURL( access_t * );
> +void RtspKeepAliveThread( access_t * );
> +void RtspDestroyData( access_t * );
> +
> +#endif
I am sure there are probably more issues in this piece of code. However
I do not have the time and equipement to test it that thouroughly. Maybe
it is a good idea to run valgrind when doing your test with the anevia
manager.
Gtz,
Jean-Paul Saman.
--
This is the vlc-devel mailing-list, see http://www.videolan.org/vlc/
To unsubscribe, please read http://developers.videolan.org/lists.html
More information about the vlc-devel
mailing list