diff mbox series

[FFmpeg-devel] avformat/rtsp: fix infinite loop with udp transport

Message ID tencent_D4D5C489108EB365497301A85113E0D36505@qq.com
State Superseded
Headers show
Series [FFmpeg-devel] avformat/rtsp: fix infinite loop with udp transport
Related show

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Zhao Zhili Aug. 6, 2020, 8:50 a.m. UTC
From: Zhao Zhili <quinkblack@foxmail.com>

User report: http://ffmpeg.org/pipermail/ffmpeg-user/2020-August/049494.html

server:
./ffmpeg -i test.mp4 -c copy -f rtsp -rtsp_transport udp  rtsp://localhost:12345/live.sdp

client:
./ffmpeg_g -y -rtsp_flags listen -timeout 100 -i rtsp://localhost:12345/live.sdp -c copy test.mp4
---
 libavformat/rtsp.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Zhao Zhili Aug. 17, 2020, 12:05 a.m. UTC | #1
Please help review the patch, thanks!

> On Aug 6, 2020, at 4:50 PM, quinkblack@foxmail.com wrote:
> 
> From: Zhao Zhili <quinkblack@foxmail.com>
> 
> User report: http://ffmpeg.org/pipermail/ffmpeg-user/2020-August/049494.html
> 
> server:
> ./ffmpeg -i test.mp4 -c copy -f rtsp -rtsp_transport udp  rtsp://localhost:12345/live.sdp
> 
> client:
> ./ffmpeg_g -y -rtsp_flags listen -timeout 100 -i rtsp://localhost:12345/live.sdp -c copy test.mp4
> ---
> libavformat/rtsp.c | 5 +++++
> 1 file changed, 5 insertions(+)
> 
> diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
> index 5d8491b74b..0fb9fde6b4 100644
> --- a/libavformat/rtsp.c
> +++ b/libavformat/rtsp.c
> @@ -2051,6 +2051,11 @@ static int udp_read_packet(AVFormatContext *s, RTSPStream **prtsp_st,
>                 if ((ret = parse_rtsp_message(s)) < 0) {
>                     return ret;
>                 }
> +                /* Since there is no way to detect eof of udp streams, break
> +                 * the loop in teardown state to prevent run into infinite.
> +                 */
> +                if (rt->state == RTSP_STATE_IDLE)
> +                    return AVERROR_EOF;
>             }
> #endif
>         } else if (n == 0 && ++timeout_cnt >= MAX_TIMEOUTS) {
> -- 
> 2.17.1
> 
> _______________________________________________
> 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 Aug. 24, 2020, 5:17 p.m. UTC | #2
Ping again.

> On Aug 17, 2020, at 8:05 AM, zhilizhao <quinkblack@foxmail.com> wrote:
> 
> Please help review the patch, thanks!
> 
>> On Aug 6, 2020, at 4:50 PM, quinkblack@foxmail.com wrote:
>> 
>> From: Zhao Zhili <quinkblack@foxmail.com>
>> 
>> User report: http://ffmpeg.org/pipermail/ffmpeg-user/2020-August/049494.html
>> 
>> server:
>> ./ffmpeg -i test.mp4 -c copy -f rtsp -rtsp_transport udp  rtsp://localhost:12345/live.sdp
>> 
>> client:
>> ./ffmpeg_g -y -rtsp_flags listen -timeout 100 -i rtsp://localhost:12345/live.sdp -c copy test.mp4
>> ---
>> libavformat/rtsp.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>> 
>> diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
>> index 5d8491b74b..0fb9fde6b4 100644
>> --- a/libavformat/rtsp.c
>> +++ b/libavformat/rtsp.c
>> @@ -2051,6 +2051,11 @@ static int udp_read_packet(AVFormatContext *s, RTSPStream **prtsp_st,
>>                if ((ret = parse_rtsp_message(s)) < 0) {
>>                    return ret;
>>                }
>> +                /* Since there is no way to detect eof of udp streams, break
>> +                 * the loop in teardown state to prevent run into infinite.
>> +                 */
>> +                if (rt->state == RTSP_STATE_IDLE)
>> +                    return AVERROR_EOF;
>>            }
>> #endif
>>        } else if (n == 0 && ++timeout_cnt >= MAX_TIMEOUTS) {
>> -- 
>> 2.17.1
>> 
>> _______________________________________________
>> 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".
>
Andriy Gelman Aug. 29, 2020, 5:45 p.m. UTC | #3
Hi Zhao,

On Tue, 25. Aug 01:17, Zhao Zhili wrote:
> Ping again.
> 
> > On Aug 17, 2020, at 8:05 AM, zhilizhao <quinkblack@foxmail.com> wrote:
> > 
> > Please help review the patch, thanks!
> > 
> >> On Aug 6, 2020, at 4:50 PM, quinkblack@foxmail.com wrote:
> >> 
> >> From: Zhao Zhili <quinkblack@foxmail.com>
> >> 
> >> User report: http://ffmpeg.org/pipermail/ffmpeg-user/2020-August/049494.html
> >> 
> >> server:
> >> ./ffmpeg -i test.mp4 -c copy -f rtsp -rtsp_transport udp  rtsp://localhost:12345/live.sdp
> >> 
> >> client:
> >> ./ffmpeg_g -y -rtsp_flags listen -timeout 100 -i rtsp://localhost:12345/live.sdp -c copy test.mp4
> >> ---
> >> libavformat/rtsp.c | 5 +++++
> >> 1 file changed, 5 insertions(+)
> >> 
> >> diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
> >> index 5d8491b74b..0fb9fde6b4 100644
> >> --- a/libavformat/rtsp.c
> >> +++ b/libavformat/rtsp.c
> >> @@ -2051,6 +2051,11 @@ static int udp_read_packet(AVFormatContext *s, RTSPStream **prtsp_st,
> >>                if ((ret = parse_rtsp_message(s)) < 0) {
> >>                    return ret;
> >>                }
> >> +                /* Since there is no way to detect eof of udp streams, break
> >> +                 * the loop in teardown state to prevent run into infinite.
> >> +                 */
> >> +                if (rt->state == RTSP_STATE_IDLE)
> >> +                    return AVERROR_EOF;
> >>            }
> >> #endif
> >>        } else if (n == 0 && ++timeout_cnt >= MAX_TIMEOUTS) {

Currently when the stream finishes a BYE message is not sent from the server.  

You can ensure that the message is sent by adding the send_bye option with the
flag:
"-rtpflags send_bye"

This will also fix the problem.

What I think is more confusing is that "-rtp_flags send_bye" is also valid on
the command line but will have no effect..
Zhao Zhili Aug. 30, 2020, 3:50 a.m. UTC | #4
> On Aug 30, 2020, at 1:45 AM, Andriy Gelman <andriy.gelman@gmail.com> wrote:
> 
> Hi Zhao,
> 
> On Tue, 25. Aug 01:17, Zhao Zhili wrote:
>> Ping again.
>> 
>>> On Aug 17, 2020, at 8:05 AM, zhilizhao <quinkblack@foxmail.com> wrote:
>>> 
>>> Please help review the patch, thanks!
>>> 
>>>> On Aug 6, 2020, at 4:50 PM, quinkblack@foxmail.com wrote:
>>>> 
>>>> From: Zhao Zhili <quinkblack@foxmail.com>
>>>> 
>>>> User report: http://ffmpeg.org/pipermail/ffmpeg-user/2020-August/049494.html
>>>> 
>>>> server:
>>>> ./ffmpeg -i test.mp4 -c copy -f rtsp -rtsp_transport udp  rtsp://localhost:12345/live.sdp
>>>> 
>>>> client:
>>>> ./ffmpeg_g -y -rtsp_flags listen -timeout 100 -i rtsp://localhost:12345/live.sdp -c copy test.mp4
>>>> ---
>>>> libavformat/rtsp.c | 5 +++++
>>>> 1 file changed, 5 insertions(+)
>>>> 
>>>> diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
>>>> index 5d8491b74b..0fb9fde6b4 100644
>>>> --- a/libavformat/rtsp.c
>>>> +++ b/libavformat/rtsp.c
>>>> @@ -2051,6 +2051,11 @@ static int udp_read_packet(AVFormatContext *s, RTSPStream **prtsp_st,
>>>>               if ((ret = parse_rtsp_message(s)) < 0) {
>>>>                   return ret;
>>>>               }
>>>> +                /* Since there is no way to detect eof of udp streams, break
>>>> +                 * the loop in teardown state to prevent run into infinite.
>>>> +                 */
>>>> +                if (rt->state == RTSP_STATE_IDLE)
>>>> +                    return AVERROR_EOF;
>>>>           }
>>>> #endif
>>>>       } else if (n == 0 && ++timeout_cnt >= MAX_TIMEOUTS) {
> 
> Currently when the stream finishes a BYE message is not sent from the server.  
> 
> You can ensure that the message is sent by adding the send_bye option with the
> flag:
> "-rtpflags send_bye"
> 
> This will also fix the problem.

Thanks for your reply. It's a clean solution from the viewpoint of FFmpeg users.
However, I think FFmpeg should handle the case without BYE packets, even
without RTCP. The busy loop leads to high cpu usage. I'm not saying the patch
is the right solution, I'd like to hear other suggestions to fix the problem on the
rtsp receiver's side.

> 
> What I think is more confusing is that "-rtp_flags send_bye" is also valid on
> the command line but will have no effect..
> 
> -- 
> Andriy
> _______________________________________________
> 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".
Andriy Gelman Sept. 7, 2020, 6:02 p.m. UTC | #5
On Sun, 30. Aug 11:50, Zhao Zhili wrote:
> 
> 
> > On Aug 30, 2020, at 1:45 AM, Andriy Gelman <andriy.gelman@gmail.com> wrote:
> > 
> > Hi Zhao,
> > 
> > On Tue, 25. Aug 01:17, Zhao Zhili wrote:
> >> Ping again.
> >> 
> >>> On Aug 17, 2020, at 8:05 AM, zhilizhao <quinkblack@foxmail.com> wrote:
> >>> 
> >>> Please help review the patch, thanks!
> >>> 
> >>>> On Aug 6, 2020, at 4:50 PM, quinkblack@foxmail.com wrote:
> >>>> 
> >>>> From: Zhao Zhili <quinkblack@foxmail.com>
> >>>> 
> >>>> User report: http://ffmpeg.org/pipermail/ffmpeg-user/2020-August/049494.html
> >>>> 
> >>>> server:
> >>>> ./ffmpeg -i test.mp4 -c copy -f rtsp -rtsp_transport udp  rtsp://localhost:12345/live.sdp
> >>>> 
> >>>> client:
> >>>> ./ffmpeg_g -y -rtsp_flags listen -timeout 100 -i rtsp://localhost:12345/live.sdp -c copy test.mp4
> >>>> ---
> >>>> libavformat/rtsp.c | 5 +++++
> >>>> 1 file changed, 5 insertions(+)
> >>>> 
> >>>> diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
> >>>> index 5d8491b74b..0fb9fde6b4 100644
> >>>> --- a/libavformat/rtsp.c
> >>>> +++ b/libavformat/rtsp.c
> >>>> @@ -2051,6 +2051,11 @@ static int udp_read_packet(AVFormatContext *s, RTSPStream **prtsp_st,
> >>>>               if ((ret = parse_rtsp_message(s)) < 0) {
> >>>>                   return ret;
> >>>>               }
> >>>> +                /* Since there is no way to detect eof of udp streams, break
> >>>> +                 * the loop in teardown state to prevent run into infinite.
> >>>> +                 */
> >>>> +                if (rt->state == RTSP_STATE_IDLE)
> >>>> +                    return AVERROR_EOF;
> >>>>           }
> >>>> #endif
> >>>>       } else if (n == 0 && ++timeout_cnt >= MAX_TIMEOUTS) {
> > 
> > Currently when the stream finishes a BYE message is not sent from the server.  
> > 
> > You can ensure that the message is sent by adding the send_bye option with the
> > flag:
> > "-rtpflags send_bye"
> > 
> > This will also fix the problem.
> 
> Thanks for your reply. It's a clean solution from the viewpoint of FFmpeg users.
> However, I think FFmpeg should handle the case without BYE packets, even
> without RTCP. The busy loop leads to high cpu usage. I'm not saying the patch
> is the right solution, I'd like to hear other suggestions to fix the problem on the
> rtsp receiver's side.
> 

The server does send a teardown message when it exits. Currently this sets 
the client state to rt->state = RTSP_STATE_IDLE.

Could adding RTSP_STATE_TEARDOWN to RTSPClientState enum be better? You could
then return AVERROR_EOF if the client is in the teardown state.
diff mbox series

Patch

diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
index 5d8491b74b..0fb9fde6b4 100644
--- a/libavformat/rtsp.c
+++ b/libavformat/rtsp.c
@@ -2051,6 +2051,11 @@  static int udp_read_packet(AVFormatContext *s, RTSPStream **prtsp_st,
                 if ((ret = parse_rtsp_message(s)) < 0) {
                     return ret;
                 }
+                /* Since there is no way to detect eof of udp streams, break
+                 * the loop in teardown state to prevent run into infinite.
+                 */
+                if (rt->state == RTSP_STATE_IDLE)
+                    return AVERROR_EOF;
             }
 #endif
         } else if (n == 0 && ++timeout_cnt >= MAX_TIMEOUTS) {