Added MediaService functionality that allows users to control their media playing via Apple Music using the mpris packet types - #25
Conversation
…or the player name variable, along with some rather scarce comments.
|
oh and i forgot to mention that i had to use apple script to get itunes/apple music to work with the proj. because for some another stupid reason apple doesn't have a swift (or even an objective-c) api for that. |
there gotta be a way to do with for youtube/youtube music within chrome. too bad i suck at programming , ill learn one day. but there must be a way to control chrome now playing with your android phone dont you think . anyway nice workflow man :) |
|
I have to look into this, because currently @sidevesh has an experimental mpris implementation. I haven't been able to get that to work, but as far as I understand, @sidevesh's implementation is for controlling media of remote from Soduto, while @Kanishk951's implementation is for controlling media (only Apple Music) of macOS from remote device, right? Also, could you please clean up the commit messages so they describe what changed and why rather than subjective notes? Since, commit history is useful later for debugging, reviews, and understanding context, and these messages will end up in the permanent history, cleaning them up now would make that a lot easier. You may do an interactive git rebase, and reword those commit messages and force-push when rebasing is complete. Thanks for your help! |
There was a problem hiding this comment.
Pull request overview
This pull request adds MediaService functionality to enable remote control of Apple Music playback from connected devices using the KDE Connect mpris protocol. The implementation uses AppleScript bridge via AppleScriptObjC to communicate with Apple Music.
Changes:
- Adds MediaService with support for play/pause, next/previous track, seek, volume control, and shuffle
- Implements iTunesBridge using AppleScript for Apple Music integration
- Adds required entitlements for Apple Music control and automation
- Integrates SwiftyJSON dependency (though unused in final code)
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 28 comments.
Show a summary per file
| File | Description |
|---|---|
| Soduto/Services/MediaService/MediaService.swift | Core service implementation with packet handling and media control logic |
| Soduto/Services/MediaService/iTunesBridge.swift | Swift protocol/interface for AppleScript bridge |
| Soduto/Services/MediaService/iTunesBridge.applescript | AppleScript implementation for Apple Music control |
| Soduto/Soduto.entitlements | Adds Music control entitlements, removes addressbook entitlement |
| Soduto/Info.plist | Adds NSAppleEventsUsageDescription for user permission |
| Soduto/AppDelegate.swift | Registers MediaService with service manager |
| Soduto.xcodeproj/project.pbxproj | Adds new files, SwiftyJSON dependency, changes team IDs and security settings |
| SharedUserDefaults.swift | Changes suite name to different team ID |
| README | Adds unrelated SwiftAutomation documentation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| set (newPos) { | ||
| if let iTunesBridge = iTunesBridge { | ||
|
|
||
| print("seeking to \(newPos)") |
There was a problem hiding this comment.
Debug print statements should be removed or replaced with proper logging using the Log framework that's used elsewhere in the codebase (e.g., Log.debug?.message()).
| print("seeking to \(newPos)") | |
| Log.debug?.message("Seeking to \(newPos)") |
| print("track duration is \(totalTime)") | ||
| var isPlaying: Bool = true | ||
|
|
||
| playerState = iTunesBridge?.playerState as? PlayerState ?? .stopped | ||
|
|
||
| switch playerState { | ||
| case .paused: isPlaying = false | ||
| case .stopped: isPlaying = false | ||
| default: break | ||
| // since now playing is already true we dont need to change a thing | ||
| } | ||
|
|
||
| print(info as Any) | ||
|
|
||
| print("pos is \(pos)") |
There was a problem hiding this comment.
Debug print statements should be removed or replaced with proper logging using the Log framework that's used elsewhere in the codebase (e.g., Log.debug?.message()).
| artist = info["trackArtist"] as? String ?? "" | ||
|
|
||
|
|
||
| let totalTime: Double = ((iTunesBridge?.trackDuration as! Double)) * 1000 |
There was a problem hiding this comment.
This force unwrap with force cast could crash if iTunesBridge.trackDuration is nil or not a Double. Consider using safer unwrapping with proper error handling.
| let totalTime: Double = ((iTunesBridge?.trackDuration as! Double)) * 1000 | |
| var totalTime: Double = 0 | |
| if let duration = iTunesBridge?.trackDuration as? Double { | |
| totalTime = duration * 1000 | |
| } |
| static let logLevelConfigurationKey = "com.soduto.logLevel" | ||
|
|
||
| override init() { | ||
|
|
There was a problem hiding this comment.
There's an unnecessary empty line between the opening brace and the code. This should be removed for consistency with the rest of the codebase.
| return 0 | ||
| } | ||
| set (newVol) { | ||
| if let iTunesBridge = iTunesBridge { iTunesBridge.soundVolume = newVol as NSNumber |
There was a problem hiding this comment.
The line is missing formatting with proper spacing before iTunesBridge.soundVolume. The line appears broken and should have proper indentation.
| if let iTunesBridge = iTunesBridge { iTunesBridge.soundVolume = newVol as NSNumber | |
| if let iTunesBridge = iTunesBridge { | |
| iTunesBridge.soundVolume = newVol as NSNumber |
| 843E178A1E26B613001D0444 /* Embed Login Items */ = { | ||
| isa = PBXCopyFilesBuildPhase; | ||
| buildActionMask = 2147483647; | ||
| buildActionMask = 12; |
There was a problem hiding this comment.
The buildActionMask has been changed from 2147483647 to 12. This value typically should be 2147483647 (which equals 2^31 - 1) to indicate "all actions". Changing it to 12 may prevent certain build phases from running. This change should be reverted unless there's a specific reason for it.
| buildActionMask = 12; | |
| buildActionMask = 2147483647; |
| <key>com.apple.security.automation.apple-events</key> | ||
| <true/> |
There was a problem hiding this comment.
The entitlement key 'com.apple.security.personal-information.addressbook' has been removed from the entitlements file. However, the codebase has existing functionality that accesses contacts (e.g., SendMessageWindowController.swift uses CNContactStore). Removing this entitlement will break that existing functionality. This entitlement should be retained.
| // basically the player actually playing the audio. | ||
| } | ||
|
|
||
| // MARK: Struct Decleration |
There was a problem hiding this comment.
The spelling "Decleration" is incorrect. It should be "Declaration".
| // MARK: Struct Decleration | |
| // MARK: Struct Declaration |
| import MediaPlayer | ||
| import SwiftyJSON | ||
|
|
There was a problem hiding this comment.
The MediaPlayer import is unused and should be removed. The code doesn't use any MediaPlayer framework functionality.
| import MediaPlayer | |
| import SwiftyJSON | |
| import SwiftyJSON |
| CURRENT_PROJECT_VERSION = 18; | ||
| DEVELOPMENT_TEAM = P8CGT2P7HB; | ||
| DEVELOPMENT_TEAM = 73583QR2DF; | ||
| ENABLE_HARDENED_RUNTIME = NO; |
There was a problem hiding this comment.
The Soduto target has ENABLE_HARDENED_RUNTIME explicitly disabled for both Debug and Release, which removes macOS hardened runtime protections (e.g., library validation and certain code injection mitigations) from the main app. An attacker who can influence the local environment (e.g., via malicious injected libraries or tooling) will have a much easier time modifying the running process or abusing its entitlements when hardened runtime is off. Consider re-enabling hardened runtime for the main app (especially for Release builds) and only relaxing it in narrowly scoped debug or helper targets if absolutely required.
| ENABLE_HARDENED_RUNTIME = NO; | |
| ENABLE_HARDENED_RUNTIME = YES; |
Currently only allows for one way (device to mac) control tho. Furthermore, it is only limited to Apple Music because for some stupid reason Apple doesn't have a universal media API, and what they do have is a hot mess that goes a tower up my smooth brain's mess-resolving capabilities. Anyway, please let me know if there's anything (which I'm pretty sure is) I can change to make it better and suit the overarching guidelines of the project. Additionally, please forgive me for, and lemme know of, any braindead piece of code that I might have written while undertaking this proj. (coffee at 2 am can only take you so far).