refactor: don't use nitro dispose (#4802)

2026-05-23 15:07:45 +00:00 · 2025-12-19 13:19:46 +01:00
parent 1eb317a566
commit ff882c23d0
24 changed files with 164 additions and 27 deletions
@@ -52,7 +52,7 @@
    },
    "example": {
      "name": "react-native-video-example",
-      "version": "7.0.0-beta.0",
+      "version": "7.0.0-beta.1",
      "dependencies": {
        "@react-native-community/slider": "^4.5.6",
        "@react-native-video/drm": "*",
@@ -80,7 +80,7 @@
    },
    "packages/drm-plugin": {
      "name": "@react-native-video/drm",
-      "version": "7.0.0-beta.0",
+      "version": "7.0.0-beta.1",
      "devDependencies": {
        "@react-native/babel-preset": "0.79.2",
        "@release-it/conventional-changelog": "^9.0.2",
@@ -108,7 +108,7 @@
    },
    "packages/react-native-video": {
      "name": "react-native-video",
-      "version": "7.0.0-beta.0",
+      "version": "7.0.0-beta.1",
      "devDependencies": {
        "@expo/config-plugins": "^10.0.2",
        "@react-native/eslint-config": "^0.77.0",
@@ -1565,7 +1565,7 @@ PODS:
    - React-logger (= 0.77.3)
    - React-perflogger (= 0.77.3)
    - React-utils (= 0.77.3)
-  - ReactNativeVideo (7.0.0-beta.0):
+  - ReactNativeVideo (7.0.0-beta.1):
    - DoubleConversion
    - glog
    - hermes-engine
@@ -1587,7 +1587,7 @@ PODS:
    - ReactCommon/turbomodule/bridging
    - ReactCommon/turbomodule/core
    - Yoga
-  - ReactNativeVideoDrm (7.0.0-beta.0):
+  - ReactNativeVideoDrm (7.0.0-beta.1):
    - DoubleConversion
    - glog
    - hermes-engine
@@ -1904,8 +1904,8 @@ SPEC CHECKSUMS:
  ReactAppDependencyProvider: 31015410a4a53b9fd0a908ad4d6e3e2b9a25086a
  ReactCodegen: 53316394e985ded1babc7f143c90c77d2bb1b43c
  ReactCommon: bf4612cba0fa356b529385029f470d5529dddde4
-  ReactNativeVideo: 5a5e609057e980e9ea2736914377804358c53ae9
-  ReactNativeVideoDrm: 4f266c3b018170319ed16bc511218c0d411358d5
+  ReactNativeVideo: 10dd0a47f8228b41565a3efb13df5b323633b590
+  ReactNativeVideoDrm: 07b826ab66fd0a00ab3dd3bef2dd2c026e919a9a
  SocketRocket: d4aabe649be1e368d1318fdf28a022d714d65748
  Yoga: 92f3bb322c40a86b7233b815854730442e01b8c4

@@ -33,6 +33,12 @@ namespace margelo::nitro::videodrm {
    method(_javaPart);
  }

+  std::string JHybridPluginManagerSpec::toString() {
+    static const auto method = javaClassStatic()->getMethod<jni::JString()>("toString");
+    auto javaString = method(_javaPart);
+    return javaString->toStdString();
+  }
+
  // Properties
  bool JHybridPluginManagerSpec::getIsEnabled() {
    static const auto method = javaClassStatic()->getMethod<jboolean()>("isEnabled");
@@ -41,6 +41,7 @@ namespace margelo::nitro::videodrm {
  public:
    size_t getExternalMemorySize() noexcept override;
    void dispose() noexcept override;
+    std::string toString() override;

  public:
    inline const jni::global_ref<JHybridPluginManagerSpec::javaobject>& getJavaPart() const noexcept {
@@ -10,7 +10,7 @@ package com.margelo.nitro.videodrm
 import androidx.annotation.Keep
 import com.facebook.jni.HybridData
 import com.facebook.proguard.annotations.DoNotStrip
-import com.margelo.nitro.core.*
+import com.margelo.nitro.core.HybridObject

 /**
 * A Kotlin class representing the PluginManager HybridObject.
@@ -36,6 +36,11 @@ abstract class HybridPluginManagerSpec: HybridObject() {
    super.updateNative(hybridData)
  }

+  // Default implementation of `HybridObject.toString()`
+  override fun toString(): String {
+    return "[HybridObject PluginManager]"
+  }
+
  // Properties
  @get:DoNotStrip
  @get:Keep
@@ -10,6 +10,7 @@
 // Include C++ implementation defined types
 #include "HybridPluginManagerSpecSwift.hpp"
 #include "ReactNativeVideoDrm-Swift-Cxx-Umbrella.hpp"
+#include <NitroModules/NitroDefines.hpp>

 namespace margelo::nitro::videodrm::bridge::swift {

@@ -50,6 +50,9 @@ namespace margelo::nitro::videodrm {
    void dispose() noexcept override {
      _swiftPart.dispose();
    }
+    std::string toString() override {
+      return _swiftPart.toString();
+    }

  public:
    // Properties
@@ -18,6 +18,13 @@ public protocol HybridPluginManagerSpec_protocol: HybridObject {
  func disable() throws -> Void
 }

+public extension HybridPluginManagerSpec_protocol {
+  /// Default implementation of ``HybridObject.toString``
+  func toString() -> String {
+    return "[HybridObject PluginManager]"
+  }
+}
+
 /// See ``HybridPluginManagerSpec``
 open class HybridPluginManagerSpec_base {
  private weak var cxxWrapper: HybridPluginManagerSpec_cxx? = nil
@@ -76,7 +76,7 @@ open class HybridPluginManagerSpec_cxx {
   */
  public func getCxxPart() -> bridge.std__shared_ptr_HybridPluginManagerSpec_ {
    let cachedCxxPart = self.__cxxPart.lock()
-    if cachedCxxPart.__convertToBool() {
+    if Bool(fromCxx: cachedCxxPart) {
      return cachedCxxPart
    } else {
      let newCxxPart = bridge.create_std__shared_ptr_HybridPluginManagerSpec_(self.toUnsafe())
@@ -105,6 +105,14 @@ open class HybridPluginManagerSpec_cxx {
    self.__implementation.dispose()
  }

+  /**
+   * Call toString() on the Swift class.
+   */
+  @inline(__always)
+  public func toString() -> String {
+    return self.__implementation.toString()
+  }
+
  // Properties
  public final var isEnabled: Bool {
    @inline(__always)
@@ -170,9 +170,17 @@ object VideoManager : LifecycleEventListener {
  }

  fun unregisterPlayer(player: HybridVideoPlayer) {
-    players.remove(player)
    audioFocusManager.unregisterPlayer(player)
    PluginsRegistry.shared.notifyPlayerDestroyed(WeakReference(player))
+
+    // Remove player from any views that were using it
+    players[player]?.forEach { nitroId ->
+      views[nitroId]?.get()?.let { view ->
+        view.hybridPlayer = null
+      }
+    }
+
+    players.remove(player)
  }

  fun getPlayerByNitroId(nitroId: Int): HybridVideoPlayer? {
@@ -43,7 +43,7 @@ import kotlin.math.max

@UnstableApi
@DoNotStrip
-class HybridVideoPlayer() : HybridVideoPlayerSpec() {
+class HybridVideoPlayer() : HybridVideoPlayerSpec(), AutoCloseable {
  override lateinit var source: HybridVideoPlayerSourceSpec
  override var eventEmitter = HybridVideoPlayerEventEmitter()
    set(value) {
@@ -348,7 +348,7 @@ class HybridVideoPlayer() : HybridVideoPlayerSpec() {
    }
  }

-  private fun release() {
+  override fun release() {
    if (playInBackground || showNotificationControls) {
      VideoPlaybackService.stopService(this, videoPlaybackServiceConnection)
    }
@@ -358,6 +358,8 @@ class HybridVideoPlayer() : HybridVideoPlayerSpec() {
      stopProgressUpdates()
      loadedWithSource = false

+      eventEmitter.clearAllListeners()
+
      player.removeListener(playerListener)
      player.removeAnalyticsListener(analyticsListener)
      player.release() // Release player
@@ -384,8 +386,13 @@ class HybridVideoPlayer() : HybridVideoPlayerSpec() {
    release()
  }

+  override fun close() {
+    release()
+  }
+
  override val memorySize: Long
-    get() = allocator?.totalBytesAllocated?.toLong() ?: 0L
+    // 1 MiB by default
+    get() = allocator?.totalBytesAllocated?.toLong() ?: (1024L * 1024L)

  private fun startProgressUpdates() {
    stopProgressUpdates() // Ensure no multiple runnables
@@ -7,31 +7,43 @@ import java.util.UUID
 data class ListenerPair(val id: UUID, val eventName: String, val callback: Any)

 class HybridVideoPlayerEventEmitter : HybridVideoPlayerEventEmitterSpec() {
+  private val lock = Any()
+
  var listeners: MutableList<ListenerPair> = mutableListOf()

  // MARK: - Private helpers
  private fun <T : Any> addListener(eventName: String, listener: T): ListenerSubscription {
    val id = UUID.randomUUID()
-    listeners.add(ListenerPair(id, eventName, listener))
-    return ListenerSubscription { listeners.removeAll { it.id == id } }
+    synchronized(lock) {
+      listeners.add(ListenerPair(id, eventName, listener))
+    }
+    return ListenerSubscription {
+      synchronized(lock) {
+        listeners.removeAll { it.id == id }
+      }
+    }
  }

  private inline fun <reified T> emitEvent(eventName: String, invokeCallback: (T) -> Unit) {
-    listeners.filter { it.eventName == eventName }.forEach { pair ->
+    val snapshot: List<ListenerPair> = synchronized(lock) {
+      listeners.filter { it.eventName == eventName }.toList()
+    }
+
+    snapshot.forEach { pair ->
      try {
        @Suppress("UNCHECKED_CAST")
-        val callback = pair.callback as? T
-        if (callback == null) {
+        val callback = pair.callback as? T ?: run {
          Log.d(TAG, "Invalid callback type for $eventName")
          return@forEach
        }
        invokeCallback(callback)
-      } catch (error: Error) {
-        Log.d(TAG, "Error calling $eventName listener $error")
+      } catch (t: Throwable) {
+        Log.d(TAG, "Error calling $eventName listener", t)
      }
    }
  }

+
  // MARK: - Listener registration methods
  
  override fun addOnAudioBecomingNoisyListener(listener: () -> Unit) =
@@ -92,7 +104,9 @@ class HybridVideoPlayerEventEmitter : HybridVideoPlayerEventEmitterSpec() {
    addListener("onVolumeChange", listener)

  override fun clearAllListeners() {
-    listeners.clear()
+    synchronized(lock) {
+      listeners.clear()
+    }
  }

  // MARK: - Event emission methods
@@ -49,7 +49,7 @@ extension HybridVideoPlayer: VideoPlayerObserverDelegate {

  func onPlaybackLikelyToKeepUp() {
    isCurrentlyBuffering = false
-    if player.timeControlStatus == .playing {
+    if player.timeControlStatus != .waitingToPlayAtSpecifiedRate {
      status = .readytoplay
    }
    updateAndEmitPlaybackState()
@@ -206,6 +206,9 @@ class HybridVideoPlayer: HybridVideoPlayerSpec, NativeVideoPlayerSpec {
  func release() {
    sourceLoader.cancelSync()
    NowPlayingInfoCenterManager.shared.removePlayer(player: player)
+    
+    try? _eventEmitter?.clearAllListeners()
+    
    self.player.replaceCurrentItem(with: nil)
    self.playerItem = nil

@@ -228,6 +228,10 @@ namespace margelo::nitro::video {
    static const auto method = javaClassStatic()->getMethod<void(jni::alias_ref<JVariant_NullType_TextTrack> /* textTrack */)>("selectTextTrack");
    method(_javaPart, textTrack.has_value() ? JVariant_NullType_TextTrack::fromCpp(textTrack.value()) : nullptr);
  }
+  void JHybridVideoPlayerSpec::release() {
+    static const auto method = javaClassStatic()->getMethod<void()>("release");
+    method(_javaPart);
+  }
  std::shared_ptr<Promise<void>> JHybridVideoPlayerSpec::initialize() {
    static const auto method = javaClassStatic()->getMethod<jni::local_ref<JPromise::javaobject>()>("initialize");
    auto __result = method(_javaPart);
@@ -82,6 +82,7 @@ namespace margelo::nitro::video {
    std::shared_ptr<Promise<void>> replaceSourceAsync(const std::optional<std::variant<nitro::NullType, std::shared_ptr<HybridVideoPlayerSourceSpec>>>& source) override;
    std::vector<TextTrack> getAvailableTextTracks() override;
    void selectTextTrack(const std::optional<std::variant<nitro::NullType, TextTrack>>& textTrack) override;
+    void release() override;
    std::shared_ptr<Promise<void>> initialize() override;
    std::shared_ptr<Promise<void>> preload() override;
    void play() override;
@@ -141,6 +141,10 @@ abstract class HybridVideoPlayerSpec: HybridObject() {
  @Keep
  abstract fun selectTextTrack(textTrack: Variant_NullType_TextTrack?): Unit
  
+  @DoNotStrip
+  @Keep
+  abstract fun release(): Unit
+  
  @DoNotStrip
  @Keep
  abstract fun initialize(): Promise<Unit>
@@ -188,6 +188,12 @@ namespace margelo::nitro::video {
        std::rethrow_exception(__result.error());
      }
    }
+    inline void release() override {
+      auto __result = _swiftPart.release();
+      if (__result.hasError()) [[unlikely]] {
+        std::rethrow_exception(__result.error());
+      }
+    }
    inline std::shared_ptr<Promise<void>> initialize() override {
      auto __result = _swiftPart.initialize();
      if (__result.hasError()) [[unlikely]] {
@@ -32,6 +32,7 @@ public protocol HybridVideoPlayerSpec_protocol: HybridObject {
  func replaceSourceAsync(source: Variant_NullType__any_HybridVideoPlayerSourceSpec_?) throws -> Promise<Void>
  func getAvailableTextTracks() throws -> [TextTrack]
  func selectTextTrack(textTrack: Variant_NullType_TextTrack?) throws -> Void
+  func release() throws -> Void
  func initialize() throws -> Promise<Void>
  func preload() throws -> Promise<Void>
  func play() throws -> Void
@@ -369,6 +369,17 @@ open class HybridVideoPlayerSpec_cxx {
    }
  }
  
+  @inline(__always)
+  public final func release() -> bridge.Result_void_ {
+    do {
+      try self.__implementation.release()
+      return bridge.create_Result_void_()
+    } catch (let __error) {
+      let __exceptionPtr = __error.toCpp()
+      return bridge.create_Result_void_(__exceptionPtr)
+    }
+  }
+  
  @inline(__always)
  public final func initialize() -> bridge.Result_std__shared_ptr_Promise_void___ {
    do {
@@ -43,6 +43,7 @@ namespace margelo::nitro::video {
      prototype.registerHybridMethod("replaceSourceAsync", &HybridVideoPlayerSpec::replaceSourceAsync);
      prototype.registerHybridMethod("getAvailableTextTracks", &HybridVideoPlayerSpec::getAvailableTextTracks);
      prototype.registerHybridMethod("selectTextTrack", &HybridVideoPlayerSpec::selectTextTrack);
+      prototype.registerHybridMethod("release", &HybridVideoPlayerSpec::release);
      prototype.registerHybridMethod("initialize", &HybridVideoPlayerSpec::initialize);
      prototype.registerHybridMethod("preload", &HybridVideoPlayerSpec::preload);
      prototype.registerHybridMethod("play", &HybridVideoPlayerSpec::play);
@@ -98,6 +98,7 @@ namespace margelo::nitro::video {
      virtual std::shared_ptr<Promise<void>> replaceSourceAsync(const std::optional<std::variant<nitro::NullType, std::shared_ptr<HybridVideoPlayerSourceSpec>>>& source) = 0;
      virtual std::vector<TextTrack> getAvailableTextTracks() = 0;
      virtual void selectTextTrack(const std::optional<std::variant<nitro::NullType, TextTrack>>& textTrack) = 0;
+      virtual void release() = 0;
      virtual std::shared_ptr<Promise<void>> initialize() = 0;
      virtual std::shared_ptr<Promise<void>> preload() = 0;
      virtual void play() = 0;
@@ -18,7 +18,19 @@ import { createSource } from './utils/sourceFactory';
 import { VideoPlayerEvents } from './VideoPlayerEvents';

 class VideoPlayer extends VideoPlayerEvents implements VideoPlayerBase {
-  protected player: VideoPlayerImpl;
+  private _player: VideoPlayerImpl | undefined;
+  private _releaseTimeout: ReturnType<typeof setTimeout> | undefined;
+
+  protected get player(): VideoPlayerImpl {
+    if (this._player === undefined) {
+      throw new VideoRuntimeError(
+        'player/released',
+        "You can't access player after it's released"
+      );
+    }
+
+    return this._player;
+  }

  constructor(source: VideoSource | VideoConfig | VideoPlayerSource) {
    const hybridSource = createSource(source);
@@ -26,17 +38,39 @@ class VideoPlayer extends VideoPlayerEvents implements VideoPlayerBase {

    // Initialize events
    super(player.eventEmitter);
-    this.player = player;
+    this._player = player;
  }

  /**
-   * Cleans up player's native resources and releases native state.
-   * After calling this method, the player is no longer usable.
+   * Releases the player's native resources and releases native state.
   * @internal
   */
  __destroy() {
+    if (this._player === undefined) return;
+
    this.clearAllEvents();
-    this.player.dispose();
+
+    try {
+      this.player.release();
+    } catch (error) {
+      // Best effort cleanup: teardown must never crash app unmount.
+      console.error('Failed to cleanup native player resources', error);
+    }
+
+    // We leave hybrid object to be cleaned up by garbage collector
+    // So we update memory size to ensure that memory is released
+    // when needed
+    this.updateMemorySize();
+
+    // We wait for 5s to let late events that were triggered before release to be processed
+    if (this._releaseTimeout !== undefined) {
+      clearTimeout(this._releaseTimeout);
+    }
+
+    this._releaseTimeout = setTimeout(() => {
+      this._player = undefined;
+      this._releaseTimeout = undefined;
+    }, 5000);
  }

  /**
@@ -23,6 +23,12 @@ export interface VideoPlayer
   */
  showNotificationControls: boolean;

+  /**
+   * Replace the current source of the player.
+   * @param source - The new source of the video.
+   * @note If you want to clear the source, you can pass null. It has the same effect as {@link release}.
+   * see {@link VideoPlayerSourceBase}
+   */
  replaceSourceAsync(source: VideoPlayerSource | null): Promise<void>;

  /**
@@ -36,6 +42,11 @@ export interface VideoPlayer
   * @param textTrack - Text track to select, or null to unselect current track
   */
  selectTextTrack(textTrack: TextTrack | null): void;
+
+  /**
+   * Releases the player's native resources and releases native state.
+   */
+  release(): void;
 }

 export interface VideoPlayerFactory