On Wed, Mar 21, 2018 at 08:41:30AM +0100, Luca Barbato wrote:
> From: Sven Dueking <[email protected]>
> 
> protocol requires libsrt (https://github.com/Haivision/srt) to be
> installed
> 
> Signed-off-by: Sven Dueking <[email protected]>
> Signed-off-by: Luca Barbato <[email protected]>
> ---
> 
> Here the patch with all the changes folded in, I'll push it tonight if
> nobody has further comments.

Did the new formats checklist get out of fashion lately?

https://www.libav.org/documentation/developer.html#New-codecs-or-formats-checklist

> --- a/configure
> +++ b/configure
> @@ -1371,6 +1372,7 @@ EXTERNAL_LIBRARY_LIST="
>      libspeex
> +    libsrt
>      libtheora
> @@ -2522,6 +2524,8 @@ librtmpt_protocol_deps="librtmp"
>  mmst_protocol_select="network"
> +opensrt_protocol_select="network"
> +opensrt_protocol_deps="libsrt"
>  rtmp_protocol_conflict="librtmp_protocol"

What is it? libsrt or opensrt?

> --- a/doc/protocols.texi
> +++ b/doc/protocols.texi
> @@ -655,6 +655,145 @@ To play back the first stream announced on one the 
> default IPv6 SAP multicast ad
>  avplay sap://[ff0e::2:7ffe]
>  @end example
> 
> +@section srt
> +
> +Haivision Secure Reliable Transport Protocol via libsrt.
> +
> +The supported syntax for a SRT url is:

URL

> +@example
> +srt://@var{hostname}:@var{port}[?@var{options}]
> +@end example
> +
> +@var{options} contains a list of &-separated options of the form

Do you have to escape '&'? I don't remember offhand.

> +
> +or
> +
> +@example
> +@var{options} srt://@var{hostname}:@var{port}
> +@end example
> +
> +@var{options} contains a list of '-@var{key} @var{val}'
> +options.
> +
> +This protocol accepts the following options.
> +
> +@table @option
> +@item connect_timeout
> +Connection timeout. SRT cannot connect for RTT > 1500 msec

Connection timeout;

> +@item fc=@var{bytes}
> +Flight Flag Size (Window Size), in bytes. FC is actually an

ffs? loool ;)

> +internal parameter and you should set it to not less than
> +@option{recv_buffer_size} and @option{mss}. The default value
> +is relatively large, therefore unless you set a very large receiver buffer,
> +you do not need to change this option. Default value is 25600.

This sounds like it should not be a user-tunable parameter, but that
the implementation should take care of it instead. Oh well ..

> +@item inputbw=@var{bytes/seconds}
> +Sender nominal input rate, in bytes per seconds. Used along with
> +@option{oheadbw}, when @option{maxbw} is set to relative (0), to
> +calculate maximum sending rate when recovery packets are sent
> +along with main media stream:

with the

> +@option{inputbw} * (100 + @option{oheadbw}) / 100
> +if @option{inputbw} is not set while @option{maxbw} is set to
> +relative (0), the actual ctual input rate is evaluated inside

ctual?

> +@item maxbw=@var{bytes/seconds}
> +Maximum sending bandwidth, in bytes per seconds.
> +-1 infinite (CSRTCC limit is 30mbps)
> +0 relative to input rate (see @option{inputbw})
> +>0 absolute limit value
> +Default value is 0 (relative)
> +
> +@item mode=@var{caller|listener|rendezvous}
> +Connection mode.
> +caller opens client connection.
> +listener starts server to listen for incoming connections.
> +rendezvous use Rendez-Vous connection mode.

The values should have @option markup.

> +Default valus is caller.

s/valus/value/

> +@item mss=@var{bytes}
> +Maximum Segment Size, in bytes. Used for buffer allocation
> +and rate calculation using packet counter assuming fully

a packet counter

> +filled packets. The smallest MSS between the peers is
> +used. This is 1500 by default in the overall internet.
> +This is the maximum size of the UDP packet and can be
> +only decreased, unless you have some unusual dedicated
> +network settings. Default value is 1500.
> +
> +@item nakreport=@var{1|0}
> +If set to 1, Receiver will send `UMSG_LOSSREPORT` messages
> +periodically until the lost packet is retransmitted or

s/the/a/ I assume

> +
> +@item oheadbw=@var{percents}
> +Recovery bandwidth overhead above input rate, in percents.
> +See @option{inputbw}. Default value is 25%.
> +
> +@item passphrase=@var{string}
> +HaiCrypt Encryption/Decryption Passphrase string, length
> +from 10 to 79 characters. The passphrase is the shared
> +secret between the sender and the receiver. It is used
> +to generate the Key Encrypting Key using PBKDF2
> +(Password-Based Key Deriviation Function). It is used

Deriv_ation

> +only if @option{pbkeylen} is non-zero. It is used on
> +the receiver only if the received data is encrypted.
> +The configured passphrase cannot be get back (write-only).

s/get back/recovered/

> +@item recv_buffer_size=@var{bytes}
> +Set receive buffer size, expressed bytes.
> +
> +@item send_buffer_size=@var{bytes}
> +Set send buffer size, expressed bytes.

expressed in

> +@item rw_timeout
> +Set raise error timeout.

rw?

> +This option is only relevant in read mode:
> +if no data arrived in more than this time
> +interval, raise error.
> +
> +@item tlpktdrop=@var{1|0}
> +Too-late Packet Drop. When enabled on receiver, it skips
> +missing packets that have not been delivered in time and
> +deliver the following packets to the application when

deliverS

> +their time-to-play has come. It also send a fake ACK to

sendS

> +sender. When enabled on sender and enabled on the

the sender

> +receiving peer, sender drops the older packets that

the sender

> +have no chance to be delivered in time. It was

of being delivered

> +automatically enabled in sender if receiver supports it.

the sender, the receiver

> +@item tsbpddelay
> +Timestamp-based Packet Delivery Delay.
> +Used to absorb burst of missed packet retransmission.

I'm generally sceptical of all those options, are they really necessary?
Also, the names look like line noise.

> --- /dev/null
> +++ b/libavformat/opensrt.c
> @@ -0,0 +1,575 @@
> +
> +#include "avformat.h"
> +#include "libavutil/avassert.h"
> +#include "libavutil/parseutils.h"
> +#include "libavutil/opt.h"
> +#include "libavutil/time.h"
> +
> +#include "internal.h"
> +#include "network.h"
> +#include "os_support.h"
> +#include "url.h"
> +#if HAVE_POLL_H
> +#include <poll.h>
> +#endif
> +
> +#include <srt/srt.h>

nit: canonical order

> +static const AVClass opensrt_class = {
> +    .class_name = "opensrt",
> +    .item_name  = av_default_item_name,
> +    .option     = opensrt_options,
> +    .version    = LIBAVUTIL_VERSION_INT,
> +};

This would make more sense close to where it is used.

> +static int opensrt_socket_nonblock(int socket, int enable)
> +{
> +    int ret = srt_setsockopt(socket, 0, SRTO_SNDSYN, &enable, 
> sizeof(enable));
> +    if (ret < 0)
> +        return ret;
> +    ret = srt_setsockopt(socket, 0, SRTO_RCVSYN, &enable, sizeof(enable));
> +    return ret;

nit: return directly

> +static int opensrt_set_options_post(URLContext *h, int fd)
> +{
> +    if (s->inputbw >= 0 && opensrt_setsockopt(h, fd, SRTO_INPUTBW, 
> "SRTO_INPUTBW", &s->inputbw, sizeof(s->inputbw)) < 0) {
> +        return AVERROR(EIO);
> +    }
> +    if (s->oheadbw >= 0 && opensrt_setsockopt(h, fd, SRTO_OHEADBW, 
> "SRTO_OHEADBW", &s->oheadbw, sizeof(s->oheadbw)) < 0) {
> +        return AVERROR(EIO);
> +    }
> +    return 0;
> +}
> +
> +static int opensrt_set_options_pre(URLContext *h, int fd)
> +{
> +    if (s->mode == SRT_MODE_RENDEZVOUS && opensrt_setsockopt(h, fd, 
> SRTO_RENDEZVOUS, "SRTO_RENDEZVOUS", &yes, sizeof(yes)) < 0) {
> +        return AVERROR(EIO);
> +    }
> +    if (s->maxbw >= 0 && opensrt_setsockopt(h, fd, SRTO_MAXBW, "SRTO_MAXBW", 
> &s->maxbw, sizeof(s->maxbw)) < 0) {
> +        return AVERROR(EIO);
> +    }
> +    if (s->pbkeylen >= 0 && opensrt_setsockopt(h, fd, SRTO_PBKEYLEN, 
> "SRTO_PBKEYLEN", &s->pbkeylen, sizeof(s->pbkeylen)) < 0) {
> +        return AVERROR(EIO);
> +    }
> +    if (s->passphrase && opensrt_setsockopt(h, fd, SRTO_PASSPHRASE, 
> "SRTO_PASSPHRASE", &s->passphrase, sizeof(s->passphrase)) < 0) {
> +        return AVERROR(EIO);
> +    }
> +    if (s->mss >= 0 && opensrt_setsockopt(h, fd, SRTO_MSS, "SRTO_MMS", 
> &s->mss, sizeof(s->mss)) < 0) {
> +        return AVERROR(EIO);
> +    }
> +    if (s->fc >= 0 && opensrt_setsockopt(h, fd, SRTO_FC, "SRTO_FC", &s->fc, 
> sizeof(s->fc)) < 0) {
> +        return AVERROR(EIO);
> +    }
> +    if (s->ipttl >= 0 && opensrt_setsockopt(h, fd, SRTO_IPTTL, "SRTO_UPTTL", 
> &s->ipttl, sizeof(s->ipttl)) < 0) {
> +        return AVERROR(EIO);
> +    }
> +    if (s->iptos >= 0 && opensrt_setsockopt(h, fd, SRTO_IPTOS, "SRTO_IPTOS", 
> &s->iptos, sizeof(s->iptos)) < 0) {
> +        return AVERROR(EIO);
> +    }
> +    if (tsbpddelay >= 0 && opensrt_setsockopt(h, fd, SRTO_TSBPDDELAY, 
> "SRTO_TSBPDELAY", &tsbpddelay, sizeof(tsbpddelay)) < 0) {
> +        return AVERROR(EIO);
> +    }
> +    if (s->tlpktdrop >= 0 && opensrt_setsockopt(h, fd, SRTO_TLPKTDROP, 
> "SRTO_TLPKDROP", &s->tlpktdrop, sizeof(s->tlpktdrop)) < 0) {
> +        return AVERROR(EIO);
> +    }
> +    if (s->nakreport >= 0 && opensrt_setsockopt(h, fd, SRTO_NAKREPORT, 
> "SRTO_NAKREPORT", &s->nakreport, sizeof(s->nakreport)) < 0) {
> +        return AVERROR(EIO);
> +    }
> +    if (connect_timeout >= 0 && opensrt_setsockopt(h, fd, SRTO_CONNTIMEO, 
> "SRTO_CONNTIMEO", &connect_timeout, sizeof(connect_timeout)) < 0) {
> +        return AVERROR(EIO);
> +    }
> +    return 0;
> +}

Connecting all these through || would be more readable.

> +static int opensrt_open(URLContext *h, const char *uri, int flags)
> +    p = strchr(uri, '?');
> +    if (p)
> +    {

style

> +const URLProtocol ff_opensrt_protocol = {
> +    .name                = "srt",
> +    .url_open            = opensrt_open,
> +    .url_read            = opensrt_read,
> +    .url_write           = opensrt_write,
> +    .url_close           = opensrt_close,
> +    .url_get_file_handle = opensrt_get_file_handle,
> +    .priv_data_size      = sizeof(SRTContext),
> +    .flags               = URL_PROTOCOL_FLAG_NETWORK,
> +    .priv_data_class     = &opensrt_class,
> +};

The names mismatch between srt, opensrt, and libsrt.

Where does the opensrt name come from?

> --- a/libavformat/protocols.c
> +++ b/libavformat/protocols.c
> @@ -38,6 +38,7 @@ extern const URLProtocol ff_mmsh_protocol;
>  extern const URLProtocol ff_mmst_protocol;
>  extern const URLProtocol ff_md5_protocol;
>  extern const URLProtocol ff_pipe_protocol;
> +extern const URLProtocol ff_opensrt_protocol;
>  extern const URLProtocol ff_rtmp_protocol;
>  extern const URLProtocol ff_rtmpe_protocol;
>  extern const URLProtocol ff_rtmps_protocol;

order

Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to