[FFmpeg-devel] [PATCH] RTP local udp port issue fix (ticket 916)
Luca Abeni
lucabe72 at email.it
Tue Jan 17 17:29:26 CET 2012
On 17/01/12 16:03, Dmitry Volyntsev wrote:
> 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)
I think it should be 2 separate patches (one fixing the issue you are
seeing, and the other one fixing the "if (RTSP_RTP_PORT_MIN != 0) {"
thing), but the modifications look ok to me (assuming that you tested it
and that the random number generation is ok).
Luca
>
> 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
>>
>
More information about the ffmpeg-devel
mailing list