rasmuslos / AmpFin

Native Jellyfin music player for iOS & iPadOS
Other
107 stars 10 forks source link

fix: add mac catalyst specific workarounds #33

Closed gnattu closed 3 months ago

gnattu commented 3 months ago

The Mac Catalyst target has some behavioral differences due to UIKit being replaced by AppKit in some instances. Additionally, it has different conventions for controls, such as the volume control. On iOS, the volume control is expected to be system-wide, but on macOS, it is expected to be app-specific. To match the iOS App's appearance, the titlebar also needs to be hidden, but this cannot be done with SwiftUI and has to use UIKit methods which is weird.

This PR addressed most outstanding issues I found during my usage, but there could be more. If you found more feel free to tell me and I'm going to look at them.

gnattu commented 3 months ago

https://github.com/rasmuslos/AmpFin/blob/e256426604173b2c875cbb151e3e2cdef34ff61e/AmpFinKit/Sources/AFPlayback/LocalAudioEndpoint/LocalAudioEndpoint%2BHelper.swift#L71

Also, during debugging I found this line keep querying the system volume periodically but this code will introduce inter-process communication as the system volume can only be fetched this way, and such fetching could happen in the main thread which will potentially introduce unwanted delay and Xcode warned me about this. Can we just use the cached in-process volume inside the LocalAudioEndpoint instead?

rasmuslos commented 3 months ago

https://github.com/rasmuslos/AmpFin/blob/e256426604173b2c875cbb151e3e2cdef34ff61e/AmpFinKit/Sources/AFPlayback/LocalAudioEndpoint/LocalAudioEndpoint%2BHelper.swift#L71

Also, during debugging I found this line keep querying the system volume periodically but this code will introduce inter-process communication as the system volume can only be fetched this way, and such fetching could happen in the main thread which will potentially introduce unwanted delay and Xcode warned me about this. Can we just use the cached in-process volume inside the LocalAudioEndpoint instead?

I think this was added before multiple endpoints and the volume variable were around, so it should be safe to do

rasmuslos commented 3 months ago

Thanks!