fix: repeat video on iOS with AVPlayerLooper

[swemail]

Hello and thank you for a great library! This is a shot at the fix proposed in #2817

To test this is just to run the basic example in this repo. Maybe remove the alert at the end to get the sense of the smoother repeat 😄

The solution proposed is to create a AVQueuePlayer with a AVPlayerLooper if supported (iOS >= 10) and fall back to the previous used AVPlayer if it's not. The AVPlayerLooper need to be references to not be disposed. The variable holding that reference is typed to NSObject to make it compatible with previous iOS versions not supporting AVPlayerLooper. The order of things matter and I had to move passing the playerItem to the playerObserver until after initiation of the player

[freeboub]

code looks ok to me, but we would need additionnal testers,
@MitoMonkey @johnylawrence1987 can you please test this PR to check if it solves your issue ?

[swemail]

code looks ok to me, but we would need additionnal testers, @MitoMonkey @johnylawrence1987 can you please test this PR to check if it solves your issue ?

I've tested it some more on my own and made some improvements, will push so that is the version tested

[MitoMonkey]

Hi
Thanks for the update. I am getting build errors trying to test this branch https://github.com/29ki/react-native-video/tree/repeat-ios-using-av-player-looper.
Which versions of react-native and React are you using?

[swemail]

Hi Thanks for the update. I am getting build errors trying to test this branch https://github.com/29ki/react-native-video/tree/repeat-ios-using-av-player-looper. Which versions of react-native and React are you using?

FYI I haven't worked with native libraries before, I just wanted to help getting this fix in since our project needs it. So I didn't find a better way to test this than running it from examples/basic so I'm using what is specified there react: 16.13.1 and react-native: 0.63.4. Is there a more "proper" way to build it? BTW, made another commit so be sure to pull the latest.

[MitoMonkey]

@swemail 😅 seems like we are both rather new to this.

So what I tried to test it was npm install https://github.com/29ki/react-native-video/tree/repeat-ios-using-av-player-looper in our app folder as well as simply editing package.json to include "react-native-video": "git+https://github.com/29ki/react-native-video/tree/repeat-ios-using-av-player-looper", in the dependencies and then run npm install.
So I did not try to test your PR in a dedicated testing repo but rather directly in our app. Either way, I could not build the project anymore in Xcode. (I also tried deleting the whole node-modules folder first. And of course creating new pod files.)

We are currently using react-native 0.64.1 and react 17.0.1

[swemail]

@swemail 😅 seems like we are both rather new to this.

So what I tried to test it was npm install https://github.com/29ki/react-native-video/tree/repeat-ios-using-av-player-looper in our app folder as well as simply editing package.json to include "react-native-video": "git+https://github.com/29ki/react-native-video/tree/repeat-ios-using-av-player-looper", in the dependencies and then run npm install. So I did not try to test your PR in a dedicated testing repo but rather directly in our app. Either way, I could not build the project anymore in Xcode. (I also tried deleting the whole node-modules folder first. And of course creating new pod files.)

We are currently using react-native 0.64.1 and react 17.0.1

Ok, I've tried in our app too, installing via npm install https://github.com/29ki/react-native-video#repeat-ios-using-av-player-looper, Don't know if that makes any difference? We're running on react-native: 0.69.5 and react: 18.0.0. What error do you get?

[MitoMonkey]

This is the error I get from an npx react-native run-android :

Could not determine the dependencies of task ':app:mergeDebugNativeLibs'.
> Could not resolve all task dependencies for configuration ':app:debugRuntimeClasspath'.
   > Could not resolve project :react-native-video.
     Required by:
         project :app
      > No matching configuration of project :react-native-video was found. The consumer was configured to find a runtime of a component, as well as attribute 'com.android.build.api.attributes.BuildTypeAttr' with value 'debug', attribute 'react-native-camera' with value 'general' but:
          - None of the consumable configurations have attributes.

Xcode gives me this:

Build input file cannot be found: '[path to my repo]/app/node_modules/react-native-video/ios/Video/RCTVideo.m'. Did you forget to declare this file as an output of a script phase or custom build rule which produces it?

[swemail]

Ok, I've only made changes to the iOS implementation. The issue you posted was referring to version 5.2.0 of react-native-video and this is for 6.0.0. There could be issues there I'm not familiar with. It probably wont work for react-native 0.64.1. We created a fork of 5.2.1 that we're using in our app since we can't get 6.0.0 to work on Android. I will create a PR for the old version and maybe you can test that one. You can try it now by npm install https://github.com/29ki/react-native-video-5.2.1#repeat-ios-using-av-player-looper

EDIT:
Here is the PR for the 5.2.x version 2926

[MitoMonkey]

oh yeah that makes totally sense. I completely overlooked that your fork was based on v6 of the react-native-video library. Sorry for that.
Thank you so much for so quickly providing a Version based on 5.2.1. It works like a charm on iOS. However on Android I do not get any sound anymore. I am a bit lost in finding out why... Any ideas?

[swemail]

That is strange. Should not have to do with my change, I didn't change anything regarding Android. I though I didn't have sound on my Android emulator but had to go in and adjust the sound on the emulator itself. Seems like a stretch but worth checking

[swemail]

@MitoMonkey Sorry for all the confusion I create here but I was not planning on making a PR for the 5.2.1 version but did today in hopes of us using the library version instead of our fork. 6.0.0 could be too long for us to wait.

Could it be that there are changes on top of support/5.2.x, witch my latest PR is based on, that are not compatible with your environment? Did you install https://github.com/29ki/react-native-video-5.2.1#repeat-ios-using-av-player-looper (base on the latest 5.2.1 release) or https://github.com/swemail/react-native-video-5.2.x#repeat-ios-using-player-looper witch is based on the latest changes on support/5.2.x. Maybe you could try with the one you didn't install 😂 .

[MitoMonkey]

I'm not sure what was the problem with the audio in Android Emulator, but a restart of my macbook + cold boot of the emulator + delete and re-install of node-modules solved the problem. So apparently has nothing to do with your PR. Sorry about the unnecessary confusion here.
So now it works perfectly fine.

The problem I am facing now is that we had patches for v5.2.0 in place because of the problem described here #2371 . Theoretically the PR is merged and should the problem should be solved in v5.2.1, but for us somehow it isn't. But that is an issue to be discussed somewhere else.

Thanks a lot again @swemail for this fix for the loop function. It is really a big fix for our app.
Anything I need to do to mark this PR as approved?

[swemail]

Glad to hear it worked, at least regarding this feature @MitoMonkey! It was really important for our app too. Thank you for sticking it out and testing it. I guess that what's been tested is actually 2926 so if you could throw in a comment there that you tested it I would be so grateful.

The change is the same for both versions but of course in different languages

[tironiigor]

@swemail @freeboub
Don't we need the same check here to keep the old player if it's available like in line 234?

Suggested change

	self._player = AVQueuePlayer(playerItem: playerItem)
	self._player = self._player ?? AVQueuePlayer(playerItem: playerItem)

[chornesays]

@freeboub We're running into this issue. Any chance this will get merged+released?