ios: NSInternalInconsistencyException for "currentItem.hasEnabledAudio" observer

thisisthekap commented 3 years ago

🐛 Bug Report

Sometimes when playback stops, the app crashes throwing this exception:

NSInternalInconsistencyException Reason: Cannot update for observer NSKeyValueObservance for the key path "currentItem.hasEnabledAudio" from AVQueuePlayer, most likely because the value for the key "currentItem" has changed without an appropriate KVO notification being sent.

It seems to be not limited to currentItem.hasEnabledAudio. We saw similar exceptions for these members:

currentItem.hasEnabledVideo
currentItem.presentationSize
currentItem.seekableTimeRanges
currentItem.status
currentItem.playbackBufferEmpty

Expected behavior

No crash :)

Reproduction steps

Play and stop playback, repeat approx 15 times.

Configuration

Version: 1.0.8

Platform:

[x] :iphone: iOS
[ ] :robot: Android
[ ] :checkered_flag: WPF
[ ] :earth_americas: UWP
[ ] :apple: MacOS
[ ] :tv: tvOS
[x] :monkey: Xamarin.Forms

thisisthekap commented 3 years ago

To out point of view, this might be caused by the use of replaceCurrentItem on an instance of AVQueuePlayer.

According to Apple, this might lead to exceptions: Source: https://developer.apple.com/documentation/avfoundation/avplayer/1390806-replacecurrentitem

thisisthekap commented 3 years ago

Suggesting to replace AVPlayer.replaceCurrentItem with AVQueuePlayer.removeAllItems and AVQueuePlayer.insert.

Going to try this one internally. Will update this issue thereafter.

martijn00 commented 3 years ago

@thisisthekap did it work? Can you make a PR?

thisisthekap commented 3 years ago

No, unfortunately not (other errors appeared, skipped that approach thereafter). Created my own implementation from scratch.

jamsoft commented 3 years ago

I'm running into this issue at the moment as well. @thisisthekap any indicators as to what's different about your custom implementation over CrossMediaManager?

jamsoft commented 3 years ago

@martijn00 Hope you're well. I've spent the day looking at this issue!

I read through the Apple docs as linked above and have updated just this method:

public virtual async Task Play(AVPlayerItem playerItem)
{
    Player.ActionAtItemEnd = AVPlayerActionAtItemEnd.None;
    //Player.ReplaceCurrentItemWithPlayerItem(playerItem);
    Player.RemoveAllItems();
    Player.InsertItem(playerItem, null);
    await Play();
}

I've also plugged the MediaManager and MediaManager.Forms projects directly into the application I'm building in order to get some closer to real world testing done. So far, I'm not seeing any crashes.

The issue is that the problem was extremely intermittent, even in dev. So whilst I haven't seen any issues it's really hard for me to "sign it off" so-to-speak. I had the odd report from the test group of videos bombing the app with this logged in App Center:

SIGABRT: Objective-C exception thrown. Name: NSInternalInconsistencyException Reason: Cannot remove an observer 
<NSKeyValueObservance 0x281ff9440> for the key path "currentItem.status" from <AVQueuePlayer 0x281647980>, most likely 
because the value for the key "currentItem" has changed without an appropriate KVO notification being sent. Check the KVO-
compliance of the AVQueuePlayer class.

Having now pushed the app out to a wider beta group it seems a too large to ignore group of those users ran headlong into this issue.

I'm going to keep testing for the rest of today and see if I can at least increase confidence in this being "more compliant". I think it might be prudent for you to give this a pretty close test if it makes it to a PR.

jamsoft commented 3 years ago

@martijn00 I've pushed the current code to a branch in my fork here:

https://github.com/jamsoft/XamarinMediaManager/tree/fix/814-NSInternalInconsistencyException

jamsoft commented 3 years ago

Interestingly, I've just switched back to my main dev branch and within two debug runs BOOM the error occurs ...

This does lead me to feel the fix in fact a good fix.

Baseflow / XamarinMediaManager