[MPlayer-dev-eng] [PATCH] New native RTSP demuxer

[Roberto Togni]

On Thu, 15 Jun 2006 20:02:51 +0200
Benjamin Zores <ben at geexbox.org> wrote:

> On Wed, 14 Jun 2006 02:37:17 +0200
> Luca Barbato <lu_zero at gentoo.org> wrote:
> 
> > Reviewing it could take less if you split it in different chunks.
> 
> Ok so this is the first patch of the split in order to everyone to
> see what's actually going on in the patch.
> 
> This part fixes several error from librtsp (and also changes some
> things a bit for being used by the following parts of the patch).
> 

[...]

> Index: libmpdemux/realrtsp/rtsp.c
> ===================================================================
> --- libmpdemux/realrtsp/rtsp.c	(révision 18722)
> +++ libmpdemux/realrtsp/rtsp.c	(copie de travail)

[...]

>   * constants
>   */
>  
> -const char rtsp_protocol_version[]="RTSP/1.0";
> +#define RTSP_PROTOCOL_VERSION "RTSP/1.0"
>  

Why? Isn't it better to move all the text commands (RTSP_) as
static const instead of #define them?

>  /* server states */
>  #define RTSP_CONNECTED 1
[...]

> -int rtsp_request_setup(rtsp_t *s, const char *what) {
> +int rtsp_request_setup(rtsp_t *s, const char *what, char *control) {
>  
> -  rtsp_send_request(s,"SETUP",what);
> +  char *buf = NULL;
> +
> +  if (what)
> +    buf = strdup (what);
> +  else
> +  {
> +    int len = strlen (s->host) + strlen (s->path) + 16;
> +    if (control)
> +      len += strlen (control) + 1;
> +    
> +    buf = malloc (len * sizeof (char));

Please remove all the sizeof(char), it's useless and misleading when
auditing memory allocations.

> +    sprintf (buf, "rtsp://%s:%i/%s%s%s", s->host, s->port, s->path,
> +             control ? "/" : "", control ? control : "");
> +  }
>    

[...]

> @@ -752,6 +793,11 @@
>  
>  }
>  
> +char *rtsp_get_host (rtsp_t * s)
> +{
> +  return s->host;
> +}
> + 

Ok if it's going to be replaced with something more complex in your
later patches, else I don't think we need a function to get a field
from a struct :)

[...]

I still haven't tested the patch, but if it works you can commit it.

Ciao,
 Roberto