XamarinCommunityToolkit icon indicating copy to clipboard operation
XamarinCommunityToolkit copied to clipboard

Better MediaElement error handling

Open anvuks opened this issue 3 years ago • 7 comments
trafficstars

Description of Bug

When MediaMetadataRetriever fails to load online video metadata, because the video doesn't exist or it times out due to a connection error, it throws IllegalArgumentException. I've just wrapped the problematic method call with a try/catch that raises MetadataRetrievalFailed event.

When the event is raised, MediaElementRenderer stops the video playback, sets the state to MediaElementState.Closed, and raises OnMediaFailed in the shared code. The same implementation applies if the video itself fails to load or play to the end (native implementation details). If both of them try to call OnError(), only one can succeed, until the next media source update. This is so we can avoid calling OnMediaFailed more than once for the same source, which would be expected behavior.

Issues Fixed

  • Fixes #1592
  • Fixes #399

Behavioral Changes

PR Checklist

  • [x] Has a linked Issue, and the Issue has been approved

  • [ ] Has tests (if omitted, state reason in description)

  • [ ] Has samples (if omitted, state reason in description)

  • [ ] Rebased on top of main at time of PR

  • [x] Changes adhere to coding standard

  • [ ] Updated documentation

anvuks avatar Sep 24 '22 07:09 anvuks

@pictos

brettnguyen avatar Oct 23 '22 03:10 brettnguyen

@anvuks I can't deploy the sample app on your branch, it closes right after launches. The main branch works. Can you double check that, please?

pictos avatar Oct 23 '22 03:10 pictos

@pictos I just tried it and you are right, the app closes right after it opens, only it happened on the main branch for me... The issue seems to be in the android linker settings, you should set it to None instead of Sdk assemblies only. I think there is no benefit in linking debug builds, it just slows everything down. I'm not sure why only one branch is affected, but this seems to resolve the issue completely. We should do a PR for linker settings

anvuks avatar Oct 23 '22 16:10 anvuks

@anvuks can you rebase your code against main? Not sure if there's any update but just to make sure... Sadly I can't merge this with the issue, because that can break the package and other users somehow.

pictos avatar Oct 23 '22 18:10 pictos

@pictos I just checked and my branch is up to date with main. Can you try cleaning and rebuilding on my branch? My changes should not cause the app to crash in any way because MediaElement is not being referenced on startup in the WelcomePage. When I changed back to Sdk assemblies only, I got this error in VS 2022 (image below): XAML Hot Reload is disabled because your Android Linker settings for Xamarin.CommunityToolkit.Samle.Android are unsupported. Set your Linker Behavior to "Don't Link" and rebuild your project to use Hot Reload..

Screenshot 2022-10-23 204219

Logcat reports the following error: INTERNAL_ERROR: Unable to find Android.Runtime.JNIEnv.BridgeProcessing from monodroid. A quick Google search later and the issue seems to be with Hot Reload and linker settings. Can you temporarily disable linker just to test my PR and we can think about permanently disabling it for sample apps?

anvuks avatar Oct 23 '22 19:10 anvuks

@anvuks I'll ask someone on the team to test your branch as well.

pictos avatar Oct 24 '22 04:10 pictos

Any update on this? I get this issue in production regularly and it looks like this PR will fix it 👍

greg84 avatar Mar 05 '23 11:03 greg84

Now that we're so close to the sunsetting of Xamarin (and therefore also Xamarin Community Toolkit) unfortunately we won't be able to take this in anymore, we're really sorry about that. Nevertheless, thank you so much for your time and effort that you have put into this PR, we really appreciate it.

Please have a look at the evolution of the Xamarin Community Toolkit, The .NET MAUI Community Toolkit. A lot of development has been going on there. Hopefully this issue was already fixed in that codebase. If not, feel free to port this over to there.

Again, thank you so much for being a contributor and Xamarin user!

jfversluis avatar Apr 26 '24 07:04 jfversluis