TheWidlarzGroup / react-native-video

A <Video /> component for react-native
http://thewidlarzgroup.github.io/react-native-video/
MIT License
7.15k stars 2.88k forks source link

[BUG]: Ios single video making or repeating multiple sound #3567

Open keval7169-ops opened 6 months ago

keval7169-ops commented 6 months ago

Version

v6 (Beta)

What platforms are you having the problem on?

iOS

Architecture

Old architecture

What happened?

whenever I play the video after changing the template then it will play normally but most of the time it play multiple sounds of single video(like reverb)

Reproduction

Steps to reproduce this bug are:

listen whole video.

https://github.com/react-native-video/react-native-video/assets/159753559/99d83fb6-ab1a-470f-b43e-f61d20fc5af9

KidsOnShred commented 6 months ago

I am also experiencing this issue. I suspect it is caused by rerenders creating new native instances of the video which are not being destroyed properly.

I also have audio that is continuing to play after my component has been unmounted. I am working around this by just having the video muted on render and being manually unmuted and also checking focus to stop videos playing and muting them also. I think there must be memory leaking as a result of all this

tdammy92 commented 6 months ago

@KidsOnShred Please where u able to fix this at your end ?

freeboub commented 6 months ago

Can you please clarify which beta version you use? And can you please try on the last one please ? (Beta.6)

KidsOnShred commented 5 months ago

I worked around it by auto muting the video in the start then manually unmute the one video which stops the sound duplicating but I suspect that multiple videos are playing in the background if the component ever rerenders. I am using 5.2.1

coofzilla commented 5 months ago

this is an issue on version: 6.0.0-beta.8 as well. Basically, single player has two audio playbacks.

rajgupta027 commented 4 months ago

Facing same issue still on IOS on changing url. Version 6.1.2

freeboub commented 4 months ago

Can you please provide a sample? I don't think this is reproduced with the sample included in this repo ?

mk0116 commented 3 months ago

