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

Dmitry Volyntsev xeioexception at gmail.com
Tue Jan 17 16:03:39 CET 2012


I'm sorry for bad previous patch quality.

Here is in attach fixed one (previous patch with fixes against
tools/patcheck and tools/clean-diff)

2012/1/17 Dmitry Volyntsev <xeioexception at gmail.com>:
> 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ffmpeg_patch_rtp_randomize_local_port.diff
Type: text/x-patch
Size: 2794 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120117/cfa3a898/attachment.bin>


More information about the ffmpeg-devel mailing list