feat(shutdown): implement graceful shutdown for camera session #3650
Conversation
- Add shutdown method to stop all outputs and release resources - Schedule shutdown with warning if it takes longer than expected
|
@EthanDM is attempting to deploy a commit to the mrousavy's Team Team on Vercel. A member of the Team first needs to authorize it. |
mrousavy
left a comment
mrousavy
left a comment
There was a problem hiding this comment.
Thanks for this PR - seems like there were some issues with proper session shutdown on Fabric - I wonder why that wasn't a problem before on Paper. Maybe view caching/recycling?
Anyways - I left some review comments
| try lambda(config) | ||
| } catch CameraConfiguration.AbortThrow.abort { | ||
| // call has been aborted and changes shall be discarded | ||
| completionBlock?() |
There was a problem hiding this comment.
instead of writing that 3 times, put it in a defer { ... } block so we dont forget it when we add a fourth exit point
| deinit { | ||
| shutdownCameraSession() | ||
| } |
There was a problem hiding this comment.
Putting stuff like this in deinit is often a bad idea, especially if it runs an asynchronous operation.
Can you please test if that is absolutely necessary? Remove it if it isn't - we are calling shutdownCameraSession() at the top anyways.
| let slowShutdownWarning = DispatchWorkItem { | ||
| VisionLogger.log(level: .warning, message: "CameraSession shutdown is still running after 2 seconds.") | ||
| } | ||
| CameraQueues.cameraQueue.asyncAfter(deadline: .now() + .seconds(2), execute: slowShutdownWarning) | ||
|
|
||
| cameraSession.shutdown { [weak self] in | ||
| slowShutdownWarning.cancel() |
There was a problem hiding this comment.
| let slowShutdownWarning = DispatchWorkItem { | |
| VisionLogger.log(level: .warning, message: "CameraSession shutdown is still running after 2 seconds.") | |
| } | |
| CameraQueues.cameraQueue.asyncAfter(deadline: .now() + .seconds(2), execute: slowShutdownWarning) | |
| cameraSession.shutdown { [weak self] in | |
| slowShutdownWarning.cancel() | |
| let logSlowShutdownWarning = DispatchWorkItem { | |
| VisionLogger.log(level: .warning, message: "CameraSession shutdown is still running after 2 seconds.") | |
| } | |
| CameraQueues.cameraQueue.asyncAfter(deadline: .now() + .seconds(2), execute: logSlowShutdownWarning) | |
| cameraSession.shutdown { [weak self] in | |
| logSlowShutdownWarning.cancel() |
| if let metadataOutput = output as? AVCaptureMetadataOutput { | ||
| metadataOutput.setMetadataObjectsDelegate(nil, queue: nil) | ||
| } | ||
| if let videoOutput = output as? AVCaptureVideoDataOutput { | ||
| videoOutput.setSampleBufferDelegate(nil, queue: nil) | ||
| } |
There was a problem hiding this comment.
I'm 99% sure this is unnecessary. The outputs store the delegates as weak references, and there's no Thread hops involved.
Can you try to remove this code and check if it still works fine for you? I'd rather have that out tbh because it is super specific and feels like monkeypatching
3 participants
What
Fix the navigation freeze on React Navigation/Fabric by shutting the camera session down deterministically when the view unmounts.
Changes
Tested on
Related issues