| Message ID | 20200407100743.54801-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 |
On Tue, 7 Apr 2020, phunkyfish wrote: > --- > libavformat/rtsp.c | 48 +++++++++++++++++++++++++++++++++++++--------- > 1 file changed, 39 insertions(+), 9 deletions(-) > > diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c > index cd6fc32a29..dad3f7915e 100644 > --- a/libavformat/rtsp.c > +++ b/libavformat/rtsp.c > @@ -21,6 +21,7 @@ > > #include "libavutil/avassert.h" > #include "libavutil/base64.h" > +#include "libavutil/bprint.h" > #include "libavutil/avstring.h" > #include "libavutil/intreadwrite.h" > #include "libavutil/mathematics.h" > @@ -2447,7 +2448,7 @@ 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]; > + char host[500], filters_buf[1000]; > int ret, port; > URLContext* in = NULL; > int payload_type; > @@ -2456,6 +2457,8 @@ static int rtp_read_header(AVFormatContext *s) > AVIOContext pb; > socklen_t addrlen = sizeof(addr); > RTSPState *rt = s->priv_data; > + const char *p; > + AVBPrint sdp; > > if (!ff_network_init()) > return AVERROR(EIO); > @@ -2513,16 +2516,38 @@ 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); > - av_log(s, AV_LOG_VERBOSE, "SDP:\n%s\n", sdp); > + av_bprint_init(&sdp, 0, AV_BPRINT_SIZE_UNLIMITED); You may also use AV_BPRINT_SIZE_AUTOMATIC in which case you will get a static buffer which will be limited (roughly 1000 chars) but you don't have to free it with av_bprint_finalize(). > + av_bprintf(&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 = ' '; > + av_bprintf(&sdp, "a=source-filter:%s IN IP%d %s %s\r\n", > + filters[i][1], > + addr.ss_family == AF_INET ? 4 : 6, host, > + filters_buf); > + } > + } > + } > + > + av_bprintf(&sdp, "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.str); > + if (av_bprint_is_complete(&sdp)) I think this check should be negated here, because you want to report error if the buffer is truncated, not if it is complete. > + goto fail_nobuf; > avcodec_parameters_free(&par); > > - ffio_init_context(&pb, sdp, strlen(sdp), 0, NULL, NULL, NULL, NULL); > + ffio_init_context(&pb, sdp.str, strlen(sdp.str), 0, NULL, NULL, NULL, NULL); You can use sdp.len instead of strlen(). > s->pb = &pb; > > /* sdp_read_header initializes this again */ > @@ -2532,9 +2557,14 @@ static int rtp_read_header(AVFormatContext *s) > > ret = sdp_read_header(s); > s->pb = NULL; > + av_bprint_finalize(&sdp, NULL); > return ret; > > +fail_nobuf: > + ret = AVERROR(ENOMEM); > + av_log(s, AV_LOG_ERROR, "rtp_read_header(): not enough buffer space for sdp-headers\n"); > fail: > + av_bprint_finalize(&sdp, NULL); > avcodec_parameters_free(&par); > if (in) > ffurl_close(in); Regards, Marton
Great, thanks again. A question about AV_BPRINT_SIZE_AUTOMATIC. Is there a heuristic for when to use this versus unlimited? Or is it that generally if you would have used a buffer of 1000 or less automatic is the right choice? > On 7 Apr 2020, at 20:50, Marton Balint <cus@passwd.hu> wrote: > > > >> On Tue, 7 Apr 2020, phunkyfish wrote: >> >> --- >> libavformat/rtsp.c | 48 +++++++++++++++++++++++++++++++++++++--------- >> 1 file changed, 39 insertions(+), 9 deletions(-) >> >> diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c >> index cd6fc32a29..dad3f7915e 100644 >> --- a/libavformat/rtsp.c >> +++ b/libavformat/rtsp.c >> @@ -21,6 +21,7 @@ >> #include "libavutil/avassert.h" >> #include "libavutil/base64.h" >> +#include "libavutil/bprint.h" >> #include "libavutil/avstring.h" >> #include "libavutil/intreadwrite.h" >> #include "libavutil/mathematics.h" >> @@ -2447,7 +2448,7 @@ 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]; >> + char host[500], filters_buf[1000]; >> int ret, port; >> URLContext* in = NULL; >> int payload_type; >> @@ -2456,6 +2457,8 @@ static int rtp_read_header(AVFormatContext *s) >> AVIOContext pb; >> socklen_t addrlen = sizeof(addr); >> RTSPState *rt = s->priv_data; >> + const char *p; >> + AVBPrint sdp; >> >> if (!ff_network_init()) >> return AVERROR(EIO); >> @@ -2513,16 +2516,38 @@ 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); >> - av_log(s, AV_LOG_VERBOSE, "SDP:\n%s\n", sdp); >> + av_bprint_init(&sdp, 0, AV_BPRINT_SIZE_UNLIMITED); > > You may also use AV_BPRINT_SIZE_AUTOMATIC in which case you will get a static buffer which will be limited (roughly 1000 chars) but you don't have to free it with av_bprint_finalize(). > >> + av_bprintf(&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 = ' '; >> + av_bprintf(&sdp, "a=source-filter:%s IN IP%d %s %s\r\n", >> + filters[i][1], >> + addr.ss_family == AF_INET ? 4 : 6, host, >> + filters_buf); >> + } >> + } >> + } >> + >> + av_bprintf(&sdp, "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.str); >> + if (av_bprint_is_complete(&sdp)) > > I think this check should be negated here, because you want to report error if the buffer is truncated, not if it is complete. > >> + goto fail_nobuf; >> avcodec_parameters_free(&par); >> - ffio_init_context(&pb, sdp, strlen(sdp), 0, NULL, NULL, NULL, NULL); >> + ffio_init_context(&pb, sdp.str, strlen(sdp.str), 0, NULL, NULL, NULL, NULL); > > You can use sdp.len instead of strlen(). > >> s->pb = &pb; >> >> /* sdp_read_header initializes this again */ >> @@ -2532,9 +2557,14 @@ static int rtp_read_header(AVFormatContext *s) >> >> ret = sdp_read_header(s); >> s->pb = NULL; >> + av_bprint_finalize(&sdp, NULL); >> return ret; >> +fail_nobuf: >> + ret = AVERROR(ENOMEM); >> + av_log(s, AV_LOG_ERROR, "rtp_read_header(): not enough buffer space for sdp-headers\n"); >> fail: >> + av_bprint_finalize(&sdp, NULL); >> avcodec_parameters_free(&par); >> if (in) >> ffurl_close(in); > > Regards, > Marton > _______________________________________________ > 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".
On Tue, 7 Apr 2020, Ross Nicholson wrote: > Great, thanks again. > > A question about AV_BPRINT_SIZE_AUTOMATIC. Is there a heuristic for when to use this versus unlimited? > > Or is it that generally if you would have used a buffer of 1000 or less automatic is the right choice? It depends on what you want. With AUTOMATIC you limit length to 1000 chars but you don't have to free the buffer. Otherwise you are not limiting the buffer size, but you have to free it. So if it is impossible to hit the limit, you should always use AUTOMATIC. In this case you have to decide for yourself which to use, because I don't know if 1000 char buffer is big enough for the possible use cases. I only suggested to consider it, because you are using limited buffers for other strings, so it may not even be possible to outgrow 1000 chars. It is up to you decide depending on what you want to support. Regards, Marton > >> On 7 Apr 2020, at 20:50, Marton Balint <cus@passwd.hu> wrote: >> >> >> >>> On Tue, 7 Apr 2020, phunkyfish wrote: >>> >>> --- >>> libavformat/rtsp.c | 48 +++++++++++++++++++++++++++++++++++++--------- >>> 1 file changed, 39 insertions(+), 9 deletions(-) >>> >>> diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c >>> index cd6fc32a29..dad3f7915e 100644 >>> --- a/libavformat/rtsp.c >>> +++ b/libavformat/rtsp.c >>> @@ -21,6 +21,7 @@ >>> #include "libavutil/avassert.h" >>> #include "libavutil/base64.h" >>> +#include "libavutil/bprint.h" >>> #include "libavutil/avstring.h" >>> #include "libavutil/intreadwrite.h" >>> #include "libavutil/mathematics.h" >>> @@ -2447,7 +2448,7 @@ 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]; >>> + char host[500], filters_buf[1000]; >>> int ret, port; >>> URLContext* in = NULL; >>> int payload_type; >>> @@ -2456,6 +2457,8 @@ static int rtp_read_header(AVFormatContext *s) >>> AVIOContext pb; >>> socklen_t addrlen = sizeof(addr); >>> RTSPState *rt = s->priv_data; >>> + const char *p; >>> + AVBPrint sdp; >>> >>> if (!ff_network_init()) >>> return AVERROR(EIO); >>> @@ -2513,16 +2516,38 @@ 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); >>> - av_log(s, AV_LOG_VERBOSE, "SDP:\n%s\n", sdp); >>> + av_bprint_init(&sdp, 0, AV_BPRINT_SIZE_UNLIMITED); >> >> You may also use AV_BPRINT_SIZE_AUTOMATIC in which case you will get a static buffer which will be limited (roughly 1000 chars) but you don't have to free it with av_bprint_finalize(). >> >>> + av_bprintf(&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 = ' '; >>> + av_bprintf(&sdp, "a=source-filter:%s IN IP%d %s %s\r\n", >>> + filters[i][1], >>> + addr.ss_family == AF_INET ? 4 : 6, host, >>> + filters_buf); >>> + } >>> + } >>> + } >>> + >>> + av_bprintf(&sdp, "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.str); >>> + if (av_bprint_is_complete(&sdp)) >> >> I think this check should be negated here, because you want to report error if the buffer is truncated, not if it is complete. >> >>> + goto fail_nobuf; >>> avcodec_parameters_free(&par); >>> - ffio_init_context(&pb, sdp, strlen(sdp), 0, NULL, NULL, NULL, NULL); >>> + ffio_init_context(&pb, sdp.str, strlen(sdp.str), 0, NULL, NULL, NULL, NULL); >> >> You can use sdp.len instead of strlen(). >> >>> s->pb = &pb; >>> >>> /* sdp_read_header initializes this again */ >>> @@ -2532,9 +2557,14 @@ static int rtp_read_header(AVFormatContext *s) >>> >>> ret = sdp_read_header(s); >>> s->pb = NULL; >>> + av_bprint_finalize(&sdp, NULL); >>> return ret; >>> +fail_nobuf: >>> + ret = AVERROR(ENOMEM); >>> + av_log(s, AV_LOG_ERROR, "rtp_read_header(): not enough buffer space for sdp-headers\n"); >>> fail: >>> + av_bprint_finalize(&sdp, NULL); >>> avcodec_parameters_free(&par); >>> if (in) >>> ffurl_close(in); >> >> Regards, >> Marton >> _______________________________________________ >> 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".
Thank you for the explanation Marton. It's make perfect sense to me know. So UNLIMITED would be the right choice here. All of your other comments are addressed in the latest version. Thanks again for reviewing. On Tue, 7 Apr 2020 at 22:03, Marton Balint <cus@passwd.hu> wrote: > > > On Tue, 7 Apr 2020, Ross Nicholson wrote: > > > Great, thanks again. > > > > A question about AV_BPRINT_SIZE_AUTOMATIC. Is there a heuristic for when > to use this versus unlimited? > > > > Or is it that generally if you would have used a buffer of 1000 or less > automatic is the right choice? > > It depends on what you want. With AUTOMATIC you limit length to 1000 chars > but you don't have to free the buffer. Otherwise you are not limiting the > buffer size, but you have to free it. So if it is impossible to hit the > limit, you should always use AUTOMATIC. > > In this case you have to decide for yourself which to use, because I don't > know if 1000 char buffer is big enough for the possible use cases. I > only suggested to consider it, because you are using limited buffers for > other strings, so it may not even be possible to outgrow 1000 chars. It is > up to you decide depending on what you want to support. > > Regards, > Marton > > > > >> On 7 Apr 2020, at 20:50, Marton Balint <cus@passwd.hu> wrote: > >> > >> > >> > >>> On Tue, 7 Apr 2020, phunkyfish wrote: > >>> > >>> --- > >>> libavformat/rtsp.c | 48 +++++++++++++++++++++++++++++++++++++--------- > >>> 1 file changed, 39 insertions(+), 9 deletions(-) > >>> > >>> diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c > >>> index cd6fc32a29..dad3f7915e 100644 > >>> --- a/libavformat/rtsp.c > >>> +++ b/libavformat/rtsp.c > >>> @@ -21,6 +21,7 @@ > >>> #include "libavutil/avassert.h" > >>> #include "libavutil/base64.h" > >>> +#include "libavutil/bprint.h" > >>> #include "libavutil/avstring.h" > >>> #include "libavutil/intreadwrite.h" > >>> #include "libavutil/mathematics.h" > >>> @@ -2447,7 +2448,7 @@ 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]; > >>> + char host[500], filters_buf[1000]; > >>> int ret, port; > >>> URLContext* in = NULL; > >>> int payload_type; > >>> @@ -2456,6 +2457,8 @@ static int rtp_read_header(AVFormatContext *s) > >>> AVIOContext pb; > >>> socklen_t addrlen = sizeof(addr); > >>> RTSPState *rt = s->priv_data; > >>> + const char *p; > >>> + AVBPrint sdp; > >>> > >>> if (!ff_network_init()) > >>> return AVERROR(EIO); > >>> @@ -2513,16 +2516,38 @@ 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); > >>> - av_log(s, AV_LOG_VERBOSE, "SDP:\n%s\n", sdp); > >>> + av_bprint_init(&sdp, 0, AV_BPRINT_SIZE_UNLIMITED); > >> > >> You may also use AV_BPRINT_SIZE_AUTOMATIC in which case you will get a > static buffer which will be limited (roughly 1000 chars) but you don't have > to free it with av_bprint_finalize(). > >> > >>> + av_bprintf(&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 = ' '; > >>> + av_bprintf(&sdp, "a=source-filter:%s IN IP%d %s > %s\r\n", > >>> + filters[i][1], > >>> + addr.ss_family == AF_INET ? 4 : 6, host, > >>> + filters_buf); > >>> + } > >>> + } > >>> + } > >>> + > >>> + av_bprintf(&sdp, "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.str); > >>> + if (av_bprint_is_complete(&sdp)) > >> > >> I think this check should be negated here, because you want to report > error if the buffer is truncated, not if it is complete. > >> > >>> + goto fail_nobuf; > >>> avcodec_parameters_free(&par); > >>> - ffio_init_context(&pb, sdp, strlen(sdp), 0, NULL, NULL, NULL, > NULL); > >>> + ffio_init_context(&pb, sdp.str, strlen(sdp.str), 0, NULL, NULL, > NULL, NULL); > >> > >> You can use sdp.len instead of strlen(). > >> > >>> s->pb = &pb; > >>> > >>> /* sdp_read_header initializes this again */ > >>> @@ -2532,9 +2557,14 @@ static int rtp_read_header(AVFormatContext *s) > >>> > >>> ret = sdp_read_header(s); > >>> s->pb = NULL; > >>> + av_bprint_finalize(&sdp, NULL); > >>> return ret; > >>> +fail_nobuf: > >>> + ret = AVERROR(ENOMEM); > >>> + av_log(s, AV_LOG_ERROR, "rtp_read_header(): not enough buffer > space for sdp-headers\n"); > >>> fail: > >>> + av_bprint_finalize(&sdp, NULL); > >>> avcodec_parameters_free(&par); > >>> if (in) > >>> ffurl_close(in); > >> > >> Regards, > >> Marton > >> _______________________________________________ > >> 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". > _______________________________________________ > 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".
User testing has been completed successfully so this is ready to be applied. Thanks > On 7 Apr 2020, at 23:50, Ross Nicholson <phunkyfish@gmail.com> wrote: > > > Thank you for the explanation Marton. It's make perfect sense to me know. So UNLIMITED would be the right choice here. > > All of your other comments are addressed in the latest version. Thanks again for reviewing. > >> On Tue, 7 Apr 2020 at 22:03, Marton Balint <cus@passwd.hu> wrote: >> >> >> On Tue, 7 Apr 2020, Ross Nicholson wrote: >> >> > Great, thanks again. >> > >> > A question about AV_BPRINT_SIZE_AUTOMATIC. Is there a heuristic for when to use this versus unlimited? >> > >> > Or is it that generally if you would have used a buffer of 1000 or less automatic is the right choice? >> >> It depends on what you want. With AUTOMATIC you limit length to 1000 chars >> but you don't have to free the buffer. Otherwise you are not limiting the >> buffer size, but you have to free it. So if it is impossible to hit the >> limit, you should always use AUTOMATIC. >> >> In this case you have to decide for yourself which to use, because I don't >> know if 1000 char buffer is big enough for the possible use cases. I >> only suggested to consider it, because you are using limited buffers for >> other strings, so it may not even be possible to outgrow 1000 chars. It is >> up to you decide depending on what you want to support. >> >> Regards, >> Marton >> >> > >> >> On 7 Apr 2020, at 20:50, Marton Balint <cus@passwd.hu> wrote: >> >> >> >> >> >> >> >>> On Tue, 7 Apr 2020, phunkyfish wrote: >> >>> >> >>> --- >> >>> libavformat/rtsp.c | 48 +++++++++++++++++++++++++++++++++++++--------- >> >>> 1 file changed, 39 insertions(+), 9 deletions(-) >> >>> >> >>> diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c >> >>> index cd6fc32a29..dad3f7915e 100644 >> >>> --- a/libavformat/rtsp.c >> >>> +++ b/libavformat/rtsp.c >> >>> @@ -21,6 +21,7 @@ >> >>> #include "libavutil/avassert.h" >> >>> #include "libavutil/base64.h" >> >>> +#include "libavutil/bprint.h" >> >>> #include "libavutil/avstring.h" >> >>> #include "libavutil/intreadwrite.h" >> >>> #include "libavutil/mathematics.h" >> >>> @@ -2447,7 +2448,7 @@ 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]; >> >>> + char host[500], filters_buf[1000]; >> >>> int ret, port; >> >>> URLContext* in = NULL; >> >>> int payload_type; >> >>> @@ -2456,6 +2457,8 @@ static int rtp_read_header(AVFormatContext *s) >> >>> AVIOContext pb; >> >>> socklen_t addrlen = sizeof(addr); >> >>> RTSPState *rt = s->priv_data; >> >>> + const char *p; >> >>> + AVBPrint sdp; >> >>> >> >>> if (!ff_network_init()) >> >>> return AVERROR(EIO); >> >>> @@ -2513,16 +2516,38 @@ 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); >> >>> - av_log(s, AV_LOG_VERBOSE, "SDP:\n%s\n", sdp); >> >>> + av_bprint_init(&sdp, 0, AV_BPRINT_SIZE_UNLIMITED); >> >> >> >> You may also use AV_BPRINT_SIZE_AUTOMATIC in which case you will get a static buffer which will be limited (roughly 1000 chars) but you don't have to free it with av_bprint_finalize(). >> >> >> >>> + av_bprintf(&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 = ' '; >> >>> + av_bprintf(&sdp, "a=source-filter:%s IN IP%d %s %s\r\n", >> >>> + filters[i][1], >> >>> + addr.ss_family == AF_INET ? 4 : 6, host, >> >>> + filters_buf); >> >>> + } >> >>> + } >> >>> + } >> >>> + >> >>> + av_bprintf(&sdp, "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.str); >> >>> + if (av_bprint_is_complete(&sdp)) >> >> >> >> I think this check should be negated here, because you want to report error if the buffer is truncated, not if it is complete. >> >> >> >>> + goto fail_nobuf; >> >>> avcodec_parameters_free(&par); >> >>> - ffio_init_context(&pb, sdp, strlen(sdp), 0, NULL, NULL, NULL, NULL); >> >>> + ffio_init_context(&pb, sdp.str, strlen(sdp.str), 0, NULL, NULL, NULL, NULL); >> >> >> >> You can use sdp.len instead of strlen(). >> >> >> >>> s->pb = &pb; >> >>> >> >>> /* sdp_read_header initializes this again */ >> >>> @@ -2532,9 +2557,14 @@ static int rtp_read_header(AVFormatContext *s) >> >>> >> >>> ret = sdp_read_header(s); >> >>> s->pb = NULL; >> >>> + av_bprint_finalize(&sdp, NULL); >> >>> return ret; >> >>> +fail_nobuf: >> >>> + ret = AVERROR(ENOMEM); >> >>> + av_log(s, AV_LOG_ERROR, "rtp_read_header(): not enough buffer space for sdp-headers\n"); >> >>> fail: >> >>> + av_bprint_finalize(&sdp, NULL); >> >>> avcodec_parameters_free(&par); >> >>> if (in) >> >>> ffurl_close(in); >> >> >> >> Regards, >> >> Marton >> >> _______________________________________________ >> >> 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". >> _______________________________________________ >> 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".
Ping to hopefully apply this patch! > On 12 Apr 2020, at 16:54, Ross Nicholson <phunkyfish@gmail.com> wrote: > > > User testing has been completed successfully so this is ready to be applied. > > Thanks > >>> On 7 Apr 2020, at 23:50, Ross Nicholson <phunkyfish@gmail.com> wrote: >>> >> >> Thank you for the explanation Marton. It's make perfect sense to me know. So UNLIMITED would be the right choice here. >> >> All of your other comments are addressed in the latest version. Thanks again for reviewing. >> >>> On Tue, 7 Apr 2020 at 22:03, Marton Balint <cus@passwd.hu> wrote: >>> >>> >>> On Tue, 7 Apr 2020, Ross Nicholson wrote: >>> >>> > Great, thanks again. >>> > >>> > A question about AV_BPRINT_SIZE_AUTOMATIC. Is there a heuristic for when to use this versus unlimited? >>> > >>> > Or is it that generally if you would have used a buffer of 1000 or less automatic is the right choice? >>> >>> It depends on what you want. With AUTOMATIC you limit length to 1000 chars >>> but you don't have to free the buffer. Otherwise you are not limiting the >>> buffer size, but you have to free it. So if it is impossible to hit the >>> limit, you should always use AUTOMATIC. >>> >>> In this case you have to decide for yourself which to use, because I don't >>> know if 1000 char buffer is big enough for the possible use cases. I >>> only suggested to consider it, because you are using limited buffers for >>> other strings, so it may not even be possible to outgrow 1000 chars. It is >>> up to you decide depending on what you want to support. >>> >>> Regards, >>> Marton >>> >>> > >>> >> On 7 Apr 2020, at 20:50, Marton Balint <cus@passwd.hu> wrote: >>> >> >>> >> >>> >> >>> >>> On Tue, 7 Apr 2020, phunkyfish wrote: >>> >>> >>> >>> --- >>> >>> libavformat/rtsp.c | 48 +++++++++++++++++++++++++++++++++++++--------- >>> >>> 1 file changed, 39 insertions(+), 9 deletions(-) >>> >>> >>> >>> diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c >>> >>> index cd6fc32a29..dad3f7915e 100644 >>> >>> --- a/libavformat/rtsp.c >>> >>> +++ b/libavformat/rtsp.c >>> >>> @@ -21,6 +21,7 @@ >>> >>> #include "libavutil/avassert.h" >>> >>> #include "libavutil/base64.h" >>> >>> +#include "libavutil/bprint.h" >>> >>> #include "libavutil/avstring.h" >>> >>> #include "libavutil/intreadwrite.h" >>> >>> #include "libavutil/mathematics.h" >>> >>> @@ -2447,7 +2448,7 @@ 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]; >>> >>> + char host[500], filters_buf[1000]; >>> >>> int ret, port; >>> >>> URLContext* in = NULL; >>> >>> int payload_type; >>> >>> @@ -2456,6 +2457,8 @@ static int rtp_read_header(AVFormatContext *s) >>> >>> AVIOContext pb; >>> >>> socklen_t addrlen = sizeof(addr); >>> >>> RTSPState *rt = s->priv_data; >>> >>> + const char *p; >>> >>> + AVBPrint sdp; >>> >>> >>> >>> if (!ff_network_init()) >>> >>> return AVERROR(EIO); >>> >>> @@ -2513,16 +2516,38 @@ 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); >>> >>> - av_log(s, AV_LOG_VERBOSE, "SDP:\n%s\n", sdp); >>> >>> + av_bprint_init(&sdp, 0, AV_BPRINT_SIZE_UNLIMITED); >>> >> >>> >> You may also use AV_BPRINT_SIZE_AUTOMATIC in which case you will get a static buffer which will be limited (roughly 1000 chars) but you don't have to free it with av_bprint_finalize(). >>> >> >>> >>> + av_bprintf(&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 = ' '; >>> >>> + av_bprintf(&sdp, "a=source-filter:%s IN IP%d %s %s\r\n", >>> >>> + filters[i][1], >>> >>> + addr.ss_family == AF_INET ? 4 : 6, host, >>> >>> + filters_buf); >>> >>> + } >>> >>> + } >>> >>> + } >>> >>> + >>> >>> + av_bprintf(&sdp, "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.str); >>> >>> + if (av_bprint_is_complete(&sdp)) >>> >> >>> >> I think this check should be negated here, because you want to report error if the buffer is truncated, not if it is complete. >>> >> >>> >>> + goto fail_nobuf; >>> >>> avcodec_parameters_free(&par); >>> >>> - ffio_init_context(&pb, sdp, strlen(sdp), 0, NULL, NULL, NULL, NULL); >>> >>> + ffio_init_context(&pb, sdp.str, strlen(sdp.str), 0, NULL, NULL, NULL, NULL); >>> >> >>> >> You can use sdp.len instead of strlen(). >>> >> >>> >>> s->pb = &pb; >>> >>> >>> >>> /* sdp_read_header initializes this again */ >>> >>> @@ -2532,9 +2557,14 @@ static int rtp_read_header(AVFormatContext *s) >>> >>> >>> >>> ret = sdp_read_header(s); >>> >>> s->pb = NULL; >>> >>> + av_bprint_finalize(&sdp, NULL); >>> >>> return ret; >>> >>> +fail_nobuf: >>> >>> + ret = AVERROR(ENOMEM); >>> >>> + av_log(s, AV_LOG_ERROR, "rtp_read_header(): not enough buffer space for sdp-headers\n"); >>> >>> fail: >>> >>> + av_bprint_finalize(&sdp, NULL); >>> >>> avcodec_parameters_free(&par); >>> >>> if (in) >>> >>> ffurl_close(in); >>> >> >>> >> Regards, >>> >> Marton >>> >> _______________________________________________ >>> >> 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". >>> _______________________________________________ >>> 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".
Ping > On 15 Apr 2020, at 17:21, Ross Nicholson <phunkyfish@gmail.com> wrote: > > > Ping to hopefully apply this patch! > >>> On 12 Apr 2020, at 16:54, Ross Nicholson <phunkyfish@gmail.com> wrote: >>> >> >> User testing has been completed successfully so this is ready to be applied. >> >> Thanks >> >>>> On 7 Apr 2020, at 23:50, Ross Nicholson <phunkyfish@gmail.com> wrote: >>>> >>> >>> Thank you for the explanation Marton. It's make perfect sense to me know. So UNLIMITED would be the right choice here. >>> >>> All of your other comments are addressed in the latest version. Thanks again for reviewing. >>> >>>> On Tue, 7 Apr 2020 at 22:03, Marton Balint <cus@passwd.hu> wrote: >>>> >>>> >>>> On Tue, 7 Apr 2020, Ross Nicholson wrote: >>>> >>>> > Great, thanks again. >>>> > >>>> > A question about AV_BPRINT_SIZE_AUTOMATIC. Is there a heuristic for when to use this versus unlimited? >>>> > >>>> > Or is it that generally if you would have used a buffer of 1000 or less automatic is the right choice? >>>> >>>> It depends on what you want. With AUTOMATIC you limit length to 1000 chars >>>> but you don't have to free the buffer. Otherwise you are not limiting the >>>> buffer size, but you have to free it. So if it is impossible to hit the >>>> limit, you should always use AUTOMATIC. >>>> >>>> In this case you have to decide for yourself which to use, because I don't >>>> know if 1000 char buffer is big enough for the possible use cases. I >>>> only suggested to consider it, because you are using limited buffers for >>>> other strings, so it may not even be possible to outgrow 1000 chars. It is >>>> up to you decide depending on what you want to support. >>>> >>>> Regards, >>>> Marton >>>> >>>> > >>>> >> On 7 Apr 2020, at 20:50, Marton Balint <cus@passwd.hu> wrote: >>>> >> >>>> >> >>>> >> >>>> >>> On Tue, 7 Apr 2020, phunkyfish wrote: >>>> >>> >>>> >>> --- >>>> >>> libavformat/rtsp.c | 48 +++++++++++++++++++++++++++++++++++++--------- >>>> >>> 1 file changed, 39 insertions(+), 9 deletions(-) >>>> >>> >>>> >>> diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c >>>> >>> index cd6fc32a29..dad3f7915e 100644 >>>> >>> --- a/libavformat/rtsp.c >>>> >>> +++ b/libavformat/rtsp.c >>>> >>> @@ -21,6 +21,7 @@ >>>> >>> #include "libavutil/avassert.h" >>>> >>> #include "libavutil/base64.h" >>>> >>> +#include "libavutil/bprint.h" >>>> >>> #include "libavutil/avstring.h" >>>> >>> #include "libavutil/intreadwrite.h" >>>> >>> #include "libavutil/mathematics.h" >>>> >>> @@ -2447,7 +2448,7 @@ 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]; >>>> >>> + char host[500], filters_buf[1000]; >>>> >>> int ret, port; >>>> >>> URLContext* in = NULL; >>>> >>> int payload_type; >>>> >>> @@ -2456,6 +2457,8 @@ static int rtp_read_header(AVFormatContext *s) >>>> >>> AVIOContext pb; >>>> >>> socklen_t addrlen = sizeof(addr); >>>> >>> RTSPState *rt = s->priv_data; >>>> >>> + const char *p; >>>> >>> + AVBPrint sdp; >>>> >>> >>>> >>> if (!ff_network_init()) >>>> >>> return AVERROR(EIO); >>>> >>> @@ -2513,16 +2516,38 @@ 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); >>>> >>> - av_log(s, AV_LOG_VERBOSE, "SDP:\n%s\n", sdp); >>>> >>> + av_bprint_init(&sdp, 0, AV_BPRINT_SIZE_UNLIMITED); >>>> >> >>>> >> You may also use AV_BPRINT_SIZE_AUTOMATIC in which case you will get a static buffer which will be limited (roughly 1000 chars) but you don't have to free it with av_bprint_finalize(). >>>> >> >>>> >>> + av_bprintf(&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 = ' '; >>>> >>> + av_bprintf(&sdp, "a=source-filter:%s IN IP%d %s %s\r\n", >>>> >>> + filters[i][1], >>>> >>> + addr.ss_family == AF_INET ? 4 : 6, host, >>>> >>> + filters_buf); >>>> >>> + } >>>> >>> + } >>>> >>> + } >>>> >>> + >>>> >>> + av_bprintf(&sdp, "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.str); >>>> >>> + if (av_bprint_is_complete(&sdp)) >>>> >> >>>> >> I think this check should be negated here, because you want to report error if the buffer is truncated, not if it is complete. >>>> >> >>>> >>> + goto fail_nobuf; >>>> >>> avcodec_parameters_free(&par); >>>> >>> - ffio_init_context(&pb, sdp, strlen(sdp), 0, NULL, NULL, NULL, NULL); >>>> >>> + ffio_init_context(&pb, sdp.str, strlen(sdp.str), 0, NULL, NULL, NULL, NULL); >>>> >> >>>> >> You can use sdp.len instead of strlen(). >>>> >> >>>> >>> s->pb = &pb; >>>> >>> >>>> >>> /* sdp_read_header initializes this again */ >>>> >>> @@ -2532,9 +2557,14 @@ static int rtp_read_header(AVFormatContext *s) >>>> >>> >>>> >>> ret = sdp_read_header(s); >>>> >>> s->pb = NULL; >>>> >>> + av_bprint_finalize(&sdp, NULL); >>>> >>> return ret; >>>> >>> +fail_nobuf: >>>> >>> + ret = AVERROR(ENOMEM); >>>> >>> + av_log(s, AV_LOG_ERROR, "rtp_read_header(): not enough buffer space for sdp-headers\n"); >>>> >>> fail: >>>> >>> + av_bprint_finalize(&sdp, NULL); >>>> >>> avcodec_parameters_free(&par); >>>> >>> if (in) >>>> >>> ffurl_close(in); >>>> >> >>>> >> Regards, >>>> >> Marton >>>> >> _______________________________________________ >>>> >> 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". >>>> _______________________________________________ >>>> 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".
Ross, Could you, please, fix your git email and name? Thanks On Fri, Apr 17, 2020, at 13:07, Ross Nicholson wrote: > Ping > > > On 15 Apr 2020, at 17:21, Ross Nicholson <phunkyfish@gmail.com> wrote: > > > > > > Ping to hopefully apply this patch! > > > >>> On 12 Apr 2020, at 16:54, Ross Nicholson <phunkyfish@gmail.com> wrote: > >>> > >> > >> User testing has been completed successfully so this is ready to be applied. > >> > >> Thanks > >> > >>>> On 7 Apr 2020, at 23:50, Ross Nicholson <phunkyfish@gmail.com> wrote: > >>>> > >>> > >>> Thank you for the explanation Marton. It's make perfect sense to me know. So UNLIMITED would be the right choice here. > >>> > >>> All of your other comments are addressed in the latest version. Thanks again for reviewing. > >>> > >>>> On Tue, 7 Apr 2020 at 22:03, Marton Balint <cus@passwd.hu> wrote: > >>>> > >>>> > >>>> On Tue, 7 Apr 2020, Ross Nicholson wrote: > >>>> > >>>> > Great, thanks again. > >>>> > > >>>> > A question about AV_BPRINT_SIZE_AUTOMATIC. Is there a heuristic for when to use this versus unlimited? > >>>> > > >>>> > Or is it that generally if you would have used a buffer of 1000 or less automatic is the right choice? > >>>> > >>>> It depends on what you want. With AUTOMATIC you limit length to 1000 chars > >>>> but you don't have to free the buffer. Otherwise you are not limiting the > >>>> buffer size, but you have to free it. So if it is impossible to hit the > >>>> limit, you should always use AUTOMATIC. > >>>> > >>>> In this case you have to decide for yourself which to use, because I don't > >>>> know if 1000 char buffer is big enough for the possible use cases. I > >>>> only suggested to consider it, because you are using limited buffers for > >>>> other strings, so it may not even be possible to outgrow 1000 chars. It is > >>>> up to you decide depending on what you want to support. > >>>> > >>>> Regards, > >>>> Marton > >>>> > >>>> > > >>>> >> On 7 Apr 2020, at 20:50, Marton Balint <cus@passwd.hu> wrote: > >>>> >> > >>>> >> > >>>> >> > >>>> >>> On Tue, 7 Apr 2020, phunkyfish wrote: > >>>> >>> > >>>> >>> --- > >>>> >>> libavformat/rtsp.c | 48 +++++++++++++++++++++++++++++++++++++--------- > >>>> >>> 1 file changed, 39 insertions(+), 9 deletions(-) > >>>> >>> > >>>> >>> diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c > >>>> >>> index cd6fc32a29..dad3f7915e 100644 > >>>> >>> --- a/libavformat/rtsp.c > >>>> >>> +++ b/libavformat/rtsp.c > >>>> >>> @@ -21,6 +21,7 @@ > >>>> >>> #include "libavutil/avassert.h" > >>>> >>> #include "libavutil/base64.h" > >>>> >>> +#include "libavutil/bprint.h" > >>>> >>> #include "libavutil/avstring.h" > >>>> >>> #include "libavutil/intreadwrite.h" > >>>> >>> #include "libavutil/mathematics.h" > >>>> >>> @@ -2447,7 +2448,7 @@ 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]; > >>>> >>> + char host[500], filters_buf[1000]; > >>>> >>> int ret, port; > >>>> >>> URLContext* in = NULL; > >>>> >>> int payload_type; > >>>> >>> @@ -2456,6 +2457,8 @@ static int rtp_read_header(AVFormatContext *s) > >>>> >>> AVIOContext pb; > >>>> >>> socklen_t addrlen = sizeof(addr); > >>>> >>> RTSPState *rt = s->priv_data; > >>>> >>> + const char *p; > >>>> >>> + AVBPrint sdp; > >>>> >>> > >>>> >>> if (!ff_network_init()) > >>>> >>> return AVERROR(EIO); > >>>> >>> @@ -2513,16 +2516,38 @@ 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); > >>>> >>> - av_log(s, AV_LOG_VERBOSE, "SDP:\n%s\n", sdp); > >>>> >>> + av_bprint_init(&sdp, 0, AV_BPRINT_SIZE_UNLIMITED); > >>>> >> > >>>> >> You may also use AV_BPRINT_SIZE_AUTOMATIC in which case you will get a static buffer which will be limited (roughly 1000 chars) but you don't have to free it with av_bprint_finalize(). > >>>> >> > >>>> >>> + av_bprintf(&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 = ' '; > >>>> >>> + av_bprintf(&sdp, "a=source-filter:%s IN IP%d %s %s\r\n", > >>>> >>> + filters[i][1], > >>>> >>> + addr.ss_family == AF_INET ? 4 : 6, host, > >>>> >>> + filters_buf); > >>>> >>> + } > >>>> >>> + } > >>>> >>> + } > >>>> >>> + > >>>> >>> + av_bprintf(&sdp, "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.str); > >>>> >>> + if (av_bprint_is_complete(&sdp)) > >>>> >> > >>>> >> I think this check should be negated here, because you want to report error if the buffer is truncated, not if it is complete. > >>>> >> > >>>> >>> + goto fail_nobuf; > >>>> >>> avcodec_parameters_free(&par); > >>>> >>> - ffio_init_context(&pb, sdp, strlen(sdp), 0, NULL, NULL, NULL, NULL); > >>>> >>> + ffio_init_context(&pb, sdp.str, strlen(sdp.str), 0, NULL, NULL, NULL, NULL); > >>>> >> > >>>> >> You can use sdp.len instead of strlen(). > >>>> >> > >>>> >>> s->pb = &pb; > >>>> >>> > >>>> >>> /* sdp_read_header initializes this again */ > >>>> >>> @@ -2532,9 +2557,14 @@ static int rtp_read_header(AVFormatContext *s) > >>>> >>> > >>>> >>> ret = sdp_read_header(s); > >>>> >>> s->pb = NULL; > >>>> >>> + av_bprint_finalize(&sdp, NULL); > >>>> >>> return ret; > >>>> >>> +fail_nobuf: > >>>> >>> + ret = AVERROR(ENOMEM); > >>>> >>> + av_log(s, AV_LOG_ERROR, "rtp_read_header(): not enough buffer space for sdp-headers\n"); > >>>> >>> fail: > >>>> >>> + av_bprint_finalize(&sdp, NULL); > >>>> >>> avcodec_parameters_free(&par); > >>>> >>> if (in) > >>>> >>> ffurl_close(in); > >>>> >> > >>>> >> Regards, > >>>> >> Marton > >>>> >> _______________________________________________ > >>>> >> 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". > >>>> _______________________________________________ > >>>> 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".
How do you mean? What’s that problem? I generally just use phunkyfish as the name and it’s corresponding gmail address for stuff. Do you require my real name for git? > On 19 Apr 2020, at 14:23, Jean-Baptiste Kempf <jb@videolan.org> wrote: > > Ross, > > Could you, please, fix your git email and name? > > Thanks > >> On Fri, Apr 17, 2020, at 13:07, Ross Nicholson wrote: >> Ping >> >>>> On 15 Apr 2020, at 17:21, Ross Nicholson <phunkyfish@gmail.com> wrote: >>> >>> >>> Ping to hopefully apply this patch! >>> >>>>> On 12 Apr 2020, at 16:54, Ross Nicholson <phunkyfish@gmail.com> wrote: >>>>> >>>> >>>> User testing has been completed successfully so this is ready to be applied. >>>> >>>> Thanks >>>> >>>>>> On 7 Apr 2020, at 23:50, Ross Nicholson <phunkyfish@gmail.com> wrote: >>>>>> >>>>> >>>>> Thank you for the explanation Marton. It's make perfect sense to me know. So UNLIMITED would be the right choice here. >>>>> >>>>> All of your other comments are addressed in the latest version. Thanks again for reviewing. >>>>> >>>>>> On Tue, 7 Apr 2020 at 22:03, Marton Balint <cus@passwd.hu> wrote: >>>>>> >>>>>> >>>>>> On Tue, 7 Apr 2020, Ross Nicholson wrote: >>>>>> >>>>>>> Great, thanks again. >>>>>>> >>>>>>> A question about AV_BPRINT_SIZE_AUTOMATIC. Is there a heuristic for when to use this versus unlimited? >>>>>>> >>>>>>> Or is it that generally if you would have used a buffer of 1000 or less automatic is the right choice? >>>>>> >>>>>> It depends on what you want. With AUTOMATIC you limit length to 1000 chars >>>>>> but you don't have to free the buffer. Otherwise you are not limiting the >>>>>> buffer size, but you have to free it. So if it is impossible to hit the >>>>>> limit, you should always use AUTOMATIC. >>>>>> >>>>>> In this case you have to decide for yourself which to use, because I don't >>>>>> know if 1000 char buffer is big enough for the possible use cases. I >>>>>> only suggested to consider it, because you are using limited buffers for >>>>>> other strings, so it may not even be possible to outgrow 1000 chars. It is >>>>>> up to you decide depending on what you want to support. >>>>>> >>>>>> Regards, >>>>>> Marton >>>>>> >>>>>>> >>>>>>>> On 7 Apr 2020, at 20:50, Marton Balint <cus@passwd.hu> wrote: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>>> On Tue, 7 Apr 2020, phunkyfish wrote: >>>>>>>>> >>>>>>>>> --- >>>>>>>>> libavformat/rtsp.c | 48 +++++++++++++++++++++++++++++++++++++--------- >>>>>>>>> 1 file changed, 39 insertions(+), 9 deletions(-) >>>>>>>>> >>>>>>>>> diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c >>>>>>>>> index cd6fc32a29..dad3f7915e 100644 >>>>>>>>> --- a/libavformat/rtsp.c >>>>>>>>> +++ b/libavformat/rtsp.c >>>>>>>>> @@ -21,6 +21,7 @@ >>>>>>>>> #include "libavutil/avassert.h" >>>>>>>>> #include "libavutil/base64.h" >>>>>>>>> +#include "libavutil/bprint.h" >>>>>>>>> #include "libavutil/avstring.h" >>>>>>>>> #include "libavutil/intreadwrite.h" >>>>>>>>> #include "libavutil/mathematics.h" >>>>>>>>> @@ -2447,7 +2448,7 @@ 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]; >>>>>>>>> + char host[500], filters_buf[1000]; >>>>>>>>> int ret, port; >>>>>>>>> URLContext* in = NULL; >>>>>>>>> int payload_type; >>>>>>>>> @@ -2456,6 +2457,8 @@ static int rtp_read_header(AVFormatContext *s) >>>>>>>>> AVIOContext pb; >>>>>>>>> socklen_t addrlen = sizeof(addr); >>>>>>>>> RTSPState *rt = s->priv_data; >>>>>>>>> + const char *p; >>>>>>>>> + AVBPrint sdp; >>>>>>>>> >>>>>>>>> if (!ff_network_init()) >>>>>>>>> return AVERROR(EIO); >>>>>>>>> @@ -2513,16 +2516,38 @@ 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); >>>>>>>>> - av_log(s, AV_LOG_VERBOSE, "SDP:\n%s\n", sdp); >>>>>>>>> + av_bprint_init(&sdp, 0, AV_BPRINT_SIZE_UNLIMITED); >>>>>>>> >>>>>>>> You may also use AV_BPRINT_SIZE_AUTOMATIC in which case you will get a static buffer which will be limited (roughly 1000 chars) but you don't have to free it with av_bprint_finalize(). >>>>>>>> >>>>>>>>> + av_bprintf(&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 = ' '; >>>>>>>>> + av_bprintf(&sdp, "a=source-filter:%s IN IP%d %s %s\r\n", >>>>>>>>> + filters[i][1], >>>>>>>>> + addr.ss_family == AF_INET ? 4 : 6, host, >>>>>>>>> + filters_buf); >>>>>>>>> + } >>>>>>>>> + } >>>>>>>>> + } >>>>>>>>> + >>>>>>>>> + av_bprintf(&sdp, "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.str); >>>>>>>>> + if (av_bprint_is_complete(&sdp)) >>>>>>>> >>>>>>>> I think this check should be negated here, because you want to report error if the buffer is truncated, not if it is complete. >>>>>>>> >>>>>>>>> + goto fail_nobuf; >>>>>>>>> avcodec_parameters_free(&par); >>>>>>>>> - ffio_init_context(&pb, sdp, strlen(sdp), 0, NULL, NULL, NULL, NULL); >>>>>>>>> + ffio_init_context(&pb, sdp.str, strlen(sdp.str), 0, NULL, NULL, NULL, NULL); >>>>>>>> >>>>>>>> You can use sdp.len instead of strlen(). >>>>>>>> >>>>>>>>> s->pb = &pb; >>>>>>>>> >>>>>>>>> /* sdp_read_header initializes this again */ >>>>>>>>> @@ -2532,9 +2557,14 @@ static int rtp_read_header(AVFormatContext *s) >>>>>>>>> >>>>>>>>> ret = sdp_read_header(s); >>>>>>>>> s->pb = NULL; >>>>>>>>> + av_bprint_finalize(&sdp, NULL); >>>>>>>>> return ret; >>>>>>>>> +fail_nobuf: >>>>>>>>> + ret = AVERROR(ENOMEM); >>>>>>>>> + av_log(s, AV_LOG_ERROR, "rtp_read_header(): not enough buffer space for sdp-headers\n"); >>>>>>>>> fail: >>>>>>>>> + av_bprint_finalize(&sdp, NULL); >>>>>>>>> avcodec_parameters_free(&par); >>>>>>>>> if (in) >>>>>>>>> ffurl_close(in); >>>>>>>> >>>>>>>> Regards, >>>>>>>> Marton >>>>>>>> _______________________________________________ >>>>>>>> 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". >>>>>> _______________________________________________ >>>>>> 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". > > -- > Jean-Baptiste Kempf - President > +33 672 704 734
Am So., 19. Apr. 2020 um 17:33 Uhr schrieb Ross Nicholson <phunkyfish@gmail.com>: > > How do you mean? What’s that problem? > > I generally just use phunkyfish as the name and it’s corresponding gmail address for stuff. > > Do you require my real name for git? No, absolutely not. Carl Eugen
On Sun, Apr 19, 2020, at 16:28, Ross Nicholson wrote:
> How do you mean? What’s that problem?
You've used Ross Nicholson <phunkyfish@gmail.com> for committing, and now you only use
phunkyfish <phunkyfish@gmail.com>.
Since you are not anonymous, why not use your actual name?
That’s really strange all my git config’s are just phunkyfish. I don’t really mind which name is used but I guess I’ll update everything to my real name going forward now that you mention it. > On 19 Apr 2020, at 18:55, Jean-Baptiste Kempf <jb@videolan.org> wrote: > > On Sun, Apr 19, 2020, at 16:28, Ross Nicholson wrote: >> How do you mean? What’s that problem? > > You've used Ross Nicholson <phunkyfish@gmail.com> for committing, and now you only use > phunkyfish <phunkyfish@gmail.com>. > > Since you are not anonymous, why not use your actual name? > > > -- > Jean-Baptiste Kempf - President > +33 672 704 734 > _______________________________________________ > 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".
Am So., 19. Apr. 2020 um 20:45 Uhr schrieb Ross Nicholson <phunkyfish@gmail.com>: > > That’s really strange all my git config’s are just phunkyfish. > > I don’t really mind which name is used but I guess I’ll update > everything to my real name going forward now that you mention it. It is completely your decision which "name" you choose for your commits (invented or real) but please avoid top-posting on all FFmpeg mailing lists. Carl Eugen
Am So., 19. Apr. 2020 um 19:55 Uhr schrieb Jean-Baptiste Kempf <jb@videolan.org>: > > On Sun, Apr 19, 2020, at 16:28, Ross Nicholson wrote: > > How do you mean? What’s that problem? > > You've used Ross Nicholson <phunkyfish@gmail.com> for committing It looks to me as if he has not done that. Carl Eugen
> On 19 Apr 2020, at 20:30, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > Am So., 19. Apr. 2020 um 20:45 Uhr schrieb Ross Nicholson > <phunkyfish@gmail.com>: >> >> That’s really strange all my git config’s are just phunkyfish. >> >> I don’t really mind which name is used but I guess I’ll update >> everything to my real name going forward now that you mention it. > > It is completely your decision which "name" you choose for > your commits (invented or real) but please avoid top-posting > on all FFmpeg mailing lists. > > > Carl Eugen > _______________________________________________ > 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". Apologies Carl. I only just learned what top-posting is. Won’t happen again.
diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c index cd6fc32a29..dad3f7915e 100644 --- a/libavformat/rtsp.c +++ b/libavformat/rtsp.c @@ -21,6 +21,7 @@ #include "libavutil/avassert.h" #include "libavutil/base64.h" +#include "libavutil/bprint.h" #include "libavutil/avstring.h" #include "libavutil/intreadwrite.h" #include "libavutil/mathematics.h" @@ -2447,7 +2448,7 @@ 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]; + char host[500], filters_buf[1000]; int ret, port; URLContext* in = NULL; int payload_type; @@ -2456,6 +2457,8 @@ static int rtp_read_header(AVFormatContext *s) AVIOContext pb; socklen_t addrlen = sizeof(addr); RTSPState *rt = s->priv_data; + const char *p; + AVBPrint sdp; if (!ff_network_init()) return AVERROR(EIO); @@ -2513,16 +2516,38 @@ 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); - av_log(s, AV_LOG_VERBOSE, "SDP:\n%s\n", sdp); + av_bprint_init(&sdp, 0, AV_BPRINT_SIZE_UNLIMITED); + av_bprintf(&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 = ' '; + av_bprintf(&sdp, "a=source-filter:%s IN IP%d %s %s\r\n", + filters[i][1], + addr.ss_family == AF_INET ? 4 : 6, host, + filters_buf); + } + } + } + + av_bprintf(&sdp, "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.str); + if (av_bprint_is_complete(&sdp)) + goto fail_nobuf; avcodec_parameters_free(&par); - ffio_init_context(&pb, sdp, strlen(sdp), 0, NULL, NULL, NULL, NULL); + ffio_init_context(&pb, sdp.str, strlen(sdp.str), 0, NULL, NULL, NULL, NULL); s->pb = &pb; /* sdp_read_header initializes this again */ @@ -2532,9 +2557,14 @@ static int rtp_read_header(AVFormatContext *s) ret = sdp_read_header(s); s->pb = NULL; + av_bprint_finalize(&sdp, NULL); return ret; +fail_nobuf: + ret = AVERROR(ENOMEM); + av_log(s, AV_LOG_ERROR, "rtp_read_header(): not enough buffer space for sdp-headers\n"); fail: + av_bprint_finalize(&sdp, NULL); avcodec_parameters_free(&par); if (in) ffurl_close(in);