[FFmpeg-devel] [PATCH] rtsp: Fix infinite loop in listen mode with UDP transport

[Andriy Gelman]

On Thu, 01. Oct 22:00, Zhao Zhili wrote:
> 
> On 10/1/20 4:15 AM, Martin Storsjö wrote:
> > On Wed, 30 Sep 2020, Zhao Zhili wrote:
> > 
> > > Hi Martin,
> > > 
> > > On 9/30/20 5:41 PM, Martin Storsjö wrote:
> > > > In listen mode with UDP transport, once the sender has sent
> > > > the TEARDOWN and closed the connection, poll will indicate that
> > > > one can read from the connection (indicating that the socket has
> > > > reached EOF and should be closed by the receiver as well). In this
> > > > case, parse_rtsp_message won't try to parse the command (because
> > > > it's no longer in state STREAMING), but previously just returned
> > > > zero.
> > > > 
> > > > Prior to f6161fccf8c5720ceac1ed1df8ba60ff8fed69f5, this caused
> > > > udp_read_packet to return zero, which is treated as EOF by
> > > > read_packet. But after that commit, udp_read_packet would continue
> > > > if parse_rtsp_message didn't return an explicit error code.
> > > > 
> > > > To keep the original behaviour from before that commit, more
> > > > explicitly return an error in parse_rtsp_message when in the wrong
> > > > state.
> > > > 
> > > > Fixes: #8840
> > > > ---
> > > >   libavformat/rtsp.c | 2 +-
> > > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
> > > > index 5d8491b74b..ad12f2ae98 100644
> > > > --- a/libavformat/rtsp.c
> > > > +++ b/libavformat/rtsp.c
> > > > @@ -1970,7 +1970,7 @@ static int parse_rtsp_message(AVFormatContext *s)
> > > >                   av_log(s, AV_LOG_WARNING,
> > > >                          "Unable to answer to TEARDOWN\n");
> > > >           } else
> > > > -            return 0;
> > > > +            return AVERROR_EOF;
> > > 
> > > Does the else part needs the same fix?
> > 
> > Which else part do you refer to? The else above (with the warning)? Yes
> > that bit looks a bit odd to me as well - your patch 2/2 looks like a
> > good fix for that.

> 
> I mean the else below, especially
> 
>         /* XXX: parse message */
>         if (rt->state != RTSP_STATE_STREAMING)
>             return 0;

I did some tests with the rtsp server from:
https://github.com/revmischa/rtsp-server

This point can be reached with rt->state = RTSP_STATE_IDLE when the
initial_pause option is set:
./ffmpeg -initial_pause 1 -i rtsp://127.0.0.1/abc -f null -

Then it seems changing the return value in the above code would lead to
unintended results.

-- 
Andriy