Skip to content

Conversation

@cl-ment
Copy link
Collaborator

@cl-ment cl-ment commented Mar 24, 2025

This pull request proposes an alternative to PR #3138 and #3149, both of which address the compilation of SRT on platforms that do not support the pthread_condattr_setclock function. To achieve this, we introduced a new compilation flag, HAVE_PTHREAD_CONDATTR_SETCLOCK, which is separate from ENABLE_MONOTONIC_CLOCK. Additionally, we moved the function availability check to occur after determining the thread library.

@cl-ment cl-ment added Type: Maintenance Work required to maintain or clean up the code [core] Area: Changes in SRT library core [build] Area: Changes in build files labels Mar 24, 2025
@cl-ment cl-ment added this to the v1.5.5 milestone Mar 24, 2025
Copy link
Collaborator

@maxsharabayko maxsharabayko left a comment

Choose a reason for hiding this comment

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

Apart from the formatting comment LGTM. 👍

CMakeLists.txt Outdated
Comment on lines 965 to 978
message(STATUS "Pthread library: ${PTHREAD_LIBRARY}")
endif()

# To avoid the need for other judgments when ENABLE_STDCXX_SYNC is OFF in the future, this is a separate conditional statement.
if (NOT ENABLE_STDCXX_SYNC AND ENABLE_MONOTONIC_CLOCK)
list(PREPEND CMAKE_REQUIRED_LIBRARIES "${PTHREAD_LIBRARY}")
check_symbol_exists(pthread_condattr_setclock "pthread.h" HAVE_PTHREAD_CONDATTR_SETCLOCK)
if (HAVE_PTHREAD_CONDATTR_SETCLOCK)
add_definitions(-DHAVE_PTHREAD_CONDATTR_SETCLOCK=1)
else ()
message(WARNING, "Can't find pthread_condattr_setclock. Try -DENABLE_STDCXX_SYNC=ON or -DENABLE_MONOTONIC_CLOCK=OFF; the latter is not recommended though.")
endif ()
endif ()

Copy link
Collaborator

Choose a reason for hiding this comment

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

Indent using tabs, please.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe add_definitions(-DHAVE_PTHREAD_CONDATTR_SETCLOCK=0) otherwise? Not necessary though. Just a thought.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not necessary. If this function doesn't exist, monotonic clock should be simply off, as it's not usable, and SRT with this enforced won't be simply usable at all.

@maxsharabayko
Copy link
Collaborator

maxsharabayko commented Mar 26, 2025

Travis PowerPC jobs seems to be failing on the google tests's cmake configure stage. 🤔

@ethouris
Copy link
Collaborator

This is idiotic. No indication of an error at all. Maybe add to .travis.yml some thing like cat CMakeFiles/CMakeError.log?

@cl-ment cl-ment merged commit d6d0e5a into Haivision:master Jun 6, 2025
10 of 11 checks passed
@cl-ment cl-ment deleted the 3149-alternative branch June 6, 2025 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[build] Area: Changes in build files [core] Area: Changes in SRT library core Type: Maintenance Work required to maintain or clean up the code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants