diff mbox series

[FFmpeg-devel] libavformat/rtsp.c: fix RTSP not setup the UDP stream in SDP

Message ID CA+s8LNjmy2RaOdeSsLoZDjmH8cGfFvJGWzXXAv-YwMtNUwygTQ@mail.gmail.com
State New
Headers show
Series [FFmpeg-devel] libavformat/rtsp.c: fix RTSP not setup the UDP stream in SDP | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Chiu, Yung-Hsiang July 30, 2021, 2:55 a.m. UTC
Hi,

Our RTSP video server reply two streams in SDP are UDP and RTP/AVP/UDP.
>> m=video 0 UDP 33
>> m=video 0 RTP/AVP/UDP 33

ffmpeg now setup twice with the same transport "RTP/AVP/UDP"

The first time is
>> SETUP rtsp://192.168.1.100:554/MOV_000012353521.mpg RTSP/1.0
>> Transport: RTP/AVP/UDP;unicast;client_port=26972-26973

And the second time is
>> SETUP rtsp://192.168.1.100:554/MOV_000012353521.mpg RTSP/1.0
>> Transport: RTP/AVP/UDP;unicast;client_port=26974-26975

After reviewing the rtsp.c, I found the bug is caused by the
acceptable string in sdp_parse_line() is "udp" only.

The transport with "udp" or "UDP" should be acceptable in sdp_parse_line().

The full rtsp negotiation is shown below.

OPTIONS rtsp://192.168.1.100:554/MOV_000012353521.mpg RTSP/1.0
CSeq: 1
User-Agent: Lavf58.35.101

RTSP/1.0 200 OK
Server: Orbit2x
CSeq: 1
Public: OPTIONS, DESCRIBE, SETUP, TEARDOWN, PLAY, PAUSE, GET_PARAMETER


DESCRIBE rtsp://192.168.1.100:554/MOV_000012353521.mpg RTSP/1.0
Accept: application/sdp
CSeq: 2
User-Agent: Lavf58.35.101


RTSP/1.0 200 OK
Server: Orbit2x
CSeq: 2
Public: OPTIONS, DESCRIBE, SETUP, TEARDOWN, PLAY, PAUSE, GET_PARAMETER
Content-Type: application/sdp
Content-Length: 179

v=0
o=- 60596 0 IN IP4 192.168.1.100
s=RTSP Session
t=0 0
c=IN IP4 192.168.1.100
b=AS:7984.000
a=type:vod
a=range:npt=0-5313
m=video 0 UDP 33
m=video 0 RTP/AVP/UDP 33

SETUP rtsp://192.168.1.100:554/MOV_000012353521.mpg RTSP/1.0
Transport: RTP/AVP/UDP;unicast;client_port=26972-26973
CSeq: 3
User-Agent: Lavf58.35.101

RTSP/1.0 200 OK
Server: Orbit2x
CSeq: 3
Session: 728680355;timeout=60
Transport: RTP/AVP/UDP;unicast;destination=192.168.2.184;client_port=26972;source=192.168.1.100;server_port=10000-10001;ssrc=7b9507dd

SETUP rtsp://192.168.1.100:554/MOV_000012353521.mpg RTSP/1.0
Transport: RTP/AVP/UDP;unicast;client_port=26974-26975
CSeq: 4
User-Agent: Lavf58.35.101
Session: 728680355

RTSP/1.0 200 OK
Server: Orbit2x
CSeq: 4
Session: 728680355;timeout=60
Transport: RTP/AVP/UDP;unicast;destination=192.168.2.184;client_port=26974;source=192.168.1.100;server_port=10000-10001;ssrc=7b9507dd

PLAY rtsp://192.168.1.100:554/MOV_000012353521.mpg RTSP/1.0
Range: npt=0.000-
CSeq: 5
User-Agent: Lavf58.35.101
Session: 728680355

RTSP/1.0 200 OK
Server: Orbit2x
CSeq: 5
Session: 728680355
Scale: 1.00
Range: npt=0-
Subject: [PATCH] libavformat/rtsp.c: fix RTSP not setup the UDP stream in SDP

---
 libavformat/rtsp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Zhao Zhili July 30, 2021, 6:57 a.m. UTC | #1
> On Jul 30, 2021, at 10:55 AM, Chiu, Yung-Hsiang <mirror.tw@gmail.com> wrote:
> 
> Hi,
> 
> Our RTSP video server reply two streams in SDP are UDP and RTP/AVP/UDP.
>>> m=video 0 UDP 33
>>> m=video 0 RTP/AVP/UDP 33
> 
> ffmpeg now setup twice with the same transport "RTP/AVP/UDP"
> 
> The first time is
>>> SETUP rtsp://192.168.1.100:554/MOV_000012353521.mpg RTSP/1.0
>>> Transport: RTP/AVP/UDP;unicast;client_port=26972-26973
> 
> And the second time is
>>> SETUP rtsp://192.168.1.100:554/MOV_000012353521.mpg RTSP/1.0
>>> Transport: RTP/AVP/UDP;unicast;client_port=26974-26975
> 
> After reviewing the rtsp.c, I found the bug is caused by the
> acceptable string in sdp_parse_line() is "udp" only.

From RFC 4566, the protocol should be specified as “udp”:

https://datatracker.ietf.org/doc/html/rfc4566#section-8.2.2

From IANA, “UDP” is not a registered protocol field of SDP: 
https://www.iana.org/assignments/sdp-parameters/sdp-parameters.xhtml

Is there any reference that the field can be uppercase? I want figure out
whether it’s a bug of FFmpeg, or a bug of the server. I don’t object the
patch, but the patch commit message should reflect the difference between
‘fix bug of FFmpeg’ vs ‘workaround buggy server'.

> 
> The transport with "udp" or "UDP" should be acceptable in sdp_parse_line().
> 
> The full rtsp negotiation is shown below.
> 
> OPTIONS rtsp://192.168.1.100:554/MOV_000012353521.mpg RTSP/1.0
> CSeq: 1
> User-Agent: Lavf58.35.101
> 
> RTSP/1.0 200 OK
> Server: Orbit2x
> CSeq: 1
> Public: OPTIONS, DESCRIBE, SETUP, TEARDOWN, PLAY, PAUSE, GET_PARAMETER
> 
> 
> DESCRIBE rtsp://192.168.1.100:554/MOV_000012353521.mpg RTSP/1.0
> Accept: application/sdp
> CSeq: 2
> User-Agent: Lavf58.35.101
> 
> 
> RTSP/1.0 200 OK
> Server: Orbit2x
> CSeq: 2
> Public: OPTIONS, DESCRIBE, SETUP, TEARDOWN, PLAY, PAUSE, GET_PARAMETER
> Content-Type: application/sdp
> Content-Length: 179
> 
> v=0
> o=- 60596 0 IN IP4 192.168.1.100
> s=RTSP Session
> t=0 0
> c=IN IP4 192.168.1.100
> b=AS:7984.000
> a=type:vod
> a=range:npt=0-5313
> m=video 0 UDP 33
> m=video 0 RTP/AVP/UDP 33
> 
> SETUP rtsp://192.168.1.100:554/MOV_000012353521.mpg RTSP/1.0
> Transport: RTP/AVP/UDP;unicast;client_port=26972-26973
> CSeq: 3
> User-Agent: Lavf58.35.101
> 
> RTSP/1.0 200 OK
> Server: Orbit2x
> CSeq: 3
> Session: 728680355;timeout=60
> Transport: RTP/AVP/UDP;unicast;destination=192.168.2.184;client_port=26972;source=192.168.1.100;server_port=10000-10001;ssrc=7b9507dd
> 
> SETUP rtsp://192.168.1.100:554/MOV_000012353521.mpg RTSP/1.0
> Transport: RTP/AVP/UDP;unicast;client_port=26974-26975
> CSeq: 4
> User-Agent: Lavf58.35.101
> Session: 728680355
> 
> RTSP/1.0 200 OK
> Server: Orbit2x
> CSeq: 4
> Session: 728680355;timeout=60
> Transport: RTP/AVP/UDP;unicast;destination=192.168.2.184;client_port=26974;source=192.168.1.100;server_port=10000-10001;ssrc=7b9507dd
> 
> PLAY rtsp://192.168.1.100:554/MOV_000012353521.mpg RTSP/1.0
> Range: npt=0.000-
> CSeq: 5
> User-Agent: Lavf58.35.101
> Session: 728680355
> 
> RTSP/1.0 200 OK
> Server: Orbit2x
> CSeq: 5
> Session: 728680355
> Scale: 1.00
> Range: npt=0-
> 
> <0001-libavformat-rtsp.c-fix-RTSP-not-setup-the-UDP-stream.patch>_______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
Chiu, Yung-Hsiang July 30, 2021, 8:36 a.m. UTC | #2
Thanks for your review.

I don't have any standards or formal documents mentioned the proto
'udp' in SDP should be uppercase.

The only document I google for the video server 'orbit2x' is shown below
https://developer.samsung.com/smarttv/develop/legacy-platform-library/art00071/index.html#sdp-supported-feature
Apologize for our buggy video server...

Could you give me some suggestions I should write down in the commit message?

"zhilizhao(赵志立)" <quinkblack@foxmail.com> 於 2021年7月30日 週五 下午2:57寫道:

>
>
>
> > On Jul 30, 2021, at 10:55 AM, Chiu, Yung-Hsiang <mirror.tw@gmail.com> wrote:
> >
> > Hi,
> >
> > Our RTSP video server reply two streams in SDP are UDP and RTP/AVP/UDP.
> >>> m=video 0 UDP 33
> >>> m=video 0 RTP/AVP/UDP 33
> >
> > ffmpeg now setup twice with the same transport "RTP/AVP/UDP"
> >
> > The first time is
> >>> SETUP rtsp://192.168.1.100:554/MOV_000012353521.mpg RTSP/1.0
> >>> Transport: RTP/AVP/UDP;unicast;client_port=26972-26973
> >
> > And the second time is
> >>> SETUP rtsp://192.168.1.100:554/MOV_000012353521.mpg RTSP/1.0
> >>> Transport: RTP/AVP/UDP;unicast;client_port=26974-26975
> >
> > After reviewing the rtsp.c, I found the bug is caused by the
> > acceptable string in sdp_parse_line() is "udp" only.
>
> From RFC 4566, the protocol should be specified as “udp”:
>
> https://datatracker.ietf.org/doc/html/rfc4566#section-8.2.2
>
> From IANA, “UDP” is not a registered protocol field of SDP:
> https://www.iana.org/assignments/sdp-parameters/sdp-parameters.xhtml
>
> Is there any reference that the field can be uppercase? I want figure out
> whether it’s a bug of FFmpeg, or a bug of the server. I don’t object the
> patch, but the patch commit message should reflect the difference between
> ‘fix bug of FFmpeg’ vs ‘workaround buggy server'.
>
> >
> > The transport with "udp" or "UDP" should be acceptable in sdp_parse_line().
> >
> > The full rtsp negotiation is shown below.
> >
> > OPTIONS rtsp://192.168.1.100:554/MOV_000012353521.mpg RTSP/1.0
> > CSeq: 1
> > User-Agent: Lavf58.35.101
> >
> > RTSP/1.0 200 OK
> > Server: Orbit2x
> > CSeq: 1
> > Public: OPTIONS, DESCRIBE, SETUP, TEARDOWN, PLAY, PAUSE, GET_PARAMETER
> >
> >
> > DESCRIBE rtsp://192.168.1.100:554/MOV_000012353521.mpg RTSP/1.0
> > Accept: application/sdp
> > CSeq: 2
> > User-Agent: Lavf58.35.101
> >
> >
> > RTSP/1.0 200 OK
> > Server: Orbit2x
> > CSeq: 2
> > Public: OPTIONS, DESCRIBE, SETUP, TEARDOWN, PLAY, PAUSE, GET_PARAMETER
> > Content-Type: application/sdp
> > Content-Length: 179
> >
> > v=0
> > o=- 60596 0 IN IP4 192.168.1.100
> > s=RTSP Session
> > t=0 0
> > c=IN IP4 192.168.1.100
> > b=AS:7984.000
> > a=type:vod
> > a=range:npt=0-5313
> > m=video 0 UDP 33
> > m=video 0 RTP/AVP/UDP 33
> >
> > SETUP rtsp://192.168.1.100:554/MOV_000012353521.mpg RTSP/1.0
> > Transport: RTP/AVP/UDP;unicast;client_port=26972-26973
> > CSeq: 3
> > User-Agent: Lavf58.35.101
> >
> > RTSP/1.0 200 OK
> > Server: Orbit2x
> > CSeq: 3
> > Session: 728680355;timeout=60
> > Transport: RTP/AVP/UDP;unicast;destination=192.168.2.184;client_port=26972;source=192.168.1.100;server_port=10000-10001;ssrc=7b9507dd
> >
> > SETUP rtsp://192.168.1.100:554/MOV_000012353521.mpg RTSP/1.0
> > Transport: RTP/AVP/UDP;unicast;client_port=26974-26975
> > CSeq: 4
> > User-Agent: Lavf58.35.101
> > Session: 728680355
> >
> > RTSP/1.0 200 OK
> > Server: Orbit2x
> > CSeq: 4
> > Session: 728680355;timeout=60
> > Transport: RTP/AVP/UDP;unicast;destination=192.168.2.184;client_port=26974;source=192.168.1.100;server_port=10000-10001;ssrc=7b9507dd
> >
> > PLAY rtsp://192.168.1.100:554/MOV_000012353521.mpg RTSP/1.0
> > Range: npt=0.000-
> > CSeq: 5
> > User-Agent: Lavf58.35.101
> > Session: 728680355
> >
> > RTSP/1.0 200 OK
> > Server: Orbit2x
> > CSeq: 5
> > Session: 728680355
> > Scale: 1.00
> > Range: npt=0-
> >
> > <0001-libavformat-rtsp.c-fix-RTSP-not-setup-the-UDP-stream.patch>_______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> >
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Zhao Zhili July 30, 2021, 8:54 a.m. UTC | #3
> On Jul 30, 2021, at 4:36 PM, Chiu, Yung-Hsiang <mirror.tw@gmail.com> wrote:
> 
> Thanks for your review.
> 
> I don't have any standards or formal documents mentioned the proto
> 'udp' in SDP should be uppercase.
> 
> The only document I google for the video server 'orbit2x' is shown below
> https://developer.samsung.com/smarttv/develop/legacy-platform-library/art00071/index.html#sdp-supported-feature
> Apologize for our buggy video server...
> 
> Could you give me some suggestions I should write down in the commit message?

