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

[apps] Fixed verbose linkage difference. #3002

Merged
merged 2 commits into from
Aug 12, 2024

Conversation

maxsharabayko
Copy link
Collaborator

  • Verbose log now has a single version with a mutex lock.
  • Removed SRT_ENABLE_VERBOSE_LOCK build definition.

Fixes #2145.

Notes

I am not quite sure if the implementation of the Verbose::Log with a mutex is thread safe given the template <class V> Log& operator<<(const V& arg) is not protected by a mutex lock. What's protected is putting the end of line symbol on log destruction.

Verb() << VerbLock << "Starting TargetMedium: " << this;

@maxsharabayko maxsharabayko added Type: Maintenance Work required to maintain or clean up the code [apps] Area: Test applications related improvements labels Aug 8, 2024
@maxsharabayko maxsharabayko added this to the v1.5.4 milestone Aug 8, 2024
@ethouris
Copy link
Collaborator

Ok, let me explain.

The locking was normally not necessary, and the only merit for logging is to prevent printing from different threads to be interleaved. Verb is simply redirecting to the stream (cout or cerr, depending on settings) and it's by definition thread-safe. This locking is only necessary to keep one line not interrupted by printing of the other and prevent messup in the verbose messages.

Therefore it's only required in apps that uses threads and may have a situtation of printing Verb from multiple threads simultaneously. Apps like srt-test-live don't use multiple threads and so they don't use this locking.

apps/verbose.cpp Outdated Show resolved Hide resolved
@maxsharabayko maxsharabayko merged commit c5f166c into Haivision:master Aug 12, 2024
11 of 12 checks passed
@maxsharabayko maxsharabayko deleted the hotfix/verbose-no-lock branch August 12, 2024 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[apps] Area: Test applications related improvements Type: Maintenance Work required to maintain or clean up the code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] 1.4.4: compile time warnings
2 participants