fix(crash): Fix crash on attaching metadata output when loading items…

… too quickly (#68)
doublesymmetry · Oct 20, 2023 · c03c830 · c03c830
1 parent 0c87d24
commit c03c830
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 8 deletions.
diff --git a/Sources/SwiftAudioEx/Observer/AVPlayerItemObserver.swift b/Sources/SwiftAudioEx/Observer/AVPlayerItemObserver.swift
@@ -61,22 +61,32 @@ class AVPlayerItemObserver: NSObject {
      */
     func startObserving(item: AVPlayerItem) {
         stopObservingCurrentItem()
-        isObserving = true
-        observingItem = item
+
+        self.isObserving = true
+        self.observingItem = item
         item.addObserver(self, forKeyPath: AVPlayerItemKeyPath.duration, options: [.new], context: &AVPlayerItemObserver.context)
         item.addObserver(self, forKeyPath: AVPlayerItemKeyPath.loadedTimeRanges, options: [.new], context: &AVPlayerItemObserver.context)
         item.addObserver(self, forKeyPath: AVPlayerItemKeyPath.playbackLikelyToKeepUp, options: [.new], context: &AVPlayerItemObserver.context)
-        item.add(metadataOutput)
+
+        // We must slightly delay adding the metadata output due to the fact that
+        // stop observation is not a synchronous action and metadataOutput may not
+        // be removed from last item before we try to attach it to a new one.
+        DispatchQueue.main.asyncAfter(deadline: .now() + 0.001) { [weak self] in
+            guard let `self` = self else { return }
+            item.add(self.metadataOutput)
+        }
     }
-
+        
     func stopObservingCurrentItem() {
         guard let observingItem = observingItem, isObserving else {
             return
         }
+
         observingItem.removeObserver(self, forKeyPath: AVPlayerItemKeyPath.duration, context: &AVPlayerItemObserver.context)
         observingItem.removeObserver(self, forKeyPath: AVPlayerItemKeyPath.loadedTimeRanges, context: &AVPlayerItemObserver.context)
         observingItem.removeObserver(self, forKeyPath: AVPlayerItemKeyPath.playbackLikelyToKeepUp, context: &AVPlayerItemObserver.context)
         observingItem.remove(metadataOutput)
+
         isObserving = false
         self.observingItem = nil
     }

diff --git a/Tests/SwiftAudioExTests/AVPlayerItemObserverTests.swift b/Tests/SwiftAudioExTests/AVPlayerItemObserverTests.swift
@@ -28,4 +28,11 @@ class AVPlayerItemObserverTests: XCTestCase {
         observer.startObserving(item: item)
         XCTAssertTrue(observer.isObserving)
     }
+
+    func testObservingInQuickSucccession() {
+        for _ in 0...1000 {
+            let item = AVPlayerItem(url: URL(fileURLWithPath: Source.path))
+            observer.startObserving(item: item)
+        }
+    }
 }
diff --git a/Tests/SwiftAudioExTests/AudioPlayerTests.swift b/Tests/SwiftAudioExTests/AudioPlayerTests.swift
@@ -209,8 +209,6 @@ class AudioPlayerTests: XCTestCase {
     }
 
     func testLoadResourceSucceedsAfterPreviousFailure() {
-        let expectation = XCTestExpectation(description: "Load resource succeeds after previous failure")
-
         var didReceiveFail = false
         listener.onReceiveFail = { error in
             didReceiveFail = true
@@ -230,8 +228,6 @@ class AudioPlayerTests: XCTestCase {
     }
 
     func testLoadResourceSucceedsAfterPreviousFailureWithPlayWhenReady() {
-        let expectation = XCTestExpectation(description: "Load resource with playWhenReady succeeds after previous failure")
-
         var didReceiveFail = false
         listener.onReceiveFail = { error in
             didReceiveFail = true