Something like “Do case-insensitive check on raw data over UDP protocol”. Add description
about why this is needed in commit message and/or code comments. I’m not native
english speaker, sorry I can’t give a proper description.

> 
> "zhilizhao(赵志立)" <quinkblack@foxmail.com> 於 2021年7月30日 週五 下午2:57寫道:
> 
>> 
>> 
>> 
>>> On Jul 30, 2021, at 10:55 AM, Chiu, Yung-Hsiang <mirror.tw@gmail.com> wrote:
>>> 
>>> Hi,
>>> 
>>> Our RTSP video server reply two streams in SDP are UDP and RTP/AVP/UDP.
>>>>> m=video 0 UDP 33
>>>>> m=video 0 RTP/AVP/UDP 33
>>> 
>>> ffmpeg now setup twice with the same transport "RTP/AVP/UDP"
>>> 
>>> The first time is
>>>>> SETUP rtsp://192.168.1.100:554/MOV_000012353521.mpg RTSP/1.0
>>>>> Transport: RTP/AVP/UDP;unicast;client_port=26972-26973
>>> 
>>> And the second time is
>>>>> SETUP rtsp://192.168.1.100:554/MOV_000012353521.mpg RTSP/1.0
>>>>> Transport: RTP/AVP/UDP;unicast;client_port=26974-26975
>>> 
>>> After reviewing the rtsp.c, I found the bug is caused by the
>>> acceptable string in sdp_parse_line() is "udp" only.
>> 
>> From RFC 4566, the protocol should be specified as “udp”:
>> 
>> https://datatracker.ietf.org/doc/html/rfc4566#section-8.2.2
>> 
>> From IANA, “UDP” is not a registered protocol field of SDP:
>> https://www.iana.org/assignments/sdp-parameters/sdp-parameters.xhtml
>> 
>> Is there any reference that the field can be uppercase? I want figure out
>> whether it’s a bug of FFmpeg, or a bug of the server. I don’t object the
>> patch, but the patch commit message should reflect the difference between
>> ‘fix bug of FFmpeg’ vs ‘workaround buggy server'.
>> 
>>> 
>>> The transport with "udp" or "UDP" should be acceptable in sdp_parse_line().
>>> 
>>> The full rtsp negotiation is shown below.
>>> 
>>> OPTIONS rtsp://192.168.1.100:554/MOV_000012353521.mpg RTSP/1.0
>>> CSeq: 1
>>> User-Agent: Lavf58.35.101
>>> 
>>> RTSP/1.0 200 OK
>>> Server: Orbit2x
>>> CSeq: 1
>>> Public: OPTIONS, DESCRIBE, SETUP, TEARDOWN, PLAY, PAUSE, GET_PARAMETER
>>> 
>>> 
>>> DESCRIBE rtsp://192.168.1.100:554/MOV_000012353521.mpg RTSP/1.0
>>> Accept: application/sdp
>>> CSeq: 2
>>> User-Agent: Lavf58.35.101
>>> 
>>> 
>>> RTSP/1.0 200 OK
>>> Server: Orbit2x
>>> CSeq: 2
>>> Public: OPTIONS, DESCRIBE, SETUP, TEARDOWN, PLAY, PAUSE, GET_PARAMETER
>>> Content-Type: application/sdp
>>> Content-Length: 179
>>> 
>>> v=0
>>> o=- 60596 0 IN IP4 192.168.1.100
>>> s=RTSP Session
>>> t=0 0
>>> c=IN IP4 192.168.1.100
>>> b=AS:7984.000
>>> a=type:vod
>>> a=range:npt=0-5313
>>> m=video 0 UDP 33
>>> m=video 0 RTP/AVP/UDP 33
>>> 
>>> SETUP rtsp://192.168.1.100:554/MOV_000012353521.mpg RTSP/1.0
>>> Transport: RTP/AVP/UDP;unicast;client_port=26972-26973
>>> CSeq: 3
>>> User-Agent: Lavf58.35.101
>>> 
>>> RTSP/1.0 200 OK
>>> Server: Orbit2x
>>> CSeq: 3
>>> Session: 728680355;timeout=60
>>> Transport: RTP/AVP/UDP;unicast;destination=192.168.2.184;client_port=26972;source=192.168.1.100;server_port=10000-10001;ssrc=7b9507dd
>>> 
>>> SETUP rtsp://192.168.1.100:554/MOV_000012353521.mpg RTSP/1.0
>>> Transport: RTP/AVP/UDP;unicast;client_port=26974-26975
>>> CSeq: 4
>>> User-Agent: Lavf58.35.101
>>> Session: 728680355
>>> 
>>> RTSP/1.0 200 OK
>>> Server: Orbit2x
>>> CSeq: 4
>>> Session: 728680355;timeout=60
>>> Transport: RTP/AVP/UDP;unicast;destination=192.168.2.184;client_port=26974;source=192.168.1.100;server_port=10000-10001;ssrc=7b9507dd
>>> 
>>> PLAY rtsp://192.168.1.100:554/MOV_000012353521.mpg RTSP/1.0
>>> Range: npt=0.000-
>>> CSeq: 5
>>> User-Agent: Lavf58.35.101
>>> Session: 728680355
>>> 
>>> RTSP/1.0 200 OK
>>> Server: Orbit2x
>>> CSeq: 5
>>> Session: 728680355
>>> Scale: 1.00
>>> Range: npt=0-
>>> 
>>> <0001-libavformat-rtsp.c-fix-RTSP-not-setup-the-UDP-stream.patch>_______________________________________________
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel@ffmpeg.org
>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>> 
>>> To unsubscribe, visit link above, or email
>>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>>> 
>> 
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> 
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff mbox series

Patch

diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
index 9869e1b72e..dfa25ec6a4 100644
--- a/libavformat/rtsp.c
+++ b/libavformat/rtsp.c
@@ -509,7 +509,7 @@  static void sdp_parse_line(AVFormatContext *s, SDPParseState *s1,
         rtsp_st->sdp_port = atoi(buf1);
 
         get_word(buf1, sizeof(buf1), &p); /* protocol */
-        if (!strcmp(buf1, "udp"))
+        if (!av_strcasecmp(buf1, "udp"))
             rt->transport = RTSP_TRANSPORT_RAW;
         else if (strstr(buf1, "/AVPF") || strstr(buf1, "/SAVPF"))
             rtsp_st->feedback = 1;