[FFmpeg-devel] ?==?utf-8?q? ?==?utf-8?q? fix:?= rtsp/transport parser should accept comma for parameter

[abalam]

Thank you for your answer Marton.
Yes I broke the parse process because I saw below in rtsp.c a specific source code which specify to have only "One transport" :
 ff_rtsp_send_cmd(s, "SETUP", rtsp_st->control_url, cmd, reply, NULL);
if (reply->status_code == 461 /* Unsupported protocol */ && i == 0) {
err = 1;
goto fail;
} else if (reply->status_code != RTSP_STATUS_OK ||
reply->nb_transports != 1) {
err = ff_rtsp_averror(reply->status_code, AVERROR_INVALIDDATA);
goto fail;
}

Maybe I should have take time to find out or ask here why there is a such condition "reply->nb_transports != 1"...

I'll check the VLC source code as soon as I find enough time... it's not easy with children around those COVID times.... :-/

Yann

On Friday, May 01, 2020 21:54 CEST, Marton Balint <cus at passwd.hu> wrote:
 

On Fri, 1 May 2020, abalam wrote:

>
> I needed this fix to make [Shinobi](https://github.com/ShinobiCCTV/Shinobi) works with recent chinese cameras.
>
> We can read that FFmpeg is perfectly following the [RTSP specs](https://tools.ietf.org/html/rfc2326#page-58) : 
> `Transports are comma separated, listed in order of preference. Parameters may be added to each transport, separated by a semicolon.`
> But considering:
> - VLC is working smart with not perfect cameras ;
> - RTSP module in FFmpeg is not programmed to handle more than one transport ; 
> => this fix could be useful to anyone
>
> I also think this fix might be improved by a better scan of parameters if ffmpeg wants to handle more than one transport.
>
> Details: 
> I'm using [Shinobi](https://github.com/ShinobiCCTV/Shinobi) to record cameras rtsp streams
> For several cameras, cheap chinese ones, VLC is working to get stream but FFmpeg always crashes after it receives the RTSP transports parameters available.
>
> `Transport: RTP/AVP;unicast;mode=PLAY;source=192.168.66.164;client_port=11658-11659;server_port=40000-40001,ssrc=FFFFCCCC`
>
> After debugging the source code, it appears that FFmpeg is not considering that a comma `,` before `ssrc` is a correct separator and think it's a second transport.
> Then it crashes because RTSP don't want to accept more than one transport.
>
> ( This was closed PR https://github.com/FFmpeg/FFmpeg/pull/336 )
>
> Thank you for having read,
> Keep good work and have a nice day!

I am not really familiar with RTSP code, but at least the parse function
does seem to parse multiple transports. So effectively breaking it does
not seem right. Maybe you should check out how VLC detects such broken
devices, and implement something similar here, because probably VLC's
workaround is proven to not cause valid transport specifications to be
misinterpreted as a single transport.

Regards,
Marton

>
> ---
>  libavformat/rtsp.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
> index b2b3f32011..f77de10119 100644
> --- a/libavformat/rtsp.c
> +++ b/libavformat/rtsp.c
> @@ -1001,11 +1001,9 @@ static void rtsp_parse_transport(AVFormatContext *s,
>
>              while (*p != ';' && *p != '\0' && *p != ',')
>                  p++;
> -            if (*p == ';')
> +            if (*p == ';' || *p == ',')
>                  p++;
>          }
> -        if (*p == ',')
> -            p++;
>
>          reply->nb_transports++;
>          if (reply->nb_transports >= RTSP_MAX_TRANSPORTS)
> --
> 2.21.1 (Apple Git-122.3)
> _______________________________________________
> 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".
_______________________________________________
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".