refactor: devicekit h264 goes to another repo#46
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR removes the ReplayKit broadcast extension target and all associated build wiring and Swift package dependencies for H264/Opus, deletes H264 streaming test utilities, replaces the LICENSE with Functional Source License v1.1 (with a future Apache‑2.0 grant), and updates SwiftLint and README license text. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
LICENSE (1)
1-46:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftMajor license change needs immediate alignment + user notification.
This PR replaces the Apache License 2.0 with Functional Source License v1.1, which adds significant commercial restrictions via the “Competing Use” prohibition, with Apache 2.0 only becoming effective on the 2nd anniversary (“Grant of Future License”).
The repo documentation is currently inconsistent:
README.mdstates “DeviceKit iOS is released under the Apache 2.0 License” while the top-levelLICENSEreflects the temporary FSL restrictions first.Key implications:
- Existing users updating now may face new restrictions during the FSL period.
- “Competing Use” (Permitted Purpose) is broad and could affect legitimate commercial deployments.
- Even if Apache becomes available later, the interim terms are materially different.
Please ensure:
README.mdand any other distribution/license statements are updated to reflect the actual FSL terms + delayed Apache 2.0 availability.- The copyright holder (“Mobile Next HQ, Inc.”) is correct and the relevant stakeholders agreed to the license change.
- Existing users/dependents were notified about the Apache → FSL transition and the interim restrictive period.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@LICENSE` around lines 1 - 46, Update repository docs and communications to reflect the license change from Apache 2.0 to Functional Source License v1.1: revise README.md and any top-level or package-level license headers to state that the project is currently licensed under the Functional Source License v1.1 with Apache 2.0 becoming available after the two‑year “Grant of Future License”; explicitly call out the “Competing Use” restrictions and interim limitations; confirm and document that the copyright holder "Mobile Next HQ, Inc." is correct and that authorized stakeholders have approved the change; and prepare a user-facing notification (release note/email) informing existing users and downstream consumers about the interim FSL terms and the delayed Apache 2.0 availability.
🧹 Nitpick comments (1)
LICENSE (1)
44-45: ⚡ Quick winDocument the Software's "initial availability" date for clarity.
The Grant of Future License references "the second anniversary of the Software's initial availability" but doesn't specify that date. For legal clarity and to help users understand when Apache 2.0 becomes available, consider documenting this date explicitly, either in the license file or in a prominent location like the README.
This is especially important for:
- Users evaluating whether to wait for Apache 2.0
- Compliance teams tracking license transitions
- Future maintainers managing the license conversion
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@LICENSE` around lines 44 - 45, Update the "Grant of Future License" language to specify the Software's "initial availability" date explicitly: replace or annotate the phrase "the Software's initial availability" in the LICENSE's "Grant of Future License" section with the concrete calendar date (e.g., "initial availability: YYYY-MM-DD") or add a clear footnote linking to a README entry; ensure the LICENSE's "Grant of Future License" header and text are updated accordingly and mirror the same explicit date in the README or another prominent project document so users and compliance teams can determine when the Apache License, Version 2.0 becomes available.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@LICENSE`:
- Around line 1-46: Update repository docs and communications to reflect the
license change from Apache 2.0 to Functional Source License v1.1: revise
README.md and any top-level or package-level license headers to state that the
project is currently licensed under the Functional Source License v1.1 with
Apache 2.0 becoming available after the two‑year “Grant of Future License”;
explicitly call out the “Competing Use” restrictions and interim limitations;
confirm and document that the copyright holder "Mobile Next HQ, Inc." is correct
and that authorized stakeholders have approved the change; and prepare a
user-facing notification (release note/email) informing existing users and
downstream consumers about the interim FSL terms and the delayed Apache 2.0
availability.
---
Nitpick comments:
In `@LICENSE`:
- Around line 44-45: Update the "Grant of Future License" language to specify
the Software's "initial availability" date explicitly: replace or annotate the
phrase "the Software's initial availability" in the LICENSE's "Grant of Future
License" section with the concrete calendar date (e.g., "initial availability:
YYYY-MM-DD") or add a clear footnote linking to a README entry; ensure the
LICENSE's "Grant of Future License" header and text are updated accordingly and
mirror the same explicit date in the README or another prominent project
document so users and compliance teams can determine when the Apache License,
Version 2.0 becomes available.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a26f48e5-adcf-4672-9307-c2f01f977743
📒 Files selected for processing (29)
.gitmodulesBroadcastUploadExtension/CGRect+Extension.swiftBroadcastUploadExtension/CMSampleBuffer+Extension.swiftBroadcastUploadExtension/Info.plistBroadcastUploadExtension/SampleHandler.swiftBroadcastUploadExtension/ScreenStreamer.swiftBroadcastUploadExtension/TCPServer.swiftDeviceKitTests/H264Stream/ScreenshotH264Stream.swiftDeviceKitTests/H264Stream/UIImageScaler.swiftDeviceKitTests/Streamer/H264/H264.swiftDeviceKitTests/Streamer/H264/H264FrameProducer.swiftLICENSEdevicekit-ios.xcodeproj/project.pbxprojh264-codec/.gitignoreh264-codec/Package.swifth264-codec/Sources/h264-codec/CGImage+Extension.swifth264-codec/Sources/h264-codec/CIImage+Extension.swifth264-codec/Sources/h264-codec/CMSampleBuffer+Extension.swifth264-codec/Sources/h264-codec/CVImageBuffer+Extension.swifth264-codec/Sources/h264-codec/H264Encoder.swifth264-codec/Tests/h264-codecTests/h264_codecTests.swiftopus-codec/.gitignoreopus-codec/Package.swiftopus-codec/Sources/Copusopus-codec/Sources/OpusCodec/AudioFormatConverter.swiftopus-codec/Sources/OpusCodec/OpusAudioEncoder.swiftopus-codec/Sources/OpusEncoder/OpusEncoder.copus-codec/Sources/OpusEncoder/include/OpusEncoder.hopus-codec/Tests/opus-codecTests/opus_codecTests.swift
💤 Files with no reviewable changes (28)
- opus-codec/.gitignore
- BroadcastUploadExtension/Info.plist
- h264-codec/Tests/h264-codecTests/h264_codecTests.swift
- h264-codec/Sources/h264-codec/CGImage+Extension.swift
- DeviceKitTests/H264Stream/UIImageScaler.swift
- h264-codec/Sources/h264-codec/CVImageBuffer+Extension.swift
- opus-codec/Sources/OpusEncoder/OpusEncoder.c
- DeviceKitTests/Streamer/H264/H264FrameProducer.swift
- BroadcastUploadExtension/SampleHandler.swift
- BroadcastUploadExtension/ScreenStreamer.swift
- .gitmodules
- opus-codec/Sources/OpusCodec/OpusAudioEncoder.swift
- h264-codec/Sources/h264-codec/H264Encoder.swift
- opus-codec/Tests/opus-codecTests/opus_codecTests.swift
- opus-codec/Sources/OpusEncoder/include/OpusEncoder.h
- opus-codec/Package.swift
- h264-codec/Sources/h264-codec/CIImage+Extension.swift
- opus-codec/Sources/OpusCodec/AudioFormatConverter.swift
- BroadcastUploadExtension/CMSampleBuffer+Extension.swift
- opus-codec/Sources/Copus
- DeviceKitTests/Streamer/H264/H264.swift
- BroadcastUploadExtension/TCPServer.swift
- h264-codec/.gitignore
- h264-codec/Sources/h264-codec/CMSampleBuffer+Extension.swift
- DeviceKitTests/H264Stream/ScreenshotH264Stream.swift
- BroadcastUploadExtension/CGRect+Extension.swift
- devicekit-ios.xcodeproj/project.pbxproj
- h264-codec/Package.swift
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
README.md (3)
214-216:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winUpdate architecture diagram to reflect removed components.
The architecture section lists
BroadcastUploadExtension/,h264-codec/, andopus-codec/directories, but these are being removed in this PR. The diagram should reflect the updated repository structure.📝 Proposed fix
DeviceKitTests/ # XCUITest runner (automation server) JSONRPC/ # JSON-RPC protocol + 15 method handlers Streamer/ # MJPEG and H264 HTTP streaming XCTest/ # Private API wrappers (touch synthesis, accessibility) - H264Stream/ # Screenshot-based H264 streaming - BroadcastUploadExtension/ # ReplayKit extension (H264 + Opus over TCP) - h264-codec/ # Swift package: H264 encoder (VideoToolbox) - opus-codec/ # Swift package: Opus encoder (wraps libopus)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 214 - 216, The architecture diagram and surrounding text still list removed components (BroadcastUploadExtension, h264-codec, opus-codec); update the README.md architecture section and the diagram to remove those three entries and adjust any arrows/labels and explanatory text to reflect the new repository structure and remaining components (e.g., update the diagram asset and the paragraph that enumerates top-level directories so references to BroadcastUploadExtension, h264-codec, and opus-codec are deleted or replaced with current modules).
180-187:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winRemove the entire Broadcast Extension section.
This section documents the ReplayKit broadcast extension feature including TCP ports for H264/Opus streams. Since the PR removes the
BroadcastUploadExtensiontarget and all codec dependencies, this entire section should be deleted.📝 Proposed fix
-### Broadcast Extension - -The ReplayKit broadcast extension provides system-level screen and audio capture over TCP, independent of the JSON-RPC server. - -| Port | Stream | -|------|--------| -| 12005 | H264 video | -| 12006 | Opus audio | -🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 180 - 187, Delete the entire "Broadcast Extension" section from the README (the "### Broadcast Extension" header and the following table describing ports 12005/12006 and H264/Opus streams) because the PR removes the BroadcastUploadExtension target and related codec dependencies; remove any references to the broadcast extension elsewhere in README.md to keep docs consistent with the codebase.
32-32:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winRemove broadcast extension feature from documentation.
This line documents the broadcast audio/video feature, but the PR removes the
BroadcastUploadExtensiontarget and all related code. This feature is no longer available and should be removed from the feature list.📝 Proposed fix
-- **Broadcast audio/video** — ReplayKit extension with H264 video and Opus audio over TCP🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` at line 32, Remove the obsolete "Broadcast audio/video — ReplayKit extension with H264 video and Opus audio over TCP" feature line from the README and any references to the BroadcastUploadExtension; update the feature list to exclude broadcast extension mentions so documentation matches the code removal of the BroadcastUploadExtension target.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@README.md`:
- Around line 214-216: The architecture diagram and surrounding text still list
removed components (BroadcastUploadExtension, h264-codec, opus-codec); update
the README.md architecture section and the diagram to remove those three entries
and adjust any arrows/labels and explanatory text to reflect the new repository
structure and remaining components (e.g., update the diagram asset and the
paragraph that enumerates top-level directories so references to
BroadcastUploadExtension, h264-codec, and opus-codec are deleted or replaced
with current modules).
- Around line 180-187: Delete the entire "Broadcast Extension" section from the
README (the "### Broadcast Extension" header and the following table describing
ports 12005/12006 and H264/Opus streams) because the PR removes the
BroadcastUploadExtension target and related codec dependencies; remove any
references to the broadcast extension elsewhere in README.md to keep docs
consistent with the codebase.
- Line 32: Remove the obsolete "Broadcast audio/video — ReplayKit extension with
H264 video and Opus audio over TCP" feature line from the README and any
references to the BroadcastUploadExtension; update the feature list to exclude
broadcast extension mentions so documentation matches the code removal of the
BroadcastUploadExtension target.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d39b92f7-0ec2-40d2-a960-bd9bcb2a4600
📒 Files selected for processing (2)
.swiftlint.ymlREADME.md
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/build.yml:
- Line 32: The CI change removed the lint gate: reintroduce a lint job in
.github/workflows/build.yml (name it lint) that runs SwiftLint (or run `make
lint`) and make build-ipa and build-sim need: [trivy, lint] so lint must pass
before builds, or alternatively update README.md and the workflow to document
the intentional removal of CI linting (remove references to `make lint` and
.swiftlint.yml or add an explicit rationale comment) — modify the workflow jobs
`lint`, `build-ipa`, and `build-sim` accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b5672246-8fba-4345-9ff7-13669fbc40c6
📒 Files selected for processing (1)
.github/workflows/build.yml
|
|
||
| build-ipa: | ||
| needs: [lint, trivy] | ||
| needs: [trivy] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if lint job still exists in the workflow
echo "Checking for lint job in workflow:"
if grep -q "^ lint:" .github/workflows/build.yml; then
echo "✓ Lint job still exists"
grep -A 10 "^ lint:" .github/workflows/build.yml
else
echo "✗ Lint job was removed from workflow"
fi
# Show that linting is still configured
echo ""
echo "SwiftLint configuration still exists:"
fd -t f "\.swiftlint\.yml$" --exec head -15
echo ""
echo "Lint target still documented:"
rg -n "make lint" README.md MakefileRepository: mobile-next/devicekit-ios
Length of output: 260
Restore or explicitly justify SwiftLint CI enforcement
- In
.github/workflows/build.yml,build-ipa/build-simnow onlyneedtrivy, and there is nolintjob in this workflow—so SwiftLint won’t fail CI. .swiftlint.ymlstill exists andREADME.mdstill documentsmake lint.
Reintroduce a lint job (and gate build-ipa/build-sim on it) or update docs/workflow with a clear rationale for intentionally removing the CI lint gate.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/build.yml at line 32, The CI change removed the lint gate:
reintroduce a lint job in .github/workflows/build.yml (name it lint) that runs
SwiftLint (or run `make lint`) and make build-ipa and build-sim need: [trivy,
lint] so lint must pass before builds, or alternatively update README.md and the
workflow to document the intentional removal of CI linting (remove references to
`make lint` and .swiftlint.yml or add an explicit rationale comment) — modify
the workflow jobs `lint`, `build-ipa`, and `build-sim` accordingly.
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
1 participant
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.
No description provided.