-
Notifications
You must be signed in to change notification settings - Fork 3k
feat: enhance source loading #4755
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
Conversation
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.
Pull Request Overview
This PR implements task cancellation support for video source loading operations to prevent race conditions and memory issues when rapidly switching video sources or reinitializing players. The changes introduce a SourceLoader class on both iOS and Android platforms that manages concurrent operations by cancelling previous pending operations when new ones are started.
- Added
SourceLoaderutility classes for iOS (Swift) and Android (Kotlin) to manage and cancel pending source loading operations - Integrated cancellation handling in video player and source initialization/loading flows
- Added new error types (
cancelled) for bothPlayerErrorandSourceErrorto handle cancellation scenarios - Updated memory size calculation logic for iOS to provide more accurate estimates based on file size and bitrate
Reviewed Changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
packages/react-native-video/src/core/VideoPlayer.ts |
Refactored updateMemorySize into a private method and added calls in replaceSourceAsync to update memory before and after source replacement |
packages/react-native-video/ios/hybrids/VideoPlayerSource/SourceLoader.swift |
New file implementing actor-based source loading with cancellation support for iOS |
packages/react-native-video/ios/hybrids/VideoPlayerSource/HybridVideoPlayerSource.swift |
Integrated SourceLoader to wrap asset loading operations with cancellation support and handle cancellation errors |
packages/react-native-video/ios/hybrids/VideoPlayer/HybridVideoPlayer.swift |
Added SourceLoader integration in initialization and source replacement, with proper cancellation and cleanup of old sources |
packages/react-native-video/ios/core/VideoError.swift |
Added cancelled error cases to PlayerError and SourceError enums |
packages/react-native-video/ios/core/Extensions/AVAsset+estimatedMemoryUsage.swift |
Rewrote memory estimation logic to use file size for local files and bitrate-based calculation for streaming content |
packages/react-native-video/android/src/main/java/com/twg/video/hybrids/videoplayersource/HybridVideoPlayerSource.kt |
Added SourceLoader instance and wrapped asset information loading |
packages/react-native-video/android/src/main/java/com/twg/video/hybrids/videoplayer/HybridVideoPlayer.kt |
Added cancellation of old source loader when replacing sources |
packages/react-native-video/android/src/main/java/com/twg/video/core/utils/SourceLoader.kt |
New file implementing atomic operation tracking with cancellation support for Android |
packages/react-native-video/android/src/main/java/com/twg/video/core/VideoError.kt |
Added Cancelled error objects to PlayerError and SourceError sealed classes |
example/src/App.tsx |
Added error handling with try-catch around replaceSourceAsync call and reordered imports |
example/ios/Podfile.lock |
Updated React Native dependencies from 0.77.2 to 0.77.3 and other related version bumps |
Comments suppressed due to low confidence (1)
packages/react-native-video/ios/core/Extensions/AVAsset+estimatedMemoryUsage.swift:1
- Corrected spelling of 'browned' to 'borrowed'.
//
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if error is CancellationError { | ||
| throw error | ||
| } |
Copilot
AI
Nov 2, 2025
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.
Lines 48-51 contain redundant logic. Both branches throw the same error. Consider removing the conditional check and just throwing the error directly, or clarify the intent if different handling was planned.
| if error is CancellationError { | |
| throw error | |
| } |
Summary