Message ID | 20200406165651.58783-1-phunkyfish@gmail.com |
---|---|
State | Superseded |
Headers | show |
Series | [FFmpeg-devel] avformat/rtp: Pass sources and block filter addresses via sdp file for rtp | expand |
Context | Check | Description |
---|---|---|
andriy/ffmpeg-patchwork | success | Make fate finished |
> sdp_length is used uninitialized here it is used uninitialized in the > version that was merged as b71685865fe761925feedda3cd0b288224d9a509. The > newer versions [2], [3] don't exhibit this flaw. > > [3] and [1] also have a flaw in common that [2] and this one are > lacking: The semicolon of the definition of const char *p is missing. > > Finally, neither of these versions here seems to have been based upon > git master which contains a call to av_log() directly after the above > snprintf. > > - Andreas Andreas, Apologies I did not get back to you sooner I was indisposed. I have updated the patch now which should be correct, addressing your comments. On Mon, 6 Apr 2020 at 17:56, phunkyfish <phunkyfish@gmail.com> wrote: > --- > libavformat/rtsp.c | 47 ++++++++++++++++++++++++++++++++++++++-------- > 1 file changed, 39 insertions(+), 8 deletions(-) > > diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c > index cd6fc32a29..0d0bc2be0d 100644 > --- a/libavformat/rtsp.c > +++ b/libavformat/rtsp.c > @@ -2447,8 +2447,8 @@ static int rtp_probe(const AVProbeData *p) > static int rtp_read_header(AVFormatContext *s) > { > uint8_t recvbuf[RTP_MAX_PACKET_LENGTH]; > - char host[500], sdp[500]; > - int ret, port; > + char host[500], sdp[1000], filters_buf[1000]; > + int ret, port, sdp_length, nc; > URLContext* in = NULL; > int payload_type; > AVCodecParameters *par = NULL; > @@ -2456,6 +2456,7 @@ static int rtp_read_header(AVFormatContext *s) > AVIOContext pb; > socklen_t addrlen = sizeof(addr); > RTSPState *rt = s->priv_data; > + const char *p; > > if (!ff_network_init()) > return AVERROR(EIO); > @@ -2513,13 +2514,40 @@ static int rtp_read_header(AVFormatContext *s) > av_url_split(NULL, 0, NULL, 0, host, sizeof(host), &port, > NULL, 0, s->url); > > - snprintf(sdp, sizeof(sdp), > - "v=0\r\nc=IN IP%d %s\r\nm=%s %d RTP/AVP %d\r\n", > - addr.ss_family == AF_INET ? 4 : 6, host, > - par->codec_type == AVMEDIA_TYPE_DATA ? "application" : > - par->codec_type == AVMEDIA_TYPE_VIDEO ? "video" : "audio", > - port, payload_type); > + sdp_length = snprintf(sdp, sizeof(sdp), > + "v=0\r\nc=IN IP%d %s\r\n", > + addr.ss_family == AF_INET ? 4 : 6, host); > + > + p = strchr(s->url, '?'); > + if (p) { > + static const char *filters[][2] = {{"sources", "incl"}, {"block", > "excl"}, {NULL, NULL}}; > + int i; > + char *q; > + for (i = 0; filters[i][0]; i++) { > + if (av_find_info_tag(filters_buf, sizeof(filters_buf), > filters[i][0], p)) { > + q = filters_buf; > + while ((q = strchr(q, ',')) != NULL) > + *q = ' '; > + nc = snprintf(sdp + sdp_length, sizeof(sdp) - sdp_length, > + "a=source-filter:%s IN IP%d %s %s\r\n", > + filters[i][1], > + addr.ss_family == AF_INET ? 4 : 6, host, > + filters_buf); > + if (nc < 0 || nc + sdp_length >= sizeof(sdp)) > + goto fail_nobuf; > + sdp_length += nc; > + } > + } > + } > + > + nc = snprintf(sdp + sdp_length, sizeof(sdp) - sdp_length, > + "m=%s %d RTP/AVP %d\r\n", > + par->codec_type == AVMEDIA_TYPE_DATA ? "application" : > + par->codec_type == AVMEDIA_TYPE_VIDEO ? "video" : > "audio", > + port, payload_type); > av_log(s, AV_LOG_VERBOSE, "SDP:\n%s\n", sdp); > + if (nc < 0 || nc + sdp_length >= sizeof(sdp)) > + goto fail_nobuf; > avcodec_parameters_free(&par); > > ffio_init_context(&pb, sdp, strlen(sdp), 0, NULL, NULL, NULL, NULL); > @@ -2534,6 +2562,9 @@ static int rtp_read_header(AVFormatContext *s) > s->pb = NULL; > return ret; > > +fail_nobuf: > + ret = AVERROR(ENOBUFS); > + av_log(s, AV_LOG_ERROR, "rtp_read_header(): not enough buffer space > for sdp-headers\n"); > fail: > avcodec_parameters_free(&par); > if (in) > -- > 2.24.1 (Apple Git-126) > >
On Mon, 6 Apr 2020, phunkyfish wrote: > --- > libavformat/rtsp.c | 47 ++++++++++++++++++++++++++++++++++++++-------- > 1 file changed, 39 insertions(+), 8 deletions(-) > > diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c > index cd6fc32a29..0d0bc2be0d 100644 > --- a/libavformat/rtsp.c > +++ b/libavformat/rtsp.c > @@ -2447,8 +2447,8 @@ static int rtp_probe(const AVProbeData *p) > static int rtp_read_header(AVFormatContext *s) > { > uint8_t recvbuf[RTP_MAX_PACKET_LENGTH]; > - char host[500], sdp[500]; > - int ret, port; > + char host[500], sdp[1000], filters_buf[1000]; > + int ret, port, sdp_length, nc; > URLContext* in = NULL; > int payload_type; > AVCodecParameters *par = NULL; > @@ -2456,6 +2456,7 @@ static int rtp_read_header(AVFormatContext *s) > AVIOContext pb; > socklen_t addrlen = sizeof(addr); > RTSPState *rt = s->priv_data; > + const char *p; > > if (!ff_network_init()) > return AVERROR(EIO); > @@ -2513,13 +2514,40 @@ static int rtp_read_header(AVFormatContext *s) > av_url_split(NULL, 0, NULL, 0, host, sizeof(host), &port, > NULL, 0, s->url); > > - snprintf(sdp, sizeof(sdp), > - "v=0\r\nc=IN IP%d %s\r\nm=%s %d RTP/AVP %d\r\n", > - addr.ss_family == AF_INET ? 4 : 6, host, > - par->codec_type == AVMEDIA_TYPE_DATA ? "application" : > - par->codec_type == AVMEDIA_TYPE_VIDEO ? "video" : "audio", > - port, payload_type); > + sdp_length = snprintf(sdp, sizeof(sdp), > + "v=0\r\nc=IN IP%d %s\r\n", > + addr.ss_family == AF_INET ? 4 : 6, host); Please use an AVBPrint buffer instead of snprintf()-ing to sdp. Thanks, Marton > + > + p = strchr(s->url, '?'); > + if (p) { > + static const char *filters[][2] = {{"sources", "incl"}, {"block", "excl"}, {NULL, NULL}}; > + int i; > + char *q; > + for (i = 0; filters[i][0]; i++) { > + if (av_find_info_tag(filters_buf, sizeof(filters_buf), filters[i][0], p)) { > + q = filters_buf; > + while ((q = strchr(q, ',')) != NULL) > + *q = ' '; > + nc = snprintf(sdp + sdp_length, sizeof(sdp) - sdp_length, > + "a=source-filter:%s IN IP%d %s %s\r\n", > + filters[i][1], > + addr.ss_family == AF_INET ? 4 : 6, host, > + filters_buf); > + if (nc < 0 || nc + sdp_length >= sizeof(sdp)) > + goto fail_nobuf; > + sdp_length += nc; > + } > + } > + } > + > + nc = snprintf(sdp + sdp_length, sizeof(sdp) - sdp_length, > + "m=%s %d RTP/AVP %d\r\n", > + par->codec_type == AVMEDIA_TYPE_DATA ? "application" : > + par->codec_type == AVMEDIA_TYPE_VIDEO ? "video" : "audio", > + port, payload_type); > av_log(s, AV_LOG_VERBOSE, "SDP:\n%s\n", sdp); > + if (nc < 0 || nc + sdp_length >= sizeof(sdp)) > + goto fail_nobuf; > avcodec_parameters_free(&par); > > ffio_init_context(&pb, sdp, strlen(sdp), 0, NULL, NULL, NULL, NULL); > @@ -2534,6 +2562,9 @@ static int rtp_read_header(AVFormatContext *s) > s->pb = NULL; > return ret; > > +fail_nobuf: > + ret = AVERROR(ENOBUFS); > + av_log(s, AV_LOG_ERROR, "rtp_read_header(): not enough buffer space for sdp-headers\n"); > fail: > avcodec_parameters_free(&par); > if (in) > -- > 2.24.1 (Apple Git-126) > > _______________________________________________ > 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".
Ok, latest patch uses AVBPrint. Please let me know if my usage is correct, I have not used it before. In the meantime I will get some users to test this. On Mon, 6 Apr 2020 at 20:57, Marton Balint <cus@passwd.hu> wrote: > > > On Mon, 6 Apr 2020, phunkyfish wrote: > > > --- > > libavformat/rtsp.c | 47 ++++++++++++++++++++++++++++++++++++++-------- > > 1 file changed, 39 insertions(+), 8 deletions(-) > > > > diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c > > index cd6fc32a29..0d0bc2be0d 100644 > > --- a/libavformat/rtsp.c > > +++ b/libavformat/rtsp.c > > @@ -2447,8 +2447,8 @@ static int rtp_probe(const AVProbeData *p) > > static int rtp_read_header(AVFormatContext *s) > > { > > uint8_t recvbuf[RTP_MAX_PACKET_LENGTH]; > > - char host[500], sdp[500]; > > - int ret, port; > > + char host[500], sdp[1000], filters_buf[1000]; > > + int ret, port, sdp_length, nc; > > URLContext* in = NULL; > > int payload_type; > > AVCodecParameters *par = NULL; > > @@ -2456,6 +2456,7 @@ static int rtp_read_header(AVFormatContext *s) > > AVIOContext pb; > > socklen_t addrlen = sizeof(addr); > > RTSPState *rt = s->priv_data; > > + const char *p; > > > > if (!ff_network_init()) > > return AVERROR(EIO); > > @@ -2513,13 +2514,40 @@ static int rtp_read_header(AVFormatContext *s) > > av_url_split(NULL, 0, NULL, 0, host, sizeof(host), &port, > > NULL, 0, s->url); > > > > - snprintf(sdp, sizeof(sdp), > > - "v=0\r\nc=IN IP%d %s\r\nm=%s %d RTP/AVP %d\r\n", > > - addr.ss_family == AF_INET ? 4 : 6, host, > > - par->codec_type == AVMEDIA_TYPE_DATA ? "application" : > > - par->codec_type == AVMEDIA_TYPE_VIDEO ? "video" : "audio", > > - port, payload_type); > > + sdp_length = snprintf(sdp, sizeof(sdp), > > + "v=0\r\nc=IN IP%d %s\r\n", > > + addr.ss_family == AF_INET ? 4 : 6, host); > > Please use an AVBPrint buffer instead of snprintf()-ing to sdp. > > Thanks, > Marton > > > + > > + p = strchr(s->url, '?'); > > + if (p) { > > + static const char *filters[][2] = {{"sources", "incl"}, > {"block", "excl"}, {NULL, NULL}}; > > + int i; > > + char *q; > > + for (i = 0; filters[i][0]; i++) { > > + if (av_find_info_tag(filters_buf, sizeof(filters_buf), > filters[i][0], p)) { > > + q = filters_buf; > > + while ((q = strchr(q, ',')) != NULL) > > + *q = ' '; > > + nc = snprintf(sdp + sdp_length, sizeof(sdp) - > sdp_length, > > + "a=source-filter:%s IN IP%d %s %s\r\n", > > + filters[i][1], > > + addr.ss_family == AF_INET ? 4 : 6, host, > > + filters_buf); > > + if (nc < 0 || nc + sdp_length >= sizeof(sdp)) > > + goto fail_nobuf; > > + sdp_length += nc; > > + } > > + } > > + } > > + > > + nc = snprintf(sdp + sdp_length, sizeof(sdp) - sdp_length, > > + "m=%s %d RTP/AVP %d\r\n", > > + par->codec_type == AVMEDIA_TYPE_DATA ? "application" > : > > + par->codec_type == AVMEDIA_TYPE_VIDEO ? "video" : > "audio", > > + port, payload_type); > > av_log(s, AV_LOG_VERBOSE, "SDP:\n%s\n", sdp); > > + if (nc < 0 || nc + sdp_length >= sizeof(sdp)) > > + goto fail_nobuf; > > avcodec_parameters_free(&par); > > > > ffio_init_context(&pb, sdp, strlen(sdp), 0, NULL, NULL, NULL, NULL); > > @@ -2534,6 +2562,9 @@ static int rtp_read_header(AVFormatContext *s) > > s->pb = NULL; > > return ret; > > > > +fail_nobuf: > > + ret = AVERROR(ENOBUFS); > > + av_log(s, AV_LOG_ERROR, "rtp_read_header(): not enough buffer space > for sdp-headers\n"); > > fail: > > avcodec_parameters_free(&par); > > if (in) > > -- > > 2.24.1 (Apple Git-126) > > > > _______________________________________________ > > 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". > _______________________________________________ > 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".
diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c index cd6fc32a29..0d0bc2be0d 100644 --- a/libavformat/rtsp.c +++ b/libavformat/rtsp.c @@ -2447,8 +2447,8 @@ static int rtp_probe(const AVProbeData *p) static int rtp_read_header(AVFormatContext *s) { uint8_t recvbuf[RTP_MAX_PACKET_LENGTH]; - char host[500], sdp[500]; - int ret, port; + char host[500], sdp[1000], filters_buf[1000]; + int ret, port, sdp_length, nc; URLContext* in = NULL; int payload_type; AVCodecParameters *par = NULL; @@ -2456,6 +2456,7 @@ static int rtp_read_header(AVFormatContext *s) AVIOContext pb; socklen_t addrlen = sizeof(addr); RTSPState *rt = s->priv_data; + const char *p; if (!ff_network_init()) return AVERROR(EIO); @@ -2513,13 +2514,40 @@ static int rtp_read_header(AVFormatContext *s) av_url_split(NULL, 0, NULL, 0, host, sizeof(host), &port, NULL, 0, s->url); - snprintf(sdp, sizeof(sdp), - "v=0\r\nc=IN IP%d %s\r\nm=%s %d RTP/AVP %d\r\n", - addr.ss_family == AF_INET ? 4 : 6, host, - par->codec_type == AVMEDIA_TYPE_DATA ? "application" : - par->codec_type == AVMEDIA_TYPE_VIDEO ? "video" : "audio", - port, payload_type); + sdp_length = snprintf(sdp, sizeof(sdp), + "v=0\r\nc=IN IP%d %s\r\n", + addr.ss_family == AF_INET ? 4 : 6, host); + + p = strchr(s->url, '?'); + if (p) { + static const char *filters[][2] = {{"sources", "incl"}, {"block", "excl"}, {NULL, NULL}}; + int i; + char *q; + for (i = 0; filters[i][0]; i++) { + if (av_find_info_tag(filters_buf, sizeof(filters_buf), filters[i][0], p)) { + q = filters_buf; + while ((q = strchr(q, ',')) != NULL) + *q = ' '; + nc = snprintf(sdp + sdp_length, sizeof(sdp) - sdp_length, + "a=source-filter:%s IN IP%d %s %s\r\n", + filters[i][1], + addr.ss_family == AF_INET ? 4 : 6, host, + filters_buf); + if (nc < 0 || nc + sdp_length >= sizeof(sdp)) + goto fail_nobuf; + sdp_length += nc; + } + } + } + + nc = snprintf(sdp + sdp_length, sizeof(sdp) - sdp_length, + "m=%s %d RTP/AVP %d\r\n", + par->codec_type == AVMEDIA_TYPE_DATA ? "application" : + par->codec_type == AVMEDIA_TYPE_VIDEO ? "video" : "audio", + port, payload_type); av_log(s, AV_LOG_VERBOSE, "SDP:\n%s\n", sdp); + if (nc < 0 || nc + sdp_length >= sizeof(sdp)) + goto fail_nobuf; avcodec_parameters_free(&par); ffio_init_context(&pb, sdp, strlen(sdp), 0, NULL, NULL, NULL, NULL); @@ -2534,6 +2562,9 @@ static int rtp_read_header(AVFormatContext *s) s->pb = NULL; return ret; +fail_nobuf: + ret = AVERROR(ENOBUFS); + av_log(s, AV_LOG_ERROR, "rtp_read_header(): not enough buffer space for sdp-headers\n"); fail: avcodec_parameters_free(&par); if (in)