[FFmpeg-devel] [PATCH] avformat/libsrt: add several options supported in srt 1.3.0

[Marton Balint]

On Fri, 19 Oct 2018, Matsuzawa Tomohiro wrote:

> Several SRT options are missing. Since pkg_config requires libsrt v1.3.0 and above, it should be able to support options added in libsrt v1.3.0 and below.
> This commit adds 8 SRT options.
> sndbuf, rcvbuf, lossmaxttl, minversion, streamid, smoother, messageapi and transtype
> The keys of option are equivalent to stransmit.
> https://github.com/Haivision/srt/blob/v1.3.0/apps/socketoptions.hpp#L196-L223
> ---
> doc/protocols.texi   | 85 ++++++++++++++++++++++++++++++++++++++++++--
> libavformat/libsrt.c | 56 +++++++++++++++++++++++++++++
> 2 files changed, 139 insertions(+), 2 deletions(-)

[...]

> + at item minversion
> +The minimum SRT version that is required from the peer. A connection
> +to a peer that does not satisfy the minimum version requirement
> +will be rejected.
> +
> +The version format in hex is 0xXXYYZZ for x.y.z in human readable
> +form, where x = ("%d", (version>>16) & 0xff), etc.

The , "where x = ..." part is super confusing. I'd just delete it, the 
first part of the sentence is clear enough.

[...]


> @@ -110,6 +118,16 @@ static const AVOption libsrt_options[] = {
>     { "caller",         NULL, 0, AV_OPT_TYPE_CONST,  { .i64 = SRT_MODE_CALLER },     INT_MIN, INT_MAX, .flags = D|E, "mode" },
>     { "listener",       NULL, 0, AV_OPT_TYPE_CONST,  { .i64 = SRT_MODE_LISTENER },   INT_MIN, INT_MAX, .flags = D|E, "mode" },
>     { "rendezvous",     NULL, 0, AV_OPT_TYPE_CONST,  { .i64 = SRT_MODE_RENDEZVOUS }, INT_MIN, INT_MAX, .flags = D|E, "mode" },
> +    { "sndbuf",         "Send buffer size (in bytes)",                                          OFFSET(sndbuf),           AV_OPT_TYPE_INT,      { .i64 = -1 }, -1, INT_MAX,   .flags = D|E },
> +    { "rcvbuf",         "Receive buffer size (in bytes)",                                       OFFSET(rcvbuf),           AV_OPT_TYPE_INT,      { .i64 = -1 }, -1, INT_MAX,   .flags = D|E },
> +    { "lossmaxttl",     "Maximum possible packet reorder tolerance",                            OFFSET(lossmaxttl),       AV_OPT_TYPE_INT,      { .i64 = -1 }, -1, INT_MAX,   .flags = D|E },
> +    { "minversion",     "The minimum SRT version that is required from the peer",               OFFSET(minversion),       AV_OPT_TYPE_INT,      { .i64 = -1 }, -1, INT_MAX,   .flags = D|E },
> +    { "streamid",       "A string of up to 512 characters that an Initiator can pass to a Responder",  OFFSET(streamid),  AV_OPT_TYPE_STRING,   { .str = NULL },              .flags = D|E },
> +    { "smoother",       "The type of Smoother used for the transmission for that socket",       OFFSET(smoother),         AV_OPT_TYPE_STRING,   { .str = NULL },              .flags = D|E },
> +    { "messageapi",     "Enable message API",                                                   OFFSET(messageapi),       AV_OPT_TYPE_INT,      { .i64 = -1 }, -1, 1,         .flags = D|E },
> +    { "transtype",      "The transmission type for the socket",                                 OFFSET(transtype),        AV_OPT_TYPE_INT,      { .i64 = SRTT_INVALID }, SRTT_LIVE, SRTT_INVALID, .flags = D|E, "transtype" },
> +    { "live",           NULL, 0, AV_OPT_TYPE_CONST,  { .i64 = SRTT_LIVE }, INT_MIN, INT_MAX, .flags = D|E, "transtype" },
> +    { "file",           NULL, 0, AV_OPT_TYPE_CONST,  { .i64 = SRTT_FILE }, INT_MIN, INT_MAX, .flags = D|E, "transtype" },
>     { NULL }
> };
>
> @@ -310,6 +328,14 @@ static int libsrt_set_options_pre(URLContext *h, int fd)
>         (s->tlpktdrop >= 0 && libsrt_setsockopt(h, fd, SRTO_TLPKTDROP, "SRTO_TLPKDROP", &s->tlpktdrop, sizeof(s->tlpktdrop)) < 0) ||
>         (s->nakreport >= 0 && libsrt_setsockopt(h, fd, SRTO_NAKREPORT, "SRTO_NAKREPORT", &s->nakreport, sizeof(s->nakreport)) < 0) ||
>         (connect_timeout >= 0 && libsrt_setsockopt(h, fd, SRTO_CONNTIMEO, "SRTO_CONNTIMEO", &connect_timeout, sizeof(connect_timeout)) <0 ) ||
> +        (s->sndbuf >= 0 && libsrt_setsockopt(h, fd, SRTO_SNDBUF, "SRTO_SNDBUF", &s->sndbuf, sizeof(s->sndbuf)) < 0) ||
> +        (s->rcvbuf >= 0 && libsrt_setsockopt(h, fd, SRTO_RCVBUF, "SRTO_RCVBUF", &s->rcvbuf, sizeof(s->rcvbuf)) < 0) ||
> +        (s->lossmaxttl >= 0 && libsrt_setsockopt(h, fd, SRTO_LOSSMAXTTL, "SRTO_LOSSMAXTTL", &s->lossmaxttl, sizeof(s->lossmaxttl)) < 0) ||
> +        (s->minversion >= 0 && libsrt_setsockopt(h, fd, SRTO_MINVERSION, "SRTO_MINVERSION", &s->minversion, sizeof(s->minversion)) < 0) ||
> +        (s->streamid && libsrt_setsockopt(h, fd, SRTO_STREAMID, "SRTO_STREAMID", s->streamid, strlen(s->streamid)) < 0) ||
> +        (s->smoother && libsrt_setsockopt(h, fd, SRTO_SMOOTHER, "SRTO_SMOOTHER", s->smoother, strlen(s->smoother)) < 0) ||
> +        (s->messageapi >= 0 && libsrt_setsockopt(h, fd, SRTO_MESSAGEAPI, "SRTO_MESSAGEAPI", &s->messageapi, sizeof(s->messageapi)) < 0) ||
> +        (s->transtype != SRTT_INVALID && libsrt_setsockopt(h, fd, SRTO_TRANSTYPE, "SRTO_TRANSTYPE", &s->transtype, sizeof(s->transtype)) < 0) ||

