Added modified API.md and new APISocketOptions.md #1510

stevomatthews · 2020-08-31T14:48:50Z

In response to PR#1378, I have removed the socket options sections from API.md, and incorporated them in the new document APISocketOptions.md. Both files have been edited and require technical review.

maxsharabayko · 2020-08-31T15:34:44Z

docs/APISocketOptions.md

+- *TODO: This option weirdly only allows the socket used in **bind()** to use the 
+local address that another socket is already using, but not to disallow another 
+socket in the same application to use the binding address that the current 
+socket is already using. What it actually changes is that when given an address in 
+**bind()** is already used by another socket, this option will make the binding 
+fail instead of adding the socket to the shared group of that socket that 
+already has bound this address - but it will not disallow another socket to reuse 
+its address.* 


@ethouris Could you comment on this TODO? Is it still valid?

Tell you what. Remove this, and I'll make a PR to fix it. This maybe isn't as bad as described here, but I still think there's some weird inconsistency.

docs/APISocketOptions.md

ethouris · 2020-09-01T07:50:20Z

docs/API.md

+
+There are some example applications so that you can see how the API is being used, 
+including `srt-live-transmit`, `srt-file-transmit` and `srt-multiplex`. All SRT 
+related material is contained in `transmitmedia.*` files in the `common` directory 


Things have changed over time here and it wasn't updated:

srt-multiplex no longer exists. The architecture was updated in srt-live-transmit and therefore this application has been moved to test, as only they preserve the old architecture. So this file should be removed from this list.

The directory that contains these transmitmedia.* files is now apps, not common.

Rest of the description can stay as is.

ethouris · 2020-09-01T07:56:34Z

docs/API.md

-    * srctime: [OUT] timestamp set for this dataset when sending
-    * pktseq: [OUT] packet sequence number (first packet from the message, if it spans multiple UDP packets)
-    * msgno: [OUT] message number assigned to the currently received message
+    * `msgttl`: inorder; unused


Here were two fields mentioned in this entry. So, either leave as is, or split into two different enums.

ethouris · 2020-09-01T07:58:53Z

docs/API.md

