Re: [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(-)

[...]

+@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
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel