[FFmpeg-devel] [PATCH] avformat/rtp: Pass sources and block filter addresses via sdp file for rtp
Jean-Baptiste Kempf
jb at videolan.org
Sun Apr 19 16:22:56 EEST 2020
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 at gmail.com> wrote:
> >
> >
> > Ping to hopefully apply this patch!
> >
> >>> On 12 Apr 2020, at 16:54, Ross Nicholson <phunkyfish at 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 at 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 at 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 at 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 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".
> >>>> _______________________________________________
> >>>> 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".
--
Jean-Baptiste Kempf - President
+33 672 704 734
More information about the ffmpeg-devel
mailing list