-Mode settings determine how the sender and receiver functions work.
-The main socket options (see below for full description) that control it are:
+Mode settings determine how the sender and receiver functions work. The main 
+[socket options](#https://github.com/Haivision/srt/blob/master/docs/API-SocketOptions.md) 


Dash in the name causes likely a wrong link.

OTOH I personally prefer the API-SocketOptions.md filename, you may consider renaming it.

BTW. I think it will be better if the links to the document use a relative, not network-absolute path.

ethouris · 2020-09-01T10:02:23Z

docs/API.md

-and receiving. For example, SNDSYN defines blocking for `srt_connect` and
-RCVSYN defines blocking for `srt_accept`. The SNDSYN also makes `srt_close`
+and receiving. For example, `SNDSYN` defines blocking for `srt_connect` and
+`RCVSYN` defines blocking for `srt_accept`. The `SNDSYN` also makes `srt_close`


That should be updated as this is completely wrong.

The non-blocking for srt_connect is set by setting SRTO_RCVSYN to false (surprisingly, as the connection-ready state in epoll is SRT_EPOLL_OUT). The non-blocking for srt_accept is, also surprisingly, set by SRTO_RCVSYN to false on the listener socket from which the accepting is done, although the readiness for acceptance is marked by SRT_EPOLL_IN event in epoll.

I'd make rather something like this:

Note also that blocking and non-blocking modes apply not only for sending and receiving. The SRTO_RCVSYN flag set to false on a socket used with srt_connect makes this function work asynchronously (connection-ready state in epoll is however SRT_EPOLL_OUT - that is, you should call this function once, and then treat it as connected when it becomes write-ready). When this flag is set to false on a listener socket, then the call to srt_accept with that socket is also asynchronous (accept-ready state in epoll is SRT_EPOLL_IN), that is, when called without having a new connection reporting in, it will return immediately with SRT_EASYNCRCV. Also when SRTO_SNDSYN set to true on the socket (default), it makes also srt_close exit only after the sending buffer is completely empty.

(Updated the comment above - I messed up a bit with explanations, but the text in the last paragraph is ok.)

How about this?

Blocking and Non-blocking Mode

SRT functions can also work in blocking and non-blocking modes, for which
there are two separate options for sending and receiving: SRTO_SNDSYN and
SRTO_RCVSYN. When blocking mode is used, a function will not exit until
the availability condition is satisfied. In non-blocking mode the function
always exits immediately, and if enough resources are not available it
returns an error with an appropriate code. The use of non-blocking mode usually
requires using some polling mechanism, which in SRT is EPoll.

Note also that blocking and non-blocking modes apply not only to sending and
receiving. For example, to make srt_connect run asynchronously (blocking mode),
set SRTO_RCVSYN to false on the socket used for connecting (the SRT_EPOLL_OUT
readiness should be checked here). To make srt_accept run asynchronously (blocking
mode), set SRTO_RCVSYN to false on the listener socket (the SRT_EPOLL_IN readiness
should be checked here). Setting SRTO_SNDSYN to true on the socket makes srt_close
exit only after the sending buffer is completely empty.

First of all, to make you imagine how it works: The SRTO_SNDSYN and SRTO_RCVSYN options are set to true by default, and this means blocking mode. So you should set appropriate option to false in order to have non-blocking mode. So, among things mentioned here:

SRTO_RCVSYN set to false makes srt_connect run asynchronously (non-blocking)

IDEM (on the listener socket), makes srt_accept run asynchronously (non-blocking)

When SRTO_SNDSYN is true (default!) then also srt_close will block until the sender buffer is empty. When this is set to false, the srt_close function exits immediately, but sending will continue in the background.

Blocking and Non-blocking Mode

SRT functions can also work in blocking and non-blocking modes, for which
there are two separate options for sending and receiving: SRTO_SNDSYN and
SRTO_RCVSYN (both are set to true by default, corresponding to blocking mode).
When blocking mode is used, a function will not exit until the availability
condition is satisfied. In non-blocking mode the function always exits immediately,
and if enough resources are not available it returns an error with an appropriate
code. The use of non-blocking mode usually requires using some polling mechanism,
which in SRT is EPoll.

Note also that blocking and non-blocking modes apply not only to sending and
receiving. For example, to make srt_connect run asynchronously (non-blocking mode),
set SRTO_RCVSYN to false on the socket used for connecting (the SRT_EPOLL_OUT
readiness should be checked here). To make srt_accept run asynchronously (non-blocking
mode), set SRTO_RCVSYN to false on the listener socket (the SRT_EPOLL_IN readiness
should be checked here). When SRTO_SNDSYN is true (default) then also srt_close
will block until the sender buffer is empty. When SRTO_SNDSYN is set to false, the
srt_close function exits immediately, but sending will continue in the background.

ethouris · 2020-09-01T10:14:08Z

docs/API.md

+
+The following terms are used in the description of transmission types:
+
+**HANGUP / RESUME**: When a function "HANGS UP" this means that it returns an 


Actually the intention of this description was to have some common abstract terms for what happens differently depending on whether the function works in blocking and non-blocking mode.

If the function makes a "HANGUP", it means that:

in blocking mode, the call will block until the condition no longer applies

in non-blocking mode, the call will end up with appropriate "again" error (as enumerated below)

If the function makes a "RESUME", it means that it has done something it was intended to do, and in blocking mode it exits at all. In case of non-blocking mode it would exit immediately anyway, but in this case with success.

I think better would be to describe these terms both at one for particular mode (separately for blocking and non-blocking), for example:

The terms of HANGUP and RESUME resolve differently depending on the blocking state.

In blocking mode:

HANGUP means that the function execution is stalled (blocking)

RESUME means that the function exits - with either success or operation failure code

In non-blocking mode the function always exits immediately, with only a different result:

HANGUP means that the function exits with the "again"-type error (such as SRT_EASYNC*, SRT_ETIMEOUT or SRT_ECONGEST, as specified in its description).

RESUME means that the function, when called, exits with either success or an operation failure

How about this:

HANGUP: When a function "HANGS UP" this means that:

In blocking mode the call will block until the condition no longer applies.

In non-blocking mode the call will end up with appropriate error. It returns
an error from the MJ_AGAIN category (see SRT_EASYNC*, SRT_ETIMEOUT and
SRT_ECONGEST symbols from the SRT_ERRNO enumeration type.

RESUME: When a function "RESUMES" this means that:

In blocking the function has done something it was intended to do and exits.

In non-blocking the function has done something and returned a non-error
status. It would exit immediately anyway, but in this case successfully.

Note that blocking mode in SRT is separate for sending and receiving, and is set
by the SRTO_SNDSYN and SRTO_RCVSYN options respectively.

Not sure how clear the "condition" statement is. I'd rather speak about "readiness".

HANGUP is when the function is not ready to perform the operation. In blocking mode it will block, in non-blocking mode will return an error as per this list.

RESUME is when the function is (or better: becomes) ready to perform the operation and it performs it. In both modes the same way.

These terms are intended to be used in the further description by stating under which condition the function is ready, so it HANGS UP if not, and RESUMES if it is.

Terminology

The following terms are used in the description of transmission types:

HANGUP: When a function "HANGS UP" this means that it is not ready to perform
an operation:

In blocking mode the call will block until the condition no longer applies.

In non-blocking mode the call will return the appropriate error from the
MJ_AGAIN category (see SRT_EASYNC*, SRT_ETIMEOUT and SRT_ECONGEST
symbols from the SRT_ERRNO enumeration type.

RESUME: When a function "RESUMES" this means that it has become ready to perform
an operation:

In blocking mode the function has done something it was intended to do and exits.

In non-blocking mode the function has done something and returned a non-error
status. It would exit immediately anyway, but in this case successfully.

In the descriptions further below, these terms denote the readiness of a function
under certain conditions. Note that blocking mode in SRT is separate for sending
and receiving, and is set by the SRTO_SNDSYN and SRTO_RCVSYN options respectively.

ethouris · 2020-09-01T10:23:01Z

docs/API.md

+
+  - **TTL** defines how much time (in ms) the message should wait in the sending 
+  buffer for the opportunity to be picked up by the sender thread and sent over 
+  the network; otherwise it is dropped.


There's one thing nonprecise here: The packet is ALWAYS sent for the first time. The TTL parameter sets only a time tolerance for a case when the packet was lost and had to be retransmitted. After that time the retransmission is given up, and if the receiver peer still requests this retransmission, it will be denied by DROPREQ message. I don't think it should be described here in such details, but it should be clearly stated that TTL does not concern regular first-time sending of the packet - which is always done regardless of any conditions - but only if any of the packets carrying this message were lost.

ethouris · 2020-09-01T10:36:09Z

docs/APISocketOptions.md

+bursts sent at the moment when an idle link is activated.
+
+- Smaller values of this option respect low latency requirements very
+well, may cause overreaction on even slightly stretched response times. This is


As the second part of this sentence is a drawback warning and contradicts the first part of the sentence being an advantage announcement, I think the second part definitely deserves some "however" or "unfortunately".

ethouris

Done review, please consider the above.

docs/API.md

Co-authored-by: stevomatthews <smatthews@haivision.com>

stevomatthews

Addressed comments from @ethouris

stevomatthews · 2020-09-02T15:39:55Z

docs/API.md

-and receiving. For example, SNDSYN defines blocking for `srt_connect` and
-RCVSYN defines blocking for `srt_accept`. The SNDSYN also makes `srt_close`
+and receiving. For example, `SNDSYN` defines blocking for `srt_connect` and
+`RCVSYN` defines blocking for `srt_accept`. The `SNDSYN` also makes `srt_close`


How about this?

stevomatthews · 2020-09-02T15:40:22Z

docs/API.md

-and receiving. For example, SNDSYN defines blocking for `srt_connect` and
-RCVSYN defines blocking for `srt_accept`. The SNDSYN also makes `srt_close`
+and receiving. For example, `SNDSYN` defines blocking for `srt_connect` and
+`RCVSYN` defines blocking for `srt_accept`. The `SNDSYN` also makes `srt_close`


Blocking and Non-blocking Mode

SRT functions can also work in blocking and non-blocking modes, for which
there are two separate options for sending and receiving: SRTO_SNDSYN and
SRTO_RCVSYN. When blocking mode is used, a function will not exit until
the availability condition is satisfied. In non-blocking mode the function
always exits immediately, and if enough resources are not available it
returns an error with an appropriate code. The use of non-blocking mode usually
requires using some polling mechanism, which in SRT is EPoll.

Note also that blocking and non-blocking modes apply not only to sending and
receiving. For example, to make srt_connect run asynchronously (blocking mode),
set SRTO_RCVSYN to false on the socket used for connecting (the SRT_EPOLL_OUT
readiness should be checked here). To make srt_accept run asynchronously (blocking
mode), set SRTO_RCVSYN to false on the listener socket (the SRT_EPOLL_IN readiness
should be checked here). Setting SRTO_SNDSYN to true on the socket makes srt_close
exit only after the sending buffer is completely empty.

stevomatthews · 2020-09-02T16:24:08Z

docs/API.md

+
+The following terms are used in the description of transmission types:
+
+**HANGUP / RESUME**: When a function "HANGS UP" this means that it returns an 


How about this:

HANGUP: When a function "HANGS UP" this means that:

In blocking mode the call will block until the condition no longer applies.

In non-blocking mode the call will end up with appropriate error. It returns
an error from the MJ_AGAIN category (see SRT_EASYNC*, SRT_ETIMEOUT and
SRT_ECONGEST symbols from the SRT_ERRNO enumeration type.

RESUME: When a function "RESUMES" this means that:

In blocking the function has done something it was intended to do and exits.

In non-blocking the function has done something and returned a non-error
status. It would exit immediately anyway, but in this case successfully.

Note that blocking mode in SRT is separate for sending and receiving, and is set
by the SRTO_SNDSYN and SRTO_RCVSYN options respectively.

docs/API.md

Co-authored-by: stevomatthews <smatthews@haivision.com>

stevomatthews

More changes in response to comments from @ethouris

docs/API.md

stevomatthews

Minor edits.

docs/APISocketOptions.md

Co-authored-by: stevomatthews <smatthews@haivision.com>

stevomatthews · 2020-09-03T16:01:11Z

docs/APISocketOptions.md

+- *TODO: This option weirdly only allows the socket used in **bind()** to use the 
+local address that another socket is already using, but not to disallow another 
+socket in the same application to use the binding address that the current 
+socket is already using. What it actually changes is that when given an address in 
+**bind()** is already used by another socket, this option will make the binding 
+fail instead of adding the socket to the shared group of that socket that 
+already has bound this address - but it will not disallow another socket to reuse 
+its address.* 


Suggested change

- *TODO: This option weirdly only allows the socket used in **bind()** to use the

local address that another socket is already using, but not to disallow another

socket in the same application to use the binding address that the current

socket is already using. What it actually changes is that when given an address in

**bind()** is already used by another socket, this option will make the binding

fail instead of adding the socket to the shared group of that socket that

already has bound this address - but it will not disallow another socket to reuse

its address.*

stevomatthews

Minor edits

Added modified API.md and new APISocketOptions.md

eea9301

maxsharabayko added this to the v1.5.0 - Sprint 22 milestone Aug 31, 2020

maxsharabayko added [docs] Area: Improvements or additions to documentation Type: Maintenance Work required to maintain or clean up the code labels Aug 31, 2020

maxsharabayko requested a review from ethouris August 31, 2020 14:53

maxsharabayko assigned ethouris, maxsharabayko and stevomatthews Aug 31, 2020

maxsharabayko reviewed Aug 31, 2020

View reviewed changes

stevomatthews commented Aug 31, 2020

View reviewed changes

docs/APISocketOptions.md Show resolved Hide resolved

ethouris reviewed Sep 1, 2020

View reviewed changes

maxsharabayko linked an issue Sep 1, 2020 that may be closed by this pull request

[docs] Move socket options from API.md to a separate document #1378

Closed

ethouris reviewed Sep 1, 2020

View reviewed changes

stevomatthews commented Sep 1, 2020

View reviewed changes

docs/API.md Outdated Show resolved Hide resolved

stevomatthews commented Sep 1, 2020

View reviewed changes

docs/API.md Outdated Show resolved Hide resolved

stevomatthews commented Sep 1, 2020

View reviewed changes

docs/API.md Outdated Show resolved Hide resolved

Post-review followup, part 1, TBC.

c298561

Co-authored-by: stevomatthews <smatthews@haivision.com>

maxsharabayko mentioned this pull request Sep 2, 2020

Added connection callback, called when the async connection succeeded or failed #1487

Merged

stevomatthews commented Sep 2, 2020

View reviewed changes

Updated: TTL parameter description.

4ba766c

Co-authored-by: stevomatthews <smatthews@haivision.com>

stevomatthews commented Sep 2, 2020

View reviewed changes

stevomatthews commented Sep 3, 2020

View reviewed changes

docs/API.md Outdated Show resolved Hide resolved

stevomatthews commented Sep 3, 2020

View reviewed changes

docs/API.md Outdated Show resolved Hide resolved

stevomatthews commented Sep 3, 2020

View reviewed changes

docs/APISocketOptions.md Outdated Show resolved Hide resolved

ethouris and others added 2 commits September 3, 2020 17:57

Apply suggestions from code review (next part)

1951fbb

Co-authored-by: stevomatthews <smatthews@haivision.com>

Followup, next part

80703bb

Co-authored-by: stevomatthews <smatthews@haivision.com>

stevomatthews commented Sep 3, 2020

View reviewed changes

ethouris approved these changes Sep 3, 2020

View reviewed changes

maxsharabayko merged commit 4ee1780 into Haivision:master Sep 4, 2020

mbakholdina modified the milestones: v1.5.0 - Sprint 22, v1.4.2 Oct 14, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added modified API.md and new APISocketOptions.md #1510

Added modified API.md and new APISocketOptions.md #1510

stevomatthews commented Aug 31, 2020

maxsharabayko Aug 31, 2020

ethouris Sep 1, 2020

ethouris Sep 1, 2020

ethouris Sep 1, 2020

ethouris Sep 1, 2020

ethouris Sep 1, 2020 •

edited

Loading

ethouris Sep 2, 2020

stevomatthews Sep 2, 2020

stevomatthews Sep 2, 2020

ethouris Sep 2, 2020

stevomatthews Sep 2, 2020

ethouris Sep 3, 2020

ethouris Sep 1, 2020 •

edited

Loading

stevomatthews Sep 2, 2020

ethouris Sep 2, 2020 •

edited

Loading

stevomatthews Sep 2, 2020

ethouris Sep 1, 2020

ethouris Sep 1, 2020

ethouris left a comment

stevomatthews left a comment

stevomatthews Sep 2, 2020

stevomatthews Sep 2, 2020

stevomatthews Sep 2, 2020

stevomatthews left a comment

stevomatthews left a comment

stevomatthews Sep 3, 2020

stevomatthews left a comment


		The following terms are used in the description of transmission types:

		HANGUP / RESUME: When a function "HANGS UP" this means that it returns an

Added modified API.md and new APISocketOptions.md #1510

Added modified API.md and new APISocketOptions.md #1510

Conversation

stevomatthews commented Aug 31, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ethouris Sep 1, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Blocking and Non-blocking Mode

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Blocking and Non-blocking Mode

Choose a reason for hiding this comment

ethouris Sep 1, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ethouris Sep 2, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Terminology

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ethouris left a comment

Choose a reason for hiding this comment

stevomatthews left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Blocking and Non-blocking Mode

Choose a reason for hiding this comment

stevomatthews left a comment

Choose a reason for hiding this comment

stevomatthews left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stevomatthews left a comment

Choose a reason for hiding this comment

ethouris Sep 1, 2020 •

edited

Loading

ethouris Sep 1, 2020 •

edited

Loading

ethouris Sep 2, 2020 •

edited

Loading