VPAAMP-881 Migrate to HEAD of middleware-player-interface - #1815
Merged
Conversation
Remove dependency on patched middleware commit bd2b3b1. All required features (setPtsOffset, GStreamer base-1.0 fixes, OpenSSL includes) are now present natively in middleware-player-interface HEAD (0.3.1). Changes: - scripts/install_options.sh: Remove pinned commit bd2b3b1, use HEAD - scripts/install_middleware_interfaces.sh: Remove commit checkout logic (sibling and clone paths) and patch application block - .github/workflows/L1-tests.yml: Remove git checkout bd2b3b1 and patch application steps; clone HEAD directly - test/utests/fakes/CMakeLists.txt: Remove deprecated internal aamp/middleware fallback; FATAL_ERROR if middleware not found - test/utests/tests/CommonTestIncludes.cmake: Same - remove deprecated aamp/middleware fallback paths - OSX/patches/middleware-fixes-4c1d90a.patch: Archived to OSX/patches/archive/ with README explaining historical context - OSX/patches/middleware-build-fixes-summary.md: Updated to note patch is no longer applied (VPAAMP-881) Verified: macOS clean build succeeds with middleware-player-interface develop branch (0165183, release 0.3.1) without any patches. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
pstroffolino
force-pushed
the
feature/VPAAMP-881
branch
3 times, most recently
from
August 2, 2026 06:09
9af618f to
c464372
Compare
DrmSession gained four new protected members in middleware develop (lifecycle guard fix): mLifecycleMutex, mLifecycleCV, mActiveOperations, mMarkedForDestruction. The fake DrmSession constructor in FakeAampDRMSessionManager.cpp was not initializing these members, leaving mActiveOperations as garbage on Linux/glibc. This caused heap corruption (glibc sysmalloc assertion) in multi-key and concurrency tests on Linux CI, while macOS passed due to its more lenient allocator behavior. Fix: 1. FakeAampDRMSessionManager.cpp: initialize all four new members in the fake DrmSession constructor, matching the real DrmSession.cpp. 2. AampDRMLicPreFetcherTestCases.cpp / AampDRMLicManagerTestCases.cpp: add ~TestDrmSession() that marks for destruction and waits for in-flight operations to drain before delete, mirroring what real DrmSessionManager now does for production sessions. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
pstroffolino
force-pushed
the
feature/VPAAMP-881
branch
from
August 2, 2026 06:58
c464372 to
478e9d6
Compare
…ally work The commit ID option was parsed and exported but never used when cloning middleware-player-interface from GitHub. Added logic to checkout the specified commit after cloning, with fallback to HEAD if checkout fails. This makes the option functional as documented. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR migrates AAMP’s build/CI integration to use the current middleware-player-interface HEAD (develop / 0.3.1) instead of a pinned, patched commit, and archives the now-obsolete macOS patch.
Changes:
- Updates install scripts and CI workflow to clone
middleware-player-interfacedevelop/HEAD directly (no patch application / no pinned checkout). - Refactors utest CMake include-path selection to prefer pkg-config or an absolute-path CI clone, and adds a compile definition gate for newer DRM session lifecycle fields.
- Archives the historical macOS middleware patch and updates associated documentation to reflect it is no longer applied.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
test/utests/tests/CommonTestIncludes.cmake |
Switches middleware include fallback to an absolute repo path variable for CI clones. |
test/utests/tests/AampDRMLicPreFetcherTests/AampDRMLicPreFetcherTestCases.cpp |
Adds conditional lifecycle-guard teardown logic for TestDrmSession when building against newer middleware headers. |
test/utests/tests/AampDRMLicManagerTests/AampDRMLicManagerTestCases.cpp |
Adds conditional lifecycle-guard teardown logic for TestDrmSession when building against newer middleware headers. |
test/utests/fakes/FakeAampDRMSessionManager.cpp |
Initializes new lifecycle-guard members in the fake DrmSession ctor when enabled. |
test/utests/fakes/CMakeLists.txt |
Updates middleware include fallback to use MIDDLEWARE_REPO_ABS_PATH instead of relative AAMP_ROOT assumptions. |
test/utests/CMakeLists.txt |
Computes/caches MIDDLEWARE_REPO_ABS_PATH and conditionally defines DRMSESSION_HAS_LIFECYCLE_GUARD. |
scripts/install_options.sh |
Removes default pinned middleware commit id (defaults to HEAD). |
scripts/install_middleware_interfaces.sh |
Removes sibling-repo checkout logic and patch application; clones develop/HEAD by default. |
OSX/patches/middleware-build-fixes-summary.md |
Marks middleware patch as historical and no longer applied. |
OSX/patches/archive/README.md |
Adds documentation for archived patch context and rationale. |
OSX/patches/archive/middleware-fixes-4c1d90a.patch |
Archives the historical patch file for reference. |
.github/workflows/L1-tests.yml |
Updates CI to clone middleware develop/HEAD and removes patch/commit pin steps. |
Suppressed comments (1)
test/utests/fakes/CMakeLists.txt:55
- This CMake still falls back to internal
aamp/middlewareheaders in theelse()branch. The PR description says the legacy internal fallback should be removed and the build should fail if middleware headers are not found. Replacing the internal fallback with a FATAL_ERROR would make missing middleware explicit and avoid silently compiling against deprecated headers.
else()
# Internal middleware paths (for local builds with aamp/middleware folder)
message(STATUS "Using internal aamp/middleware paths (deprecated)")
include_directories(${AAMP_ROOT}/middleware)
…riable name, explicit mutex includes - CommonTestIncludes.cmake: replace silent internal fallback with FATAL_ERROR so builds fail with a clear message when middleware-player-interface headers are not found. - install_middleware_interfaces.sh: fix variable name mismatch (MIDDLEWARE_PLAYER_INTERFACE_COMMIT_ID -> OPTION_MIDDLEWARE_PLAYER_INTERFACE_COMMIT_ID); update header docs to list the optional variable. - AampDRMLicManagerTestCases.cpp, AampDRMLicPreFetcherTestCases.cpp: add explicit #include <mutex> for TestDrmSession destructor. - L1-tests.yml: reword comment to clarify safe.directory must run before subsequent steps that invoke git in the workspace. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
install-aamp.sh was exporting the value under the bare name MIDDLEWARE_PLAYER_INTERFACE_COMMIT_ID before calling the sourced install_middleware_interfaces.sh, but that script now reads OPTION_MIDDLEWARE_PLAYER_INTERFACE_COMMIT_ID (fixed in previous commit). Remove the stale export so the option variable set by install_options.sh flows through directly without renaming. Also add explicit warnings in install_middleware_interfaces.sh for the two cases where --middleware-player-interface-commit-id is silently ignored: when an explicit local path is provided via --middleware-player-interface-local-path, and when a sibling middleware-player-interface directory is auto-detected. In both cases the repo is used at whatever commit it is already on. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Remove dependency on patched middleware commit bd2b3b1. All required features (setPtsOffset, GStreamer base-1.0 fixes, OpenSSL includes) are now present natively in middleware-player-interface HEAD (0.3.1).
Changes:
Verified: macOS clean build succeeds with middleware-player-interface develop branch (0165183, release 0.3.1) without any patches.
Generated with Devin