Facing same issue here too. Version 6.1.2 It is reproducible when you play a video with a url, unmount, mount a video component and play video with a url again.(doesn't matter if the urls are same or not)

rajgupta027 commented 3 months ago

@mk0116 @freeboub Its concerning now since I am unable to move to latest (6.1.2) package in my project. Seems I'll have to wait before more stable version is out. Also, I need IMA google ads enabled for my project which is NOT present in previous stable version (5.2.1). Requesting the team to please look into it at the earliest.

freeboub commented 3 months ago

I agree this is critical, but are you able to reproduce it with a sample ? Last time I tried, I was not able to reproduce with the sample included in the repo. I don't really know ho to progress...

freeboub commented 3 months ago

Ok, I am able to reproduce the issue with following patch in the sample:

diff --git a/examples/basic/src/VideoPlayer.tsx b/examples/basic/src/VideoPlayer.tsx
index f712e013..4d720221 100644
--- a/examples/basic/src/VideoPlayer.tsx
+++ b/examples/basic/src/VideoPlayer.tsx
@@ -89,6 +89,7 @@ interface StateType {
   poster?: string;
   showNotificationControls: boolean;
   isSeeking: boolean;
+  _key: number;
 }

 class VideoPlayer extends Component {
@@ -120,6 +121,7 @@ class VideoPlayer extends Component {
     poster: undefined,
     showNotificationControls: false,
     isSeeking: false,
+    _key: 0,
   };

   // internal usage change to index if you want to select tracks by index instead of lang
@@ -557,6 +560,13 @@ class VideoPlayer extends Component {
     }
   };

+  componentDidMount(): void {
+      this.setState({_key: this.state._key + 1})
+      setTimeout(()=> {
+        this.setState({_key: this.state._key + 1})
+      })
+  }
+
   onSelectedTextTrackChange = (itemValue: string) => {
     console.log('on value change ' + itemValue);
     this.setState({
@@ -762,6 +772,7 @@ class VideoPlayer extends Component {
     return (
       <TouchableOpacity style={viewStyle}>
         <Video
+          key={this.state._key}
           showNotificationControls={this.state.showNotificationControls}
           ref={(ref: VideoRef) => {
             this.video = ref;
mk0116 commented 3 months ago

@freeboub I found a related code error while debugging the issue. Player objects are not popping properly from the observers.

So that it keeps adding player in the observers object.

-        if let observer = observers[players.hashValue] {
+        if let observer = observers[player.hashValue] {
             observer.invalidate()
         }

-        observers.removeValue(forKey: players.hashValue)
+        observers.removeValue(forKey: player.hashValue)
         players.remove(player)

But this is just a piece.

I found a main problem of the issue. The views do init -> registerPlayer -> deinit first time but second time, it does init the RCTPlayer view and registerPlayer twice and deinit just once. init -> registerPlayer -> init -> registerPlayer -> deinit. This leads also to stacking player objects to the observers object and never pop out.

For me I was able to resolve the issue by patching the package with the code above and memo the react view so that view renders not too many times and ensured it does not detach and attach in a short time.

freeboub commented 3 months ago

@mk0116 on my side, I also see that we don't stop the player in deinit { This is also a part as multiple reloads still reproduce the issue. I'll keep you updated, and thank you for the patch !

freeboub commented 3 months ago

ping @KrzysztofMoch can you confirm it makes sense for you ?

freeboub commented 3 months ago

Here is an aditionnal patch which make sense I think. I didn't find the real root cause, but I think there is an issue with some ref not uninitialized. I don't reproduce the issue once this is added: Let me know if it's also fixing the issue on your side !

@@ -1239,21 +1239,31 @@ class RCTVideo: UIView, RCTVideoPlayerViewControllerDelegate, RCTPlayerObserverH
     }

     // MARK: - Lifecycle

     override func removeFromSuperview() {
+        self._player?.replaceCurrentItem(with: nil)
         if let player = _player {
             player.pause()
             NowPlayingInfoCenterManager.shared.removePlayer(player: player)
         }
+        _playerItem = nil
+        _source = nil
+        _chapters = nil
+        _drm = nil
+        _textTracks = nil
+        _selectedTextTrackCriteria = nil
+        _selectedAudioTrackCriteria = nil
+        _presentingViewController = nil

         _player = nil
         _resouceLoaderDelegate = nil
         _playerObserver.clearPlayer()

         #if USE_GOOGLE_IMA
             _imaAdsManager.releaseAds()
+            _imaAdsManager = nil
         #endif

         self.removePlayerLayer()

         if let _playerViewController {
freeboub commented 3 months ago

BTW, I reproduce similar issue on android also :'(

freeboub commented 3 months ago

@mk0116 I pushed a branch with both fixes here: https://github.com/TheWidlarzGroup/react-native-video/pull/3875, can you please test if it fixes your issue

freeboub commented 3 months ago

Will be integrated in 6.2.0

mk0116 commented 3 months ago

Thank you for your effort @freeboub I will test it soon and let you know.

MTPL0005-AbhishekDube commented 2 months ago

@freeboub I have encounter this issue when url for video changes the video plays two audios. version(6.2.0 & 6.3.0)

rajgupta027 commented 2 months ago

@MTPL0005-AbhishekDube @freeboub I am still facing this issue. Applied the above patch as well.

freeboub commented 2 months ago

Can you provide a sample please ?

MTPL0005-AbhishekDube commented 2 months ago

@freeboub I use similar code as video example <Video bufferConfig={{ minBufferMs: 15000, maxBufferMs: 50000, bufferForPlaybackMs: 2500, bufferForPlaybackAfterRebufferMs: 5000, cacheSizeMB: 0, }} // fullscreen={fullscreen} muted={muted} onAspectRatio={this.onAspectRatio} onAudioTracks={this.onAudioTracks} onBuffer={this.onBuffer} onEnd={this.onEnd} onLoad={this.onLoad} onLoadStart={this.onVideoLoadStart} onProgress={this.onProgress} onReceiveAdEvent={this._onReceiveAdEvent} onTextTracks={this.onTextTracks} paused={paused} pictureInPicture={false} playInBackground={playInBackground} playWhenInactive={playInBackground} poster={courseImage} posterResizeMode="cover" preventsDisplaySleepDuringVideoPlayback={true} rate={rate} ref={(ref) => { this.videoPlayerRef = ref; }} resizeMode={ResizeMode.CONTAIN} selectedAudioTrack={selectedAudioTrack} selectedTextTrack={selectedTextTrack} showNotificationControls={playInBackground} source={{ uri: url, metadata: { title: title, artist: mentorName, imageUri: courseImage, }, }} style={fullscreen ? styles.fullscreen : styles.inlineScreen} volume={volume} /> I change the source uri onClick of list of lessons.

freeboub commented 2 months ago

We need a sample in a dedicated repository! Stop copy pasting extract of code please...

rajgupta027 commented 3 weeks ago

@freeboub Try this snippet. I am able to reproduce this issue 100% times in ios. React Native: 0.71 React Native Video: 6.1.2 Apparently, whenever there is instant mounting/unmounting of video player, audio leak is observed.

//App.js

import React, { useState ,useEffect} from "react"; import { Dimensions } from "react-native"; import Video from "react-native-video";

const App = () => { const [mount, setMount] = useState(true); useEffect(() => { setMount(false); }, []);

return ( mount && ( <Video source={{ uri: "http://content.jwplatform.com/manifests/vM7nH0Kl.m3u8", }} style={{ height: Dimensions.get("screen").width, width: Dimensions.get("screen").width, backgroundColor: "red", }} /> ) ); };

In my case,I prevented instant mount/unmounting of player and issue is resolved for me for the time being. In my understanding, deinit method is not being called when this scenario is observed.

freeboub commented 3 weeks ago

@rajgupta027 on 6.5.0 ?

rajgupta027 commented 3 weeks ago

@rajgupta027 on 6.5.0 ? Yes, its happening on latest version(6.5.0) as well.

Screenshot 2024-09-06 at 15 49 53

Normal Case: Video Playback works fine.

https://github.com/user-attachments/assets/82544079-be84-4233-889e-88cd786d3c69

Instant Mount/Unmount Case: Audio Leak is observed.

https://github.com/user-attachments/assets/a8af4157-0235-4cdf-8c1a-10019b82c007