Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

{EPOLL} Updated documentation after adding new API #873

Merged
merged 9 commits into from
Sep 13, 2019

Conversation

ethouris
Copy link
Collaborator

No description provided.

@ethouris ethouris requested a review from rndi September 12, 2019 13:15
@ethouris ethouris added [docs] Area: Improvements or additions to documentation Priority: High Status: Review Needed Type: Enhancement Indicates new feature requests labels Sep 12, 2019
@ethouris ethouris added this to the v1.4.0 milestone Sep 12, 2019
* a variable set to epoll flags (see below) to use only selected events
* NULL if you want to subscribe a socket for all events in level-triggered mode

Epoll flags are bit flags that can be combined using `|` operator:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'...combined into a mask"

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This value is a mask, no matter if combined or a single flag is set. Maybe line 1200 should mention the "mask", at best.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove C language semantics, i.e. "using | operator"

Epoll flags are bit flags that can be combined using `|` operator:
* `SRT_EPOLL_IN`: report readiness for reading; (accept-readiness on listener socket)
* `SRT_EPOLL_OUT`: report readiness for writing (also connection succeeded)
* `SRT_EPOLL_ERR`: report error on the socket
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • SRT_EPOLL_IN: report readiness for reading or incoming connection on a listener socket
  • SRT_EPOLL_OUT: report readiness for writing or successful connection for a non-blocking socket
  • SRT_EPOLL_ERR: report errors on the socket

Mind that the readiness states reported in epoll are by default
**level-triggered**, unless `SRT_EPOLL_ET` flag is specified, in which
case they are **edge-triggered**. Note that edge-triggered is only
supported for SRT sockets (to be fixed).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The readiness states reported in by default are level-triggered. If
SRT_EPOLL_ET flag is specified, the reported states will become
edge-triggered. Note that at the time of this writing,
the edge-triggered mode is only supported for SRT sockets and not for regular
system sockets.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather say simply "at this time". This might be fixed in future.

which have changed since last call (that is, upon exit from the waiting
function any events reported there will be cleared in the internal flags
and therefore not reported in the next call, until the internals will
clear the state and set it again).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the edge-triggered mode, the function will only return socket states
that have changed since the last call. All events already reported will be
cleared in the internal flags and will not be reported again until
the internal signaling logic clears event report state and raises them again.

@@ -1237,8 +1260,8 @@ int srt_epoll_wait(int eid, SRTSOCKET* readfds, int* rnum, SRTSOCKET* writefds,
```

Blocks the call until any readiness state occurs in the epoll container.
Mind that the readiness states reported in epoll are **permanent, not
edge-triggered**.
Mind that the readiness states reported in epoll are **permanent (level-
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep in mind that...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Damn, good that you pointed that out. This is no longer true. I'll fix accordingly.

functions to check if the output array is not empty. For `srt_epoll_wait` it
is still allowed that either system or user array is empty, as long as EID
isn't subscribed to this type of socket/fd. For `srt_epoll_uwait` it's only
checked if the general output array is not empty.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

srt_epoll_uwait only checks if the general output array is not empty.

docs/API.md Outdated
Once subscriptions are made, a waiting function can be called in order to
block the call commonly for all subscribed sockets together and exit from
waiting when the first one is ready. This can be waited up to the given timeout
in `[ms]`, with two special values:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once subscriptions are made, an SRT polling function can be used to wait for
an event to be raised on any of the subscribed sockets. The polling functions
will exit as soon as at least one event is detected or a timeout occurs.
The timeout is specified in [ms], with two special values:

docs/API.md Outdated
in `[ms]`, with two special values:

- 0: check and report immediately (don't wait)
- -1: wait indefinitely (also hangup until any event happens, not interruptable)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-1: wait indefinitely (ignores system signals, not interruptable)

docs/API.md Outdated

Note: there's no separate place to report error. The only way to distinguish
error and in/out event is when you subscribe to `IN` or `OUT` only, and the
error then can be recognized by the fact that the socket appears in both arrays.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: there's no separate array to report an error. Currently, the only way
to distinguish an error from an in/out event is to provide both,
readfds and writefds arrays and subscribe to ERR. The error can then
be recognized by the fact that the socket appears in both arrays.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to rephrase it as well. What you wrote here is also not exactly true - the trick is that if you subscribe a socket to both SRT_EPOLL_IN and SRT_EPOLL_OUT, there's no way to distinguish readiness for both at once and an error simultaneously. For example, if you subscribe to SRT_EPOLL_IN and SRT_EPOLL_ERR (but not OUT), erroneous sockets will appear in the writefds array.

docs/API.md Outdated

Note that in this function there's a loop that checks for socket readiness
every 10ms. This waiting time can be shortened only for SRT sockets when
they become ready earlier after the previous check.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that in this function there's a loop that checks for socket readiness
every 10ms. This, the minimum poll timeout the function can reliably support
is also 10ms. The return time from a poll function can only be quicker
when there is an event raised on one of the active SRT sockets.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You meant "thus", I believe :)

@rndi rndi merged commit 7780333 into Haivision:master Sep 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[docs] Area: Improvements or additions to documentation Priority: High Type: Enhancement Indicates new feature requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants