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".
