[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