[FFmpeg-devel] [PATCH] avformat/rtp: Pass sources and block filter addresses via sdp file for rtp

[Marton Balint]

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