[FFmpeg-devel] [rtsp/rtp] update rtp to support rfc3550

[Martin Storsjö]

On Thu, 15 Apr 2010, Luca Barbato wrote:

> Luca A, Martin, Ronald your round to provide feedbacks =)

> @@ -157,16 +178,14 @@ static int rtp_open(URLContext *h, const char *uri, int flags)
>      }
>  
>      build_udp_url(buf, sizeof(buf),
> -                  hostname, port, local_port, ttl, max_packet_size);
> +                  hostname, rtp_port, local_rtp_port, ttl, max_packet_size);
>      if (url_open(&s->rtp_hd, buf, flags) < 0)
>          goto fail;
> -    local_port = udp_get_local_port(s->rtp_hd);
> -    /* XXX: need to open another connection if the port is not even */
> -
> -    /* well, should suppress localport in path */
> +    if (local_rtp_port>=0 && local_rtcp_port<0)
> +        local_rtcp_port = udp_get_local_port(s->rtp_hd) + 1;

Hmm, doesn't this assign something to local_rtcp_port only if something 
was assigned to local_rtp_port initially? That is, if the caller didn't 
set local_rtp_port, it would choose local_rtcp_port randomly instead of 
fetching it?

That is, should the last two lines above be:

   if (local_rtcp_port<0)
        local_rtcp_port = udp_get_local_port(s->rtp_hd) + 1;

So I guess that would make it (almost) work even without the loop for 
choosing port numbers with DSS, as long as the OS assigned port is even? 

That, on the other hand, may give problems if local_rtp+1 isn't available 
- should the code be allowed to retry with any one if localrtcpport wasn't 
specified? That way, we'd get the current behaviour with n,n+1 as long as 
it's possible, but still work well with those servers compliant with 
rfc3550 if unable to allocate the second port consecutively?


>              if (RTSP_RTP_PORT_MIN != 0) {
> -                while (j <= RTSP_RTP_PORT_MAX) {
> +               while (j <= RTSP_RTP_PORT_MAX) {

Unintentional indentation change?

Except for that, I think it looks quite sensible. I'm a bit undecided 
regarding the port choosing loop in rtsp.c - if we could tell the UDP 
protocol that "let the OS assign a local port number, any one, but an even 
one", we'd be all set. :-)

// Martin