From 6f9eff6b86d6d361d832156b82d971b55857c2a8 Mon Sep 17 00:00:00 2001 From: Krzysztof Moch Date: Mon, 25 Mar 2024 09:50:32 +0100 Subject: [PATCH 1/2] fix: remove `setNativeProps` usage --- .../exoplayer/ReactExoplayerViewManager.java | 6 -- .../brentvatne/react/VideoManagerModule.java | 38 -------- .../brentvatne/react/VideoManagerModule.kt | 59 ++++++++++++ ios/Video/RCTVideoManager.m | 3 +- ios/Video/RCTVideoManager.swift | 91 +++++++++---------- src/Video.tsx | 20 ++-- src/specs/VideoNativeComponent.ts | 1 + 7 files changed, 118 insertions(+), 100 deletions(-) delete mode 100644 android/src/main/java/com/brentvatne/react/VideoManagerModule.java create mode 100644 android/src/main/java/com/brentvatne/react/VideoManagerModule.kt diff --git a/android/src/main/java/com/brentvatne/exoplayer/ReactExoplayerViewManager.java b/android/src/main/java/com/brentvatne/exoplayer/ReactExoplayerViewManager.java index 3b2e8c5fa9..27b3f99dfc 100644 --- a/android/src/main/java/com/brentvatne/exoplayer/ReactExoplayerViewManager.java +++ b/android/src/main/java/com/brentvatne/exoplayer/ReactExoplayerViewManager.java @@ -70,7 +70,6 @@ public class ReactExoplayerViewManager extends ViewGroupManager { - View view = manager.resolveView(reactTag); - - if (view instanceof ReactExoplayerView) { - ReactExoplayerView videoView = (ReactExoplayerView) view; - videoView.setPausedModifier(paused); - } - }); - } -} diff --git a/android/src/main/java/com/brentvatne/react/VideoManagerModule.kt b/android/src/main/java/com/brentvatne/react/VideoManagerModule.kt new file mode 100644 index 0000000000..2680756b68 --- /dev/null +++ b/android/src/main/java/com/brentvatne/react/VideoManagerModule.kt @@ -0,0 +1,59 @@ +package com.brentvatne.react + +import com.brentvatne.common.toolbox.ReactBridgeUtils +import com.brentvatne.exoplayer.ReactExoplayerView +import com.facebook.react.bridge.ReactApplicationContext +import com.facebook.react.bridge.ReactContextBaseJavaModule +import com.facebook.react.bridge.ReactMethod +import com.facebook.react.bridge.ReadableMap +import com.facebook.react.bridge.UiThreadUtil +import com.facebook.react.uimanager.UIManagerModule +import kotlin.math.roundToInt + +class VideoManagerModule(reactContext: ReactApplicationContext?) : ReactContextBaseJavaModule(reactContext) { + override fun getName(): String { + return REACT_CLASS + } + + private fun performOnPlayerView(reactTag: Int, callback: (ReactExoplayerView?) -> Unit) { + UiThreadUtil.runOnUiThread { + val view = if (BuildConfig.IS_NEW_ARCHITECTURE_ENABLED) { + reactApplicationContext.fabricUIManager?.resolveView( + reactTag + ) + } else { + val uiManager = reactApplicationContext.getNativeModule(UIManagerModule::class.java) + uiManager?.resolveView(reactTag) + } + + if (view is ReactExoplayerView) { + callback(view) + } else { + callback(null) + } + } + } + + @ReactMethod + fun setPlayerPauseState(paused: Boolean?, reactTag: Int) { + performOnPlayerView(reactTag) { + it?.setPausedModifier(paused!!) + } + } + + @ReactMethod + fun seek(info: ReadableMap, reactTag: Int) { + if (!info.hasKey("time")) { + return + } + + val time = ReactBridgeUtils.safeGetInt(info, "time") + performOnPlayerView(reactTag) { + it?.seekTo((time * 1000f).roundToInt().toLong()) + } + } + + companion object { + private const val REACT_CLASS = "VideoManager" + } +} diff --git a/ios/Video/RCTVideoManager.m b/ios/Video/RCTVideoManager.m index de1140ef16..b5856b22d6 100644 --- a/ios/Video/RCTVideoManager.m +++ b/ios/Video/RCTVideoManager.m @@ -28,7 +28,6 @@ @interface RCT_EXTERN_MODULE (RCTVideoManager, RCTViewManager) RCT_EXPORT_VIEW_PROPERTY(ignoreSilentSwitch, NSString); RCT_EXPORT_VIEW_PROPERTY(mixWithOthers, NSString); RCT_EXPORT_VIEW_PROPERTY(rate, float); -RCT_EXPORT_VIEW_PROPERTY(seek, NSDictionary); RCT_EXPORT_VIEW_PROPERTY(fullscreen, BOOL); RCT_EXPORT_VIEW_PROPERTY(fullscreenAutorotate, BOOL); RCT_EXPORT_VIEW_PROPERTY(fullscreenOrientation, NSString); @@ -74,6 +73,8 @@ @interface RCT_EXTERN_MODULE (RCTVideoManager, RCTViewManager) : (RCTPromiseResolveBlock)resolve rejecter : (RCTPromiseRejectBlock)reject) +RCT_EXTERN_METHOD(seek : (NSDictionary*)info reactTag : (nonnull NSNumber*)reactTag) + RCT_EXTERN_METHOD(setLicenseResult : (NSString*)license licenseUrl : (NSString*)licenseUrl reactTag : (nonnull NSNumber*)reactTag) RCT_EXTERN_METHOD(setLicenseResultError : (NSString*)error licenseUrl : (NSString*)licenseUrl reactTag : (nonnull NSNumber*)reactTag) diff --git a/ios/Video/RCTVideoManager.swift b/ios/Video/RCTVideoManager.swift index e2055c022d..94dfcc4c1b 100644 --- a/ios/Video/RCTVideoManager.swift +++ b/ios/Video/RCTVideoManager.swift @@ -11,77 +11,70 @@ class RCTVideoManager: RCTViewManager { return bridge.uiManager.methodQueue } - @objc(save:reactTag:resolver:rejecter:) - func save(options: NSDictionary, reactTag: NSNumber, resolve: @escaping RCTPromiseResolveBlock, reject: @escaping RCTPromiseRejectBlock) { - bridge.uiManager.prependUIBlock { _, viewRegistry in - let view = viewRegistry?[reactTag] - if !(view is RCTVideo) { - RCTLogError("Invalid view returned from registry, expecting RCTVideo, got: %@", String(describing: view)) - } else if let view = view as? RCTVideo { - view.save(options: options, resolve: resolve, reject: reject) + func performOnVideoView(withReactTag reactTag: NSNumber, callback: @escaping (RCTVideo?) -> Void) { + DispatchQueue.main.async { [weak self] in + guard let self else { + callback(nil) + return + } + + guard let view = self.bridge.uiManager.view(forReactTag: reactTag) as? RCTVideo else { + RCTLogError("Invalid view returned from registry, expecting RCTVideo, got: \(String(describing: view))") + callback(nil) + return } + + callback(view) } } + @objc(save:reactTag:resolver:rejecter:) + func save(options: NSDictionary, reactTag: NSNumber, resolve: @escaping RCTPromiseResolveBlock, reject: @escaping RCTPromiseRejectBlock) { + performOnVideoView(withReactTag: reactTag, callback: { videoView in + videoView?.save(options: options, resolve: resolve, reject: reject) + }) + } + + @objc(seek:reactTag:) + func seek(info: NSDictionary, reactTag: NSNumber) { + performOnVideoView(withReactTag: reactTag, callback: { videoView in + videoView?.setSeek(info) + }) + } + @objc(setLicenseResult:licenseUrl:reactTag:) func setLicenseResult(license: NSString, licenseUrl: NSString, reactTag: NSNumber) { - bridge.uiManager.prependUIBlock { _, viewRegistry in - let view = viewRegistry?[reactTag] - if !(view is RCTVideo) { - RCTLogError("Invalid view returned from registry, expecting RCTVideo, got: %@", String(describing: view)) - } else if let view = view as? RCTVideo { - view.setLicenseResult(license as String, licenseUrl as String) - } - } + performOnVideoView(withReactTag: reactTag, callback: { videoView in + videoView?.setLicenseResult(license as String, licenseUrl as String) + }) } @objc(setLicenseResultError:licenseUrl:reactTag:) func setLicenseResultError(error: NSString, licenseUrl: NSString, reactTag: NSNumber) { - bridge.uiManager.prependUIBlock { _, viewRegistry in - let view = viewRegistry?[reactTag] - if !(view is RCTVideo) { - RCTLogError("Invalid view returned from registry, expecting RCTVideo, got: %@", String(describing: view)) - } else if let view = view as? RCTVideo { - view.setLicenseResultError(error as String, licenseUrl as String) - } - } + performOnVideoView(withReactTag: reactTag, callback: { videoView in + videoView?.setLicenseResultError(error as String, licenseUrl as String) + }) } @objc(dismissFullscreenPlayer:) func dismissFullscreenPlayer(_ reactTag: NSNumber) { - bridge.uiManager.prependUIBlock { _, viewRegistry in - let view = viewRegistry?[reactTag] - if !(view is RCTVideo) { - RCTLogError("Invalid view returned from registry, expecting RCTVideo, got: %@", String(describing: view)) - } else if let view = view as? RCTVideo { - view.dismissFullscreenPlayer() - } - } + performOnVideoView(withReactTag: reactTag, callback: { videoView in + videoView?.dismissFullscreenPlayer() + }) } @objc(presentFullscreenPlayer:) func presentFullscreenPlayer(_ reactTag: NSNumber) { - bridge.uiManager.prependUIBlock { _, viewRegistry in - let view = viewRegistry?[reactTag] - if !(view is RCTVideo) { - RCTLogError("Invalid view returned from registry, expecting RCTVideo, got: %@", String(describing: view)) - } else if let view = view as? RCTVideo { - view.presentFullscreenPlayer() - } - } + performOnVideoView(withReactTag: reactTag, callback: { videoView in + videoView?.presentFullscreenPlayer() + }) } @objc(setPlayerPauseState:reactTag:) func setPlayerPauseState(paused: NSNumber, reactTag: NSNumber) { - bridge.uiManager.prependUIBlock { _, viewRegistry in - let view = viewRegistry?[reactTag] - if !(view is RCTVideo) { - RCTLogError("Invalid view returned from registry, expecting RCTVideo, got: %@", String(describing: view)) - } else if let view = view as? RCTVideo { - let paused = paused.boolValue - view.setPaused(paused) - } - } + performOnVideoView(withReactTag: reactTag, callback: { videoView in + videoView?.setPaused(paused.boolValue) + }) } override class func requiresMainQueueSetup() -> Bool { diff --git a/src/Video.tsx b/src/Video.tsx index e496efc26e..60292d0ac2 100644 --- a/src/Video.tsx +++ b/src/Video.tsx @@ -229,17 +229,25 @@ const Video = forwardRef( Platform.select({ ios: () => { - nativeRef.current?.setNativeProps({ - seek: { + VideoManager.seek( + { time, tolerance: tolerance || 0, }, - }); + getReactTag(nativeRef), + ); + }, + android: () => { + VideoManager.seek( + { + time, + }, + getReactTag(nativeRef), + ); }, default: () => { - nativeRef.current?.setNativeProps({ - seek: time, - }); + // TODO: Implement VideoManager.seek for windows + nativeRef.current?.setNativeProps({seek: time}); }, })(); }, []); diff --git a/src/specs/VideoNativeComponent.ts b/src/specs/VideoNativeComponent.ts index 6d718b8d28..fd53d328f7 100644 --- a/src/specs/VideoNativeComponent.ts +++ b/src/specs/VideoNativeComponent.ts @@ -537,6 +537,7 @@ export type VideoSaveData = { export interface VideoManagerType { save: (option: object, reactTag: number) => Promise; + seek: (option: Seek, reactTag: number) => Promise; setPlayerPauseState: (paused: boolean, reactTag: number) => Promise; setLicenseResult: ( result: string, From 02bfb653c7370ceb8bb3cfaf5cdcbee19820fd79 Mon Sep 17 00:00:00 2001 From: Krzysztof Moch Date: Thu, 28 Mar 2024 10:55:27 +0100 Subject: [PATCH 2/2] code review --- src/Video.tsx | 29 ++++++++++++----------------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/src/Video.tsx b/src/Video.tsx index 60292d0ac2..fac54d25a8 100644 --- a/src/Video.tsx +++ b/src/Video.tsx @@ -227,24 +227,19 @@ const Video = forwardRef( return; } + const callSeekFunction = () => { + VideoManager.seek( + { + time, + tolerance: tolerance || 0, + }, + getReactTag(nativeRef), + ); + }; + Platform.select({ - ios: () => { - VideoManager.seek( - { - time, - tolerance: tolerance || 0, - }, - getReactTag(nativeRef), - ); - }, - android: () => { - VideoManager.seek( - { - time, - }, - getReactTag(nativeRef), - ); - }, + ios: callSeekFunction, + android: callSeekFunction, default: () => { // TODO: Implement VideoManager.seek for windows nativeRef.current?.setNativeProps({seek: time});