Skip to content

VPAAMP-881 Migrate to HEAD of middleware-player-interface - #1815

Merged
pstroffolino merged 5 commits into
dev_sprint_25_2from
feature/VPAAMP-881
Aug 3, 2026
Merged

VPAAMP-881 Migrate to HEAD of middleware-player-interface#1815
pstroffolino merged 5 commits into
dev_sprint_25_2from
feature/VPAAMP-881

Conversation

@pstroffolino

Copy link
Copy Markdown
Contributor

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

@pstroffolino
pstroffolino requested a review from a team as a code owner August 1, 2026 16:51
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
pstroffolino force-pushed the feature/VPAAMP-881 branch 3 times, most recently from 9af618f to c464372 Compare August 2, 2026 06:09
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>
…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>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 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-interface develop/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/middleware headers in the else() 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)

Comment thread test/utests/tests/CommonTestIncludes.cmake Outdated
Comment thread scripts/install_middleware_interfaces.sh
Comment thread .github/workflows/L1-tests.yml Outdated
pstroffolino and others added 2 commits August 3, 2026 13:59
…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>
@pstroffolino
pstroffolino merged commit b73a363 into dev_sprint_25_2 Aug 3, 2026
8 of 9 checks passed
@pstroffolino
pstroffolino deleted the feature/VPAAMP-881 branch August 3, 2026 18:41
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.

2 participants