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

[Luca Barbato]

On 04/15/2010 09:41 PM, Martin Storsj? wrote:
> 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? 

I'd rather keep the logic as is and keep dss happy either by having a
quirk for it or by using a fallback.

> 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?

I'm not sure which way is better, I was thinking about using a fallback
mode by first trying rfc3550 and then looping over a port range to pick
something working within the portpair constraint. The possibility of
having the server deplete its usable ports is higher than a client usually.

>>              if (RTSP_RTP_PORT_MIN != 0) {
>> -                while (j <= RTSP_RTP_PORT_MAX) {
>> +               while (j <= RTSP_RTP_PORT_MAX) {
> 
> Unintentional indentation change?

indeed ^^;

> 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. :-)

sadly doesn't seem possible =|

lu

-- 

Luca Barbato
Gentoo/linux
http://dev.gentoo.org/~lu_zero