Ping > On 15 Apr 2020, at 17:21, Ross Nicholson <phunkyf...@gmail.com> wrote: > > > Ping to hopefully apply this patch! > >>> On 12 Apr 2020, at 16:54, Ross Nicholson <phunkyf...@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 <phunkyf...@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 <c...@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 <c...@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-requ...@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-requ...@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-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".