[FFmpeg-devel] [PATCH] rtsp: Fix infinite loop in listen mode with UDP transport
Martin Storsjö
martin at martin.st
Thu Oct 1 23:48:13 EEST 2020
On Thu, 1 Oct 2020, Andriy Gelman wrote:
> 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.
Thanks for testing this.
Indeed, I'd rather treat that as a separate case. The listen mode is not
very widely used, and this issue can be traced back to a regression, so
that can be easily fixed in that context.
For the other case you're pointing out, I don't have a concrete bug (the
UDP mode seems to require waiting for a long timeout at the end though,
but changing this return statement to return an error doesn't seem to
help), and the normal non-listen mode code can be used in a huge variety
of cases, many that aren't very easy to test (e.g. the code used to
support RealRTSP, but I'm not sure if there's any publicly available test
clips/servers for that any longer - multimediawiki used to list some, but
I tested them last time close to a decade ago, and then there might have
been zero or one of them still responding). So for that code, I'd tread
much more carefully...
// Martin
More information about the ffmpeg-devel
mailing list