fix(ios): crash due to persistent keyPath observer

[awitherow]

@nicolai86 discovered a bug where the video player was crashing our application.

[asgvard]

Quite important fix. With upgrading to RN 0.45 it crashes every single time I deallocate the video, looking forward for this one to be merged.

[awitherow]

It's been over a month... time to ping.

@brentvatne
@dabit3
@GantMan
@ide
@knowbody
@satya164
@oblador
@magicismight
@Kureev

can one of you look into this as you're the only people with merge rights, from what I understand.

[GantMan]

Looks good to me

[brentvatne]

I don't work on this library anymore so I'm hesitant to merge this. I have given a few other people collaborator status on this project but I'm not sure how active they are anymore. If anybody is interested in becoming a collaborator/maintainer/whatever you want to call it on this project, let me know.

[awitherow]

After doing 2 minutes of reading and attempting to understand, here is what I found out.

There is a related issue.

#43

which has a fixing commit >

c5eb6b4#diff-a487e217f78006f9dbd4a4808a7400eeR107

which seems to take care of it mostly, but not fully... which is why this PR was created.

This PR just further extends removePlayerItemObservers to not only remove the PlayerItem, but also a PlayerLayer, which will remove the complete player, not just the internal Item of the layer, from the keypath observer.

@nicolai86 does this summarise the implementation or have I got it all wrong?

If this is correct, does this satisfy your uncertainty @brentvatne?

[brentvatne]

@awitherow - I don't really want to merge this because I haven't contributed to this project in over a year and I don't think it's my place. I also don't have time to test this. We use a different video implementation on expo.io which is why I wanted to move this repo to react-native-community and onboard other people to the project. If none of them are active anymore I can onboard some more contributors but I won't merge this PR and update the release myself.

[onchainguy-btc]

@awitherow would be really awesome if this one could be merged. We are using this library in more than 5 productive apps. Unfortunately we have to use our own fork where this PR is merged because otherwise we have crashes for about 5% of our users all the time. With this PR there is no crash and the library works perfectly.

[garthenweb]

@mattapperson would it be possible to get this merged? I think there are some good reactions to this fix and others could profit from it. We have it in production for 7 months now and cannot see any issues anymore :) Would be really nice if you could find the time to have a look at it!

[awitherow]

nice that it was finally merged @garthenweb 🎉