Skip to content

VPAAMP-371 Replatce SyncBegin and Sync end with modern cpp RAII imple…#1504

Merged
pstroffolino merged 3 commits into
dev_sprint_25_2from
feature/VPAAMP-371_SyncStart_End
May 23, 2026
Merged

VPAAMP-371 Replatce SyncBegin and Sync end with modern cpp RAII imple…#1504
pstroffolino merged 3 commits into
dev_sprint_25_2from
feature/VPAAMP-371_SyncStart_End

Conversation

@DomSyna
Copy link
Copy Markdown
Contributor

@DomSyna DomSyna commented May 22, 2026

SyncBegin()/SyncEnd() were thin wrappers around mLock.lock()/mLock.unlock()] that required callers to manually match every acquire with a release.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR modernizes PrivateInstanceAAMP’s “GStreamer operation” locking by replacing the manual SyncBegin()/SyncEnd() pair with an RAII-style SyncLock() that returns a std::unique_lock<std::recursive_mutex>, reducing the risk of mismatched lock/unlock calls across the player and collectors.

Changes:

  • Replace SyncBegin()/SyncEnd() with [[nodiscard]] SyncLock() in PrivateInstanceAAMP (header + implementation).
  • Update call sites across player/collectors/DRM and unit-test fakes/mocks to use scoped RAII locking.
  • Adjust unit tests to validate the new API entry point (SyncLock()).

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
priv_aamp.h Removes SyncBegin/SyncEnd declarations and introduces [[nodiscard]] SyncLock() with updated Doxygen.
priv_aamp.cpp Implements SyncLock() and updates multiple internal critical sections to use RAII locking.
aampgstplayer.cpp Updates AAMPGstPlayer::Pause() to use SyncLock() instead of manual begin/end.
fragmentcollector_mpd.cpp Updates MPD collector destructor cleanup to use scoped SyncLock().
fragmentcollector_hls.cpp Updates HLS collector destructor cleanup to use scoped SyncLock().
drm/DrmInterface.cpp Updates curl termination path to use scoped SyncLock().
test/utests/fakes/FakePrivateInstanceAAMP.cpp Removes fake SyncBegin/SyncEnd stubs and adds fake SyncLock() implementation.
test/utests/drm/mocks/aampMocks.cpp Removes mock SyncBegin/SyncEnd stubs and adds mock SyncLock() implementation.
test/utests/tests/PrivAampTests/PrivAampTests.cpp Updates unit test to call SyncLock() instead of SyncBegin/SyncEnd.

Comment thread priv_aamp.cpp Outdated
Comment thread drm/DrmInterface.cpp Outdated
Comment thread priv_aamp.cpp Outdated
Comment thread drm/DrmInterface.cpp Outdated
Comment thread test/utests/drm/mocks/aampMocks.cpp
Comment thread test/utests/fakes/FakePrivateInstanceAAMP.cpp Outdated
Comment thread fragmentcollector_hls.cpp Outdated
Comment thread priv_aamp.cpp Outdated
Comment thread priv_aamp.cpp Outdated
Copy link
Copy Markdown

@dnfaulkner dnfaulkner left a comment

Choose a reason for hiding this comment

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

Very nice :)

@DomSyna DomSyna marked this pull request as ready for review May 22, 2026 14:32
@DomSyna DomSyna requested a review from a team as a code owner May 22, 2026 14:32
@DomSyna DomSyna requested a review from Copilot May 22, 2026 14:32
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

@pstroffolino pstroffolino merged commit 119cb94 into dev_sprint_25_2 May 23, 2026
9 of 10 checks passed
@pstroffolino pstroffolino deleted the feature/VPAAMP-371_SyncStart_End branch May 23, 2026 03:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants