[FFmpeg-devel] [PATCH] RTP local udp port issue fix (ticket 916)

[Dmitry Volyntsev]

Here is new patch according Luca's proposals

diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
index d32f49e..ecc0e80 100644
--- a/libavformat/rtsp.c
+++ b/libavformat/rtsp.c
@@ -1103,7 +1103,7 @@ int ff_rtsp_make_setup_request(AVFormatContext
*s, const char *host, int port,
                               int lower_transport, const char *real_challenge)
 {
     RTSPState *rt = s->priv_data;
-    int rtx = 0, j, i, err, interleave = 0;
+    int rtx = 0, j, i, err, interleave = 0, port_off;
     RTSPStream *rtsp_st;
     RTSPMessageHeader reply1, *reply = &reply1;
     char cmd[2048];
@@ -1120,8 +1120,9 @@ int ff_rtsp_make_setup_request(AVFormatContext
*s, const char *host, int port,
     /* for each stream, make the setup request */
     /* XXX: we assume the same server is used for the control of each
      * RTSP stream */
-
-    for (j = RTSP_RTP_PORT_MIN, i = 0; i < rt->nb_rtsp_streams; ++i) {
+    port_off = av_get_random_seed()%(RTSP_RTP_PORT_MAX - RTSP_RTP_PORT_MIN);
+    port_off-= port_off & 0x01; /* even random offset */
+    for (j = RTSP_RTP_PORT_MIN + port_off, i = 0; i <
rt->nb_rtsp_streams; ++i) {
         char transport[2048];

         /*
@@ -1158,16 +1159,15 @@ int ff_rtsp_make_setup_request(AVFormatContext
*s, const char *host, int port,
             }

             /* first try in specified port range */
-            if (RTSP_RTP_PORT_MIN != 0) {
-                while (j <= RTSP_RTP_PORT_MAX) {
-                    ff_url_join(buf, sizeof(buf), "rtp", NULL, host, -1,
-                                "?localport=%d", j);
-                    /* we will use two ports per rtp stream (rtp and rtcp) */
-                    j += 2;
-                    if (ffurl_open(&rtsp_st->rtp_handle, buf,
AVIO_FLAG_READ_WRITE,
-                                   &s->interrupt_callback, NULL) == 0)
-                        goto rtp_opened;
-                }
+            while (j <= RTSP_RTP_PORT_MAX) {
+            	ff_url_join(buf, sizeof(buf), "rtp", NULL, host, -1,
+            			"?localport=%d", j);
+            	/* we will use two ports per rtp stream (rtp and rtcp) */
+            	j += 2;
+            	if (ffurl_open(&rtsp_st->rtp_handle, buf,
+            			AVIO_FLAG_READ_WRITE,
+            			&s->interrupt_callback, NULL) == 0)
+            		goto rtp_opened;
             }

             av_log(s, AV_LOG_ERROR, "Unable to open an input RTP port\n");
diff --git a/libavformat/rtsp.h b/libavformat/rtsp.h
index 36de7e5..e11dec9 100644
--- a/libavformat/rtsp.h
+++ b/libavformat/rtsp.h
@@ -70,7 +70,7 @@ enum RTSPControlTransport {
 #define RTSP_DEFAULT_NB_AUDIO_CHANNELS 1
 #define RTSP_DEFAULT_AUDIO_SAMPLERATE 44100
 #define RTSP_RTP_PORT_MIN 5000
-#define RTSP_RTP_PORT_MAX 10000
+#define RTSP_RTP_PORT_MAX 65000

 /**
  * This describes a single item in the "Transport:" line of one stream as


2012/1/17 Luca Abeni <lucabe72 at email.it>:
> On 01/17/2012 09:02 AM, Dmitry Volyntsev wrote:
> [...]
>>
>> 2012/1/17 Luca Abeni<lucabe72 at email.it>:
>>
>>> I have currently no time to look at all the details... But there is
>>> something
>>> wrong in what's happening: when the second ffmpeg instance tries to use
>>> port
>>> 5000 for the RTP port, ffurl_open() should fail. And so port 5002 is
>>> tried
>>> (thanks to the "while (j<= RTSP_RTP_PORT_MAX) {" loop) and so on...
>>> Hence, there should be no collision.
>>>
>>> Maybe libavformat ends up using SO_REUSEADDR for some reason? I'd check
>>> that...
>>>
>>> I think the proposed patch might be wrong, because it might end up in
>>> using
>>> odd
>>> numbers for the RTP port (I think RTP ports should be even numbers, and
>>> RTCP
>>> ports should be odd).
>>>
>>>
>>>                                Luca
>>> _______________________________________________
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel at ffmpeg.org
>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>>
>>
>>> when the second ffmpeg instance tries to use port
>>
>>  5000 for the RTP port, ffurl_open() should fail. And so port 5002 is
>> tried
>> It is true, but only if the seconds instance starts while first
>> instance still active. But the issue little-bit differs.
>>
>> If fact, the second instance starts when first instance already
>> stopped (killed or something), so it closes opened udp ports. But (it
>> is important for the issue), remote side still sends data to rtp/rtcp
>> ports.
>
>
> Ah, sorry... I missed this detail.
>
>
>
>>> I think the proposed patch might be wrong, because it might end up in
>>> using
>>> odd numbers for the RTP port (I think RTP ports should be even numbers,
>>> and RTCP
>>> ports should be odd).
>>
>> Yes, you are right. I'm going consider this requirement in the next patch.
>
>
> I suspect the simplest thing to do to avoid this issue is to add a random
> offset
> to RTSP_RTP_PORT_MIN: instead of
>        for (j = RTSP_RTP_PORT_MIN, i = 0; i < rt->nb_rtsp_streams; ++i) {
> you can use
>        offset = <random number>
>        for (j = RTSP_RTP_PORT_MIN + offset, i = 0; i < rt->nb_rtsp_streams;
> ++i) {
>
>
> BTW, unrelated to this issue, but... I think there is something quite wrong
> in this code: things like
>        if (RTSP_RTP_PORT_MIN != 0) {
> look _very_ strange...
> and there is an
>        extern int rtsp_rtp_port_min;
> in rtsp.h... But there is no "rtsp_rtp_port_min" variable anywhere.
>
>
>
>                        Luca



-- 
Be happy,
Best regards,
Dmitry Volyntsev