According to the docs setting transtype sets lots of other options, so 
probably transtype should be set first, so the user can override these 
options.

>                 return AVERROR(EIO);
>             }
>         }
> +        if (av_find_info_tag(buf, sizeof(buf), "sndbuf", p)) {
> +            s->sndbuf = strtol(buf, NULL, 10);
> +        }
> +        if (av_find_info_tag(buf, sizeof(buf), "rcvbuf", p)) {
> +            s->rcvbuf = strtol(buf, NULL, 10);
> +        }
> +        if (av_find_info_tag(buf, sizeof(buf), "lossmaxttl", p)) {
> +            s->lossmaxttl = strtol(buf, NULL, 10);
> +        }
> +        if (av_find_info_tag(buf, sizeof(buf), "minversion", p)) {
> +            s->minversion = strtol(buf, NULL, 0);
> +        }
> +        if (av_find_info_tag(buf, sizeof(buf), "streamid", p)) {
> +            s->streamid = av_strndup(buf, strlen(buf));
> +        }
> +        if (av_find_info_tag(buf, sizeof(buf), "smoother", p)) {
> +            s->smoother = av_strndup(buf, strlen(buf));
> +        }

Probably you have to free the existing value of the strings with 
av_freep when overriding them. Also I see no reason to use av_strndup, 
simply use av_strdup(buf).

> +            s->messageapi = strtol(buf, NULL, 10);
> +        }
> +        if (av_find_info_tag(buf, sizeof(buf), "transtype", p)) {
> +            if (!strcmp(buf, "live")) {
> +                s->transtype = SRTT_LIVE;
> +            } else if (!strcmp(buf, "file")) {
> +                s->transtype = SRTT_FILE;
> +            } else {
> +                return AVERROR(EIO);

AVERROR(EINVAL). Yes, I know, AVERROR(EIO) is used everywhere else, that 
does not make it right :)

> +            }
> +        }
>     }
>     return libsrt_setup(h, uri, flags);

Thanks,
Marton