-
Notifications
You must be signed in to change notification settings - Fork 138
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix various iOS app startup issues #386
Conversation
@@ -34,7 +34,7 @@ | |||
504EC30F1FED79650016851F /* Assets.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = 504EC30E1FED79650016851F /* Assets.xcassets */; }; | |||
504EC3121FED79650016851F /* LaunchScreen.storyboard in Resources */ = {isa = PBXBuildFile; fileRef = 504EC3101FED79650016851F /* LaunchScreen.storyboard */; }; | |||
50B271D11FEDC1A000F3C39B /* public in Resources */ = {isa = PBXBuildFile; fileRef = 50B271D01FEDC1A000F3C39B /* public */; }; | |||
A084ECDBA7D38E1E42DFC39D /* Pods_App.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = AF277DCFFFF123FFC6DF26C7 /* Pods_App.framework */; }; | |||
AC98F95F300E46A27FAE47A4 /* Pods_Audiobookshelf.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = B775656E49492CFD6763B7B6 /* Pods_Audiobookshelf.framework */; }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The target changed in #371, but I missed updating the Pods config for the new target.
@@ -85,10 +85,10 @@ class AppDelegate: UIResponder, UIApplicationDelegate { | |||
override func touchesBegan(_ touches: Set<UITouch>, with event: UIEvent?) { | |||
super.touchesBegan(touches, with: event) | |||
|
|||
let statusBarRect = UIApplication.shared.statusBarFrame | |||
let statusBarRect = self.window?.windowScene?.statusBarManager?.statusBarFrame |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixes a window access deprecation with iOS 13+
@@ -7,7 +7,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate { | |||
|
|||
private let logger = AppLogger(category: "AppDelegate") | |||
|
|||
var window: UIWindow? | |||
lazy var window: UIWindow? = UIWindow(frame: UIScreen.main.bounds) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixes a warning about accessing window
too early
@@ -14,7 +14,6 @@ public class AbsAudioPlayer: CAPPlugin { | |||
private let logger = AppLogger(category: "AbsAudioPlayer") | |||
|
|||
private var initialPlayWhenReady = false | |||
private var isUIReady = false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused variable removed. I added this in a previous PR and never removed it.
// We don't need to restore if we have an active session | ||
guard PlayerHandler.getPlaybackSession() == nil else { return } | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We actually do need to restore, in the case of the WebKit reloading and existing playback session is already in the player.
if let activeSession = activeSession { | ||
PlayerHandler.stopPlayback(currentSessionId: activeSession.id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When reloading the session, we should stop playback so audio doesn't play while the next line's network connection is made.
This is an issue because the Vue code will change playback rate during startup, which will unpause the audio. By stopping playback, changing the playback rate ensures audio doesn't start playing.
@@ -17,7 +17,7 @@ def capacitor_pods | |||
pod 'CapacitorStorage', :path => '../../node_modules/@capacitor/storage' | |||
end | |||
|
|||
target 'App' do | |||
target 'Audiobookshelf' do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The target changed in #371, but I missed updating the Pods config for the new target.
self.stopPausedTimer() | ||
self.removeSleepTimer() | ||
self.removeTimeObserver() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These needed to be moved to destroy()
, as when setting player = nil
, removeSleepTimer
will throw an event that causes PlayerHandler.player
to be read, which is causes an access crash due to a mutated variable being read.
|
||
// Remove timers | ||
self.stopPausedTimer() | ||
self.removeSleepTimer() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move these from deinit
.
guard completed else { return } | ||
self?.resumePlayback() | ||
|
||
DispatchQueue.runOnMainQueue { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actions that modify the audioPlayer
must be on the main queue. This was the case 99% of the time before, but by adding this code, we are being explicit it's happening.
player?.destroy() | ||
player = nil | ||
} | ||
resetPlayer() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor refactor as I was tracking downing the player = nil
access error.
@@ -31,15 +28,11 @@ class PlayerHandler { | |||
player = AudioPlayer(sessionId: sessionId, playWhenReady: playWhenReady, playbackRate: playbackRate) | |||
} | |||
|
|||
public static func stopPlayback() { | |||
public static func stopPlayback(currentSessionId: String? = nil) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New parameter ensures the current playback session does not get invalidated during restore, which would cause it not be restored the next time.
This PR addresses a few different issues related to app startup:
window
too early