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

[Zhao Zhili]

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;

Otherwise, I'm OK with the patch.

>
>> I tried a similar approach in
>>
>> http://ffmpeg.org/pipermail/ffmpeg-devel/2020-August/267437.html.
>
> I'm less keen on that approach. As the problem is with listen mode, 
> I'd cautiously avoid changing code that is general to both modes.
>
>> The intended behavior (e.g., return value) of parse_rtsp_message is 
>> not quite clear
>>
>> to me, could you help improve it, like adding some documentation?
>
> Well I'm not sure how much there is to document. It's a static 
> function that is called from one single place in the code, so the 
> documentation of it is the code surrounding the single call to it. 
> Ideally it'd follow common conventions like returning a negative value 
> on error and zero/positive on success that one should continue from.
>
> // Martin
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".