Added description for build options. #886

ethouris · 2019-09-24T10:48:06Z

Fixes #885

Added build options description: BuildOptions.md.

Added entry info in README.

Removed unnecessary description in CMakeLists.txt in comments; the new file is a more appropriate place for this description.

Fixed duplicated enable-unittests in configure-data.tcl.

…unnecessary descr in cmake. Fixed duplicated enable-unittests

stevomatthews · 2019-09-24T20:36:43Z

README.md

@@ -41,6 +41,8 @@ As audio/video packets are streamed from a source to a destination device, SRT d
 * OpenSSL
 * Pthreads (for POSIX systems it's builtin, for Windows there's a library)

+For detailed description of the build system and options, please [read here](BuildOptions.md).


Suggested change

For detailed description of the build system and options, please [read here](BuildOptions.md).

For detailed description of the build system and options, please [read](BuildOptions.md).

Maybe "read this"?

stevomatthews · 2019-09-24T21:57:24Z

docs/BuildOptions.md

+source IP address exactly the same as the one, which the peer is trying to
+connect to.
+
+When this is OFF, the source IP address in such outgoing UDP packet will


@ethouris Shouldn't this be "ON"?

No, this exactly describes what happens by default. But as you are mistaken here, users will be as well, so it likely needs to be said explicitly.

stevomatthews

My review of BuildOptions.md

BuildOptions_reviewSM_2019-09-24.md.zip

ethouris · 2019-09-26T09:17:26Z

I made fixes mostly in the form you provided; here is my extra description of places where my checked-in version differs. @stevomatthews Please review, serveral things may still need fixing because of that.

BuildOption_fix_comments.md.zip

stevomatthews · 2019-09-26T14:58:40Z

docs/BuildOptions.md

+time, instead of `gettimeofday`. This function forces the use of a monotonic
+clock that is independent of the currently set time in the system.
+The condition variables, for which the `*_timedwait()` functions are used
+with time specification basing on the time obtained from `clock_gettime`


with time specification based on the time obtained from clock_gettime,

stevomatthews · 2019-09-26T14:59:20Z

docs/BuildOptions.md

-`clock_gettime()` function, which is not available on every SDK, or extra
+time, instead of `gettimeofday`. This function forces the use of a monotonic
+clock that is independent of the currently set time in the system.
+The condition variables, for which the `*_timedwait()` functions are used


The condition variables (CV), for which the *_timedwait() functions are used

stevomatthews · 2019-09-26T15:00:09Z

docs/BuildOptions.md

+must be appropriately configured. For now, this is only done for the
+GarbageCollector controlling CV, not every CV used in SRT. The consequence
+of enabling this option, however, may be portability issues resulting from
+the fact that `clock_gettime` function may be unavailable in some SDKs or an extra


the fact that clock_gettime function may be unavailable in some SDKs, or that an extra

stevomatthews · 2019-09-26T15:07:38Z

docs/BuildOptions.md

+must be appropriately configured. For now, this is only done for the
+GarbageCollector controlling CV, not every CV used in SRT. The consequence
+of enabling this option, however, may be portability issues resulting from
+the fact that `clock_gettime` function may be unavailable in some SDKs or an extra
 `-lrt` option is sometimes required (this requirement will be autodetected).



I suggest adding this (edited) paragraph from your comments:

What is important here is that the clock_gettime function is used to obtain
the current time, and then the *_timedwait functions need to be passed the
time specification from the same domain. The time must be specified to the
*_timedwait functions as the absolute time up to which they have to wait.
This means that usually you first obtain the current time from clock_gettime,
add the "waiting time" to it, and then pass the result to the *_timedwait function.

Ok, I'll try to provide the description a little bit more concise.

Maybe this:

The problem is based on the fact that POSIX functions that use timeout
specification (all of *_timedwait) expect the absolute time value.
A relative timeout value can be then only specified by adding it to
the current time, which can be specified as either system or monotonic
clock (which one, is configured in the resources used in the operation).
However the current time of the monotonic clock can only be obtained by
the clock_gettime function.

stevomatthews · 2019-09-26T15:28:30Z

docs/BuildOptions.md

+directory, provided that the library and applications are installed in POSIX/GNU
+style directories. This might be useful when installing SRT and applications
+in a directory, in which the library subdirectory is not explicitly defined
+among the global library paths (such as `/opt/srt-1.4/lib`, for example).


among the global library paths. Consider, for example, this application and its
required library:

/opt/srt/bin/srt-live-transmit

/opt/srt/lib64/libsrt.so

By using the --enable-relative-libpath option, the srt-live-transmit
application has a relative library path defined inside as ../lib64. A dynamic
linker will find the required libsrt.so file by this path: ../lib64/libsrt.so.
This way the dynamic linkage will work even if /opt/srt/lib64 path isn't added
to the system paths in /etc/ld.so.conf or in the LD_LIBRARY_PATH environment
variable.

stevomatthews · 2019-09-26T15:31:26Z

docs/BuildOptions.md

-`--enable-testing` (default: OFF)
---------------------------------
+Enables building SRT as a shared and/or static library, as required for your 
+application. The only practical use is by disabling one of them


application. In practice, you would only disable one or the other

stevomatthews · 2019-09-26T20:49:58Z

docs/BuildOptions.md


+Enables `#include <threadcheck.h>`, which implements `THREAD_*` macros" to 
+support better thread debugging. This is used by one of dependent projects.


support better thread debugging. Included to support an existing project.

stevomatthews · 2019-09-26T20:51:15Z

docs/BuildOptions.md


+When ON, this option enables unit tests, possibly with the download 
+and installation of the Google test library in the build directory. The tests 
+will be run as part of the build process. This is predicted for developers only.


will be run as part of the build process. This is intended for developers only.

stevomatthews · 2019-09-26T21:03:53Z

docs/BuildOptions.md

-directory layout will be used for particular parts. As on all
-known build systems, this defaults to `/usr/local` on POSIX
-compatible systems.
+This is an alias to `--cmake-install-prefix`. This is the root directory for


This is an alias to the --cmake-install-prefix variable that establishes the
root directory for installation, inside of which a GNU/POSIX compatible directory layout will be used. As on all known build systems, this defaults to /usr/local
on GNU/POSIX compatible systems, with lower level GNU/POSIX directories
created inside: /usr/local/bin,/usr/local/lib, etc.

stevomatthews · 2019-09-26T21:06:45Z

docs/BuildOptions.md

-may give more chance to switch to the right thread at the time
-when it is expected to be revived.
+This option will cause more empty loop running, which may cause more CPU usage. 
+Although when processing high bitrate streams the share of empty loop runs will decrease 


Keep in mind, however, that when processing high bitrate streams . . .

stevomatthews · 2019-09-26T21:09:01Z

docs/BuildOptions.md


+Instead of `--with-compiler-prefix` you can use as well `--cmake-c-compiler`


Instead of --with-compiler-prefix you can use --cmake-c-compiler

stevomatthews · 2019-09-26T21:09:21Z

docs/BuildOptions.md


+Instead of `--with-compiler-prefix` you can use as well `--cmake-c-compiler`
+and `--cmake-c++-compiler` options. This can be then thought of as a


and --cmake-c++-compiler options. This can be thought of as a

stevomatthews · 2019-11-05T15:37:39Z

README.md

@@ -41,6 +41,8 @@ As audio/video packets are streamed from a source to a destination device, SRT d
 * OpenSSL
 * Pthreads (for POSIX systems it's builtin, for Windows there's a library)

+For detailed description of the build system and options, please [read](docs/BuildOptions.md).


For detailed description of the build system and options, please read BuildOptions.md.

Oh. Much better.

stevomatthews · 2019-11-05T15:48:10Z

docs/BuildOptions.md

+availabe on some version of Windows, in which case you can turn this OFF,
+however at the expense of not being able to resolve IP address by name,


available on some versions of Windows, in which case you can turn this OFF.
When this option is OFF, however, IP addresses cannot be resolved by name,

stevomatthews · 2019-11-05T15:51:37Z

docs/BuildOptions.md

+specification (all of `*_timedwait`) expect the absolute time value.
+A relative timeout value can be then only specified by adding it to
+the current time, which can be specified as either system or monotonic
+clock (which one, is configured in the resources used in the operation).


clock (as configured in the resources used in the operation).

stevomatthews · 2019-11-05T15:57:59Z

docs/BuildOptions.md

+IP address in such a response packet to 10.0.1.20 in the above example,
+as well as this address is first extracted from the incoming packet as the
+target address. This fixes the problem, as this will be interpreted by
+the caller peer correctly.


IP address in such a response packet (e.g. to 10.0.1.20 in the above example).
This address is first extracted from the incoming packet as the target address,
which fixes the problem, as that address will be interpreted by the caller peer correctly.
Commenting on lines +237 to +240

stevomatthews · 2019-11-05T16:10:37Z

docs/BuildOptions.md

+
+Encryption library to be used. Possible options for `<name>`:
+
+* openssl(default)


openssl (default)

stevomatthews

Minior edits.

stevomatthews

I've finished my review pass. No other comments at this time.

Added build options description. Added entry info in README. Removed …

75838ae

…unnecessary descr in cmake. Fixed duplicated enable-unittests

ethouris requested a review from stevomatthews September 24, 2019 10:49

Mikołaj Małecki added 2 commits September 24, 2019 12:53

Fixed some typos and added lacking option

ed1c436

Fixed some more typos and improved description

7d3f90f

stevomatthews reviewed Sep 24, 2019

View reviewed changes

ethouris added 2 commits September 25, 2019 11:35

Fixed link. Updated link name.

28d2b61

Fixed ON/OFF description for pktinfo

bda090d

ethouris added [docs] Area: Improvements or additions to documentation Status: Review Needed labels Sep 25, 2019

stevomatthews reviewed Sep 25, 2019

View reviewed changes

Fixed after review: round 1

386f908

Merged with github-fixed version

dfa75d8

stevomatthews reviewed Sep 26, 2019

View reviewed changes

Post-review fixes phase 2

cbc15bc

stevomatthews reviewed Sep 26, 2019

View reviewed changes

Review fixes (final phase)

e214088

ethouris added this to Needs review in Development via automation Sep 27, 2019

ethouris added this to the v1.4.1 milestone Sep 27, 2019

stevomatthews reviewed Nov 5, 2019

View reviewed changes

Post-review fix

a9564ac

stevomatthews reviewed Nov 5, 2019

View reviewed changes

Post-review fixes

fbea9bf

maxsharabayko requested a review from stevomatthews November 6, 2019 10:19

stevomatthews reviewed Nov 6, 2019

View reviewed changes

ethouris added the Status: Completed label Nov 6, 2019

rndi merged commit 325dc3f into Haivision:master Nov 7, 2019

Development automation moved this from Needs review to Done Nov 7, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added description for build options. #886

Added description for build options. #886

ethouris commented Sep 24, 2019 •

edited

stevomatthews Sep 24, 2019

ethouris Sep 25, 2019

stevomatthews Sep 24, 2019

ethouris Sep 25, 2019

stevomatthews left a comment

ethouris commented Sep 26, 2019 •

edited

stevomatthews Sep 26, 2019

stevomatthews Sep 26, 2019

stevomatthews Sep 26, 2019

stevomatthews Sep 26, 2019

ethouris Sep 26, 2019

ethouris Sep 26, 2019

stevomatthews Sep 26, 2019

stevomatthews Sep 26, 2019

stevomatthews Sep 26, 2019

stevomatthews Sep 26, 2019

stevomatthews Sep 26, 2019

stevomatthews Sep 26, 2019

stevomatthews Sep 26, 2019

stevomatthews Sep 26, 2019

stevomatthews Nov 5, 2019

ethouris Nov 5, 2019

stevomatthews Nov 5, 2019

stevomatthews Nov 5, 2019

stevomatthews Nov 5, 2019

stevomatthews Nov 5, 2019

stevomatthews left a comment

stevomatthews left a comment


		Enables `#include <threadcheck.h>`, which implements `THREAD_*` macros" to
		support better thread debugging. This is used by one of dependent projects.


		Instead of `--with-compiler-prefix` you can use as well `--cmake-c-compiler`


		Instead of `--with-compiler-prefix` you can use as well `--cmake-c-compiler`
		and `--cmake-c++-compiler` options. This can be then thought of as a

		availabe on some version of Windows, in which case you can turn this OFF,
		however at the expense of not being able to resolve IP address by name,


		Encryption library to be used. Possible options for `<name>`:

		* openssl(default)

Added description for build options. #886

Added description for build options. #886

Conversation

ethouris commented Sep 24, 2019 • edited

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

stevomatthews left a comment

Choose a reason for hiding this comment

ethouris commented Sep 26, 2019 • edited

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

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

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

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

stevomatthews left a comment

Choose a reason for hiding this comment

stevomatthews left a comment

Choose a reason for hiding this comment

ethouris commented Sep 24, 2019 •

edited

ethouris commented Sep 26, 2019 •

edited