On 01/06/2018 19:42, Luca Barbato wrote:
> On 30/05/2018 11:16, Sven Dueking wrote:
>>
>>
>>> -----Ursprüngliche Nachricht-----
>>> Von: libav-devel [mailto:[email protected]] Im Auftrag von
>>> Diego Biurrun
>>> Gesendet: Dienstag, 29. Mai 2018 15:33
>>> An: libav development
>>> Betreff: Re: [libav-devel] [libva-devel] [PATCH] avformat/libsrt: add
>>> payload size and latency option / deprecate ts
>>>
>>>> From 47e1d01b08494d5745d35f7a701059230c78671a Mon Sep 17 00:00:00
>>> 2001
>>>> From: Nablet Developer <[email protected]>
>>>
>>> Somebody still needs to set up their Git? :)
>>>
>>>> Date: Mon, 21 May 2018 13:55:25 +0700
>>>> Subject: [PATCH 1/2] avformat/libsrt: add payload size option
>>>>
>>>> Signed-off-by: Nablet Developer <[email protected]>
>>>> ---
>>>> doc/protocols.texi | 10 ++++++++++
>>>> libavformat/libsrt.c | 19 ++++++++++++++++++-
>>>> 2 files changed, 28 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/doc/protocols.texi b/doc/protocols.texi index
>>>> e2d06a067..247734cd8 100644
>>>> --- a/doc/protocols.texi
>>>> +++ b/doc/protocols.texi
>>>> @@ -755,6 +755,16 @@ 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 recovered (write-only).
>>>>
>>>> +@item payloadsize=@var{bytes}
>>>> +Sets the maximum declared size of a single call to sending function
>>>> +in Live mode.
>>>
>>> Apart from - I think - a missing "the" in "to the sending" this
>>> sentence confuses me. What is the size of a function call? Or is it
>>> something else that size refers to?
>>>
>>>> +Default value is for MPEG TS; if you are going to use SRT
>>>
>>> MPEG-TS
>>>
>>>> --- a/libavformat/libsrt.c
>>>> +++ b/libavformat/libsrt.c
>>>> @@ -34,6 +34,16 @@
>>>>
>>>> +/* This is for MPEG TS and it's a default SRTO_PAYLOADSIZE for
>>>> +SRTT_LIVE (8 TS packets) */
>>>
>>> same
>>>
>>>> +/* This is the maximum payload size for Live mode, should you have a
>>>> +different payload type than MPEG TS */
>>>
>>> same
>>>
>>>> @@ -86,6 +97,7 @@ static const AVOption libsrt_options[] = {
>>>> + { "payload size", "maximum declared size of a single call to
>>> sending function", OFFSET(payload_size), AV_OPT_TYPE_INT,
>>> { .i64 = SRT_LIVE_DEFAULT_PAYLOAD_SIZE }, -1,
>>> SRT_LIVE_MAX_PAYLOAD_SIZE, .flags = D|E },
>>>
>>> see above
>>>
>>>> @@ -276,7 +288,8 @@ static int libsrt_set_options_pre(URLContext *h,
>>> int fd)
>>>> (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 )) {
>>>> + (connect_timeout >= 0 && libsrt_setsockopt(h, fd,
>>> SRTO_CONNTIMEO, "SRTO_CONNTIMEO", &connect_timeout,
>>> sizeof(connect_timeout)) <0 ) ||
>>>> + (s->payload_size >= 0 && libsrt_setsockopt(h, fd,
>>>> + SRTO_PAYLOADSIZE, "SRTO_PAYLOADSIZE", &s->payload_size,
>>>> + sizeof(s->payload_size)) <0 )) {
>>>
>>> Add a space after '<' please.
>>>
>>>> @@ -454,6 +467,9 @@ static int libsrt_open(URLContext *h, const char
>>> *uri, int flags)
>>>> }
>>>> + if (av_find_info_tag(buf, sizeof(buf), "payload_size", p)) {
>>>> + s->payload_size = strtol(buf, NULL, 10);
>>>> + }
>>>
>>> stray tabs
>>>
>>>> @@ -466,6 +482,7 @@ static int libsrt_open(URLContext *h, const char
>>> *uri, int flags)
>>>> }
>>>> }
>>>> }
>>>> + h->max_packet_size = s->payload_size > 0 ? s->payload_size
>>>> + :SRT_LIVE_DEFAULT_PAYLOAD_SIZE;
>>>
>>> odd spacing around :
>>>
>>>> From af93164c05eeb62c37c21cc7a9a3cd43c6c0c4a7 Mon Sep 17 00:00:00
>>> 2001
>>>> From: Nablet Developer <[email protected]>
>>>
>>> odd developer name
>>>
>>>> --- a/doc/protocols.texi
>>>> +++ b/doc/protocols.texi
>>>> @@ -710,6 +710,17 @@ IP Type of Service. Applies to sender only.
>>> Default value is 0xB8.
>>>>
>>>> +@item latency
>>>> +Timestamp-based Packet Delivery Delay.
>>>> +Used to absorb burst of missed packet retransmission.
>>>
>>> burstS, retransmissionS
>>>
>>>> +This flag sets both @option{rcvlatency} and @option{peerlatency} to
>>>> +the same value. Note that prior to version 1.3.0 this is the only
>>>> +flag to set the latency, however this is effectively equivalent to
>>>> +setting @option{peerlatency}, when the side is sender and
>>>> +@option{rcvlatency} when the side is receiver, and the bidirectional
>>>> +stream sending is not supported.
>>>
>>> "the side"?
>>>
>>>> +
>>>> @item pbkeylen=@var{bytes}
>>>> Sender encryption key length, in bytes.
>>>> Only can be set to 0, 16, 24 and 32.
>>>> @@ -773,6 +788,18 @@ Not required on receiver (set to 0), key size
>>>> obtained from sender in HaiCrypt handshake.
>>>> Default value is 0.
>>>>
>>>> +@item rcvlatency
>>>> +The time that should elapse since the moment when the packet was
>>> sent
>>>> +and the moment when it's delivered to the receiver application in
>>> the
>>>> +receiving function.
>>>> +This time should be a buffer time large enough to cover the time
>>>> +spent for sending, unexpectedly extended RTT time, and the time
>>>> +needed to retransmit the lost UDP packet. The effective latency
>>> value
>>>> +will be the maximum of this options' value and the value of
>>>> +@option{perrlatency}
>>>
>>> pe_E_rlatency
>>>
>>>> +set by the peer side. This option in pre-1.3.0 version is available
>>>> +only as @option{latency}.
>>>
>>> Before version 1.3.0 this option is only available as ..
>>>
>>>> --- a/libavformat/libsrt.c
>>>> +++ b/libavformat/libsrt.c
>>>> @@ -93,7 +95,9 @@ static const AVOption libsrt_options[] = {
>>>> { "oheadbw", "MaxBW ceiling based on % over input stream
>>> rate", OFFSET(oheadbw), AV_OPT_TYPE_INT,
>>> { .i64 = -1 }, -1, 100, .flags = D|E },
>>>> - { "tsbpddelay", "TsbPd receiver delay to absorb burst of
>>> missed packet retransmission", OFFSET(tsbpddelay),
>>> AV_OPT_TYPE_INT64, { .i64 = -1 }, -1, INT64_MAX, .flags = D|E },
>>>> + { "latency", "TsbPd receiver delay to absorb burst of
>>> missed packet retransmission", OFFSET(latency),
>>> AV_OPT_TYPE_INT64, { .i64 = -1 }, -1, INT64_MAX, .flags = D|E },
>>>
>>> tsbpd?
>>>
>>> see above about missing plural 's'
>>>
>>> Diego
>>
>> Diego and Lu, thanks for the review. Attached a new patchset.
>>
>
> I'd merge it during the weekend. Thanks to you :)
/Users/lu_zero/Sources/libav/libavformat/libsrt.c:298:57: error: use of
undeclared identifier 'SRTO_RCVLATENCY'; did you mean 'SRTO_LATENCY'?
(s->rcvlatency >= 0 && libsrt_setsockopt(h, fd, SRTO_RCVLATENCY,
"SRTO_RCVLATENCY", &rcvlatency, sizeof(rcvlatency)) < 0) ||
^~~~~~~~~~~~~~~
SRTO_LATENCY
/usr/local/include/srt/srt.h:81:2: note: 'SRTO_LATENCY' declared here
CC libavformat/mm.o
SRTO_LATENCY = 23, // ALIAS: SRTO_TSBPDDELAY
^
CC libavformat/mmf.o
/Users/lu_zero/Sources/libav/libavformat/libsrt.c:299:58: error: use of
undeclared identifier 'SRTO_PEERLATENCY'; did you mean 'SRTO_LATENCY'?
(s->peerlatency >= 0 && libsrt_setsockopt(h, fd,
SRTO_PEERLATENCY, "SRTO_PEERLATENCY", &peerlatency, sizeof(peerlatency))
< 0) ||
^~~~~~~~~~~~~~~~
SRTO_LATENCY
/usr/local/include/srt/srt.h:81:2: note: 'SRTO_LATENCY' declared here
SRTO_LATENCY = 23, // ALIAS: SRTO_TSBPDDELAY
^
CC libavformat/mms.o
CC libavformat/mmsh.o
/Users/lu_zero/Sources/libav/libavformat/libsrt.c:303:59: error: use of
undeclared identifier 'SRTO_PAYLOADSIZE'
(s->payload_size >= 0 && libsrt_setsockopt(h, fd,
SRTO_PAYLOADSIZE, "SRTO_PAYLOADSIZE", &s->payload_size,
sizeof(s->payload_size)) < 0)) {
Looks like we should update the library version requirement.
Could you please advise in this regard.
lu
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
