vpaamp 526 remove 'comcast' references from open source AAMP repo#1559
Open
pstroffolino wants to merge 7 commits into
Open
vpaamp 526 remove 'comcast' references from open source AAMP repo#1559pstroffolino wants to merge 7 commits into
pstroffolino wants to merge 7 commits into
Conversation
…tvideo-1.0.0.dylib Risk: Low (assuming all Mac developers ok with OSX26 as baseline) Signed-off-by: Philip Stroffolino <philip_stroffolino@cable.comcast.com>
Signed-off-by: Philip Stroffolino <philip_stroffolino@cable.comcast.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…g' not found
Reason for Change: avoid compilation noise
Removed the 5 link_directories() calls in the Darwin block (for OPENSSL, GSTREAMER, GSTREAMERVIDEO, GSTREAMERBASE, GLIB) and replaced them with a comment explaining why. These were the direct source of all four warnings — Xcode appends $(CONFIGURATION) to every link_directories() entry, creating the nonexistent .../lib/Debug paths.
Changed ${OPENSSL_LIBRARIES} → ${OPENSSL_LINK_LIBRARIES} in LIBAAMP_DEPENDS. _LIBRARIES holds short names like ssl;crypto and requires a directory hint to resolve; _LINK_LIBRARIES holds full absolute paths (e.g. [libssl.dylib|vscode-file://vscode-app/Users/pstrof200@cable.comcast.com/Downloads/Visual%20Studio%20Code.app/Contents/Resources/app/out/vs/code/electron-browser/workbench/workbench.html]) and needs no directory search at all. The rest of the packages (GSTREAMER, GSTREAMERBASE, CURL, etc.) were already using the _LINK_LIBRARIES form — OpenSSL was just inconsistent.
Signed-off-by: Philip Stroffolino <philip_stroffolino@cable.comcast.com>
- gst_subtec plugins: replace author field and GST_PACKAGE_ORIGIN URL with RDK / github.com/rdkcentral/aamp - OSX patches: replace 'Source: COMCAST' with 'Source: RDK'; replace @comcast.com email in Signed-off-by with @rdkcentral.com - AAMP-UVE-API.md: replace operator-specific and tool references - AampcliShader.cpp: update comment referencing Comcast PROD builds - CMCDHeaders.h: add comment explaining why com.comcast-* vendor CMCD extension keys are retained (deployed wire format; changing would be a breaking change for server-side consumers)
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes or neutralizes “Comcast” references across AAMP CLI docs/patch metadata and GStreamer plugin metadata, and adjusts macOS/Xcode build configuration to reduce linker search-path warnings.
Changes:
- Replaces “Comcast” with operator/RDK wording in docs and GStreamer plugin metadata (and updates
GST_PACKAGE_ORIGINto the repo URL). - Updates macOS build configuration (CMake +
install_aampcli.sh) including deployment target handling and avoiding globallink_directories()for Xcode builds. - Updates several OSX patch headers/sign-offs to remove “COMCAST”/
@comcast.comstrings.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/aampcli/AampcliShader.cpp | Updates comment to remove Comcast-specific wording. |
| support/aampmetrics/CMCDHeaders.h | Adds rationale comment for vendor CMCD keys (but introduces an extra “comcast” reference in the comment). |
| scripts/install_aampcli.sh | Adds CMAKE_OSX_DEPLOYMENT_TARGET to Xcode-generator cmake invocation. |
| OSX/patches/websocket-ipplayer2-ubuntu_24_04_build.patch | Updates Signed-off-by email domain to remove @comcast.com. |
| OSX/patches/websocket-ipplayer2-typescpp.patch | Updates “Source: COMCAST” header text. |
| OSX/patches/websocket-ipplayer2-link.patch | Updates “Source: COMCAST” header text. |
| OSX/patches/subttxrend-app-xkbcommon.patch | Updates “Source: COMCAST” header text. |
| OSX/patches/subttxrend-app-ubuntu_24_04_build.patch | Updates Signed-off-by email domain to remove @comcast.com. |
| OSX/patches/subttxrend-app-packet.patch | Updates “Source: COMCAST” header text. |
| OSX/patches/JsonHelper.patch | Updates “Source: COMCAST” header text. |
| OSX/patches/aamp-cli.xscheme.patch | Updates “Source: COMCAST” header text. |
| middleware/gst-plugins/gst_subtec/gstvipertransform.cpp | Updates plugin metadata author and GST_PACKAGE_ORIGIN. |
| middleware/gst-plugins/gst_subtec/gstsubtecsink.cpp | Updates plugin metadata author and GST_PACKAGE_ORIGIN. |
| middleware/gst-plugins/gst_subtec/gstsubtecmp4transform.cpp | Updates plugin metadata author and GST_PACKAGE_ORIGIN. |
| middleware/gst-plugins/gst_subtec/gstsubtecbin.cpp | Updates plugin metadata author and GST_PACKAGE_ORIGIN. |
| CMakeLists.txt | Changes macOS deployment target default and avoids global link_directories() for Xcode builds; updates OpenSSL link variable usage. |
| AAMP-UVE-API.md | Updates documentation wording to remove Comcast-specific references. |
Comments suppressed due to low confidence (1)
CMakeLists.txt:82
CMAKE_OSX_DEPLOYMENT_TARGETdefault is set to "26.0", which is not a valid/known macOS deployment target and is likely to break Xcode/clang builds ("unsupported OS version"). Consider defaulting to the previously used value and still allowing callers to override via-DCMAKE_OSX_DEPLOYMENT_TARGET.
if(NOT DEFINED CMAKE_OSX_DEPLOYMENT_TARGET)
set(CMAKE_OSX_DEPLOYMENT_TARGET "26.0")
endif()
Comment on lines
+36
to
+39
| // The following vendor-specific CMCD extensions use a comcast.com reverse-domain | ||
| // prefix per CTA-5004 convention. Although CMCD is intended to be operator-agnostic, | ||
| // these keys are currently deployed as-is and changing them would be a breaking | ||
| // wire-format change for any server-side infrastructure consuming them. |
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.
Reason for Change: while pushing an unrelated review, "comcast" present in aamp repo was flagged as a forbidden word