Media Element Full Screen Support for Android and Windows

[vhugogarcia]

Description of Change

@ne0rrmatrix and myself have been working very closely to have the full screen feature for Android and Windows implemented on the media element. It has been an amazing team work with collaborative meetings, etc. Implements probably one of the most wanted features from developers on which apps a media element is required.

Linked Issues/Discussions

• The initial discussion can be found here: Add Full Screen/Windows Support for `MediaElement` #929

PR Checklist

• Has a linked Issue, and the Issue has been approved(bug) or Championed (feature/proposal)
• Has tests (no need of unit tests)
• Has samples (if omitted, state reason in description)
• Rebased on top of main at time of PR
• Changes adhere to coding standard
• Documentation created or updated.

Additional information

MacOS and iOS already integrate the full screen feature, so no additional work is needed for those platforms. Please find below a couple of videos demoing the full screen on Android and Windows.

Windows Sample

MediaElement-Windows.mp4

Android Sample

MediaElement-Android.1.mp4

[pictos]

Great work guys! This looks great and the community will be very happy when this get merged. I've some suggestions for you

[bijington]

Just some potentially minor comments that you may just be able to ignore but I seem to recall we had some issues around Popups where it was messing with the system bar settings on Android

[bijington]

@pictos and @ne0rrmatrix I couldn't get this into the Discord channel for some reason but this is my initial attempt at docs page for the background Android application exception. Let me know what you think

[pictos]

Testing on Android:

Screen doesn't follow the screen rotation:

During my tests, I rotated the device, but the video didn't adjust correctly. See image below:

[vhugogarcia]

Testing on Android:

Screen doesn't follow the screen rotation:

During my tests, I rotated the device, but the video didn't adjust correctly. See image below:

Thanks @pictos for sharing the tests. This happens specifically on the emulator when you rotate it under a specific side, because the emulator does not know if you are trying to see the video upside/down. However, if you keep rotating the device you will see how it works. You can actually see it on the video demo @ne0rrmatrix shared with us on the initial description of the PR.

In any cases, James is currently trying to run the Sample app on a physical Android phone to get more information. 😃

[pictos]

For me, after the last request, it looks good to go!

[vhugogarcia]

@pictos the last commit improves the rotation issue you were facing on Android. James and I ran some tests to confirm. Thanks again for testing.

Let us know if approved, so we can proceed with the next steps.

[brminnick]

Hey Gang! This is looking really good and I know many many devs are super excited for this 💯

On Android, the full-screen buttons overlap the built-in software nav buttons at the bottom. The Android bottom navigation button bar blocks me from tapping on the MediaElement full-screen buttons.

In this screen capture, I am trying to click the button on the bottom-right to close the full screen, but the Android navigation buttons on the bottom are blocking me from clicking it.

👆 This screen capture is from a Android Pixel 7 emulator running Android API 29

[bijington]

Great work guys! For the documentation, do we have any behavioral changes? If not then we might not need to add anything.

[ne0rrmatrix]

Great work guys! For the documentation, do we have any behavioral changes? If not then we might not need to add anything.

Yes the LaunchMode.SingleInstance should be referenced in docs because developers need to know that. The app might behave incorrectly unless that is set. We will get bug reports if users implement full screen and do not use that. It will crash the app otherwise.

[bijington]

Great work guys! For the documentation, do we have any behavioral changes? If not then we might not need to add anything.

Yes the LaunchMode.SingleInstance should be referenced in docs because developers need to know that. The app might behave incorrectly unless that is set. We will get bug reports if users implement full screen and do not use that. It will crash the app otherwise.

Is that in addition to this change MicrosoftDocs/CommunityToolkit#370 or is the crash you are referring to the one mentioned in that PR?

[ne0rrmatrix]

Great work guys! For the documentation, do we have any behavioral changes? If not then we might not need to add anything.

Yes the LaunchMode.SingleInstance should be referenced in docs because developers need to know that. The app might behave incorrectly unless that is set. We will get bug reports if users implement full screen and do not use that. It will crash the app otherwise.

Is that in addition to this change MicrosoftDocs/CommunityToolkit#370 or is the crash you are referring to the one mentioned in that PR?

Yes it should be added. It is separate and is required. If you do not put that in it will not work when returning from background. It will result in bug reports.

[bijington]

@ne0rrmatrix and @vhugogarcia I can add some detail to the docs PR that we have open. Do we have anything more specific than "The app might behave incorrectly unless that is set." is there a common way in which the application won't behave? Or is this statement sufficient?

[ne0rrmatrix]

@ne0rrmatrix and @vhugogarcia I can add some detail to the docs PR that we have open. Do we have anything more specific than "The app might behave incorrectly unless that is set." is there a common way in which the application won't behave? Or is this statement sufficient?

@bijington The app will crash if returning from background process if this is not set. I would phrase it differently but I lack the proper language to describe it.

[vhugogarcia]

Hello @bijington I believe,it can be also something like this wording:

Warning: The app might behave incorrectly unless LaunchMode.SingleInstance is set in the main activity. This ensures that only one instance of the app is running at a time, and prevents the Media Element from being reinitialized or duplicated. To set the launch mode, add the following attribute to the MainActivity class in the Platforms/Android folder.

[ne0rrmatrix]

That sounds perfect. I feel that is a great explanation and resolves all issues I have. Thank you.

[bijington]

@ne0rrmatrix and @vhugogarcia how does this look: MicrosoftDocs/CommunityToolkit@44bc859

[vhugogarcia]

@ne0rrmatrix and @vhugogarcia how does this look: MicrosoftDocs/CommunityToolkit@44bc859

Looks great @bijington thanks for updating the docs 😃

[ne0rrmatrix]

@ne0rrmatrix and @vhugogarcia how does this look: MicrosoftDocs/CommunityToolkit@44bc859

Looks great. I think that is written very well and provide the needed info to developers. Thank you for updating the docs :)

[bijington]

Thanks @vhugogarcia and @ne0rrmatrix. I just added a question around the nullable warning that we have suppressed but the rest of the changes look good to me

[brminnick]

Great work guys!!

I'm good merging this, pending @bijington's review comment on the additional constructor.

[ne0rrmatrix]

Yup - I've experienced this error before.

As the error indicates, you need to add an overloaded constructor to the Android implementation of CommunityToolkit.Maui.Core.Views.MauiMediaElement.

No constructor found for CommunityToolkit.Maui.Core.Views.MauiMediaElement::.ctor(System.IntPtr, Android.Runtime.JniHandleOwnership)"

MauiMediaElement.android.cs

#pragma warning disable CS8618 // Non-nullable field is uninitialized. Consider declaring as nullable.
#pragma warning disable IDE0060 // Remove unused parameter
	public MauiMediaElement(nint ptr, JniHandleOwnership jni) : base(Platform.AppContext)
	{
		//Fixes no constructor found exception
	}
#pragma warning restore CS8618 // Non-nullable field is uninitialized. Consider declaring as nullable.
#pragma warning restore IDE0060 // Remove unused parameter

For example, here's how I implemented in my GitTrends app:

https://github.com/brminnick/GitTrends/blob/e0162223f7402ac018bf4e0c69182c589433ce6f/GitTrends.Android/CustomRenderers/VideoPlayerViewCustomRenderer.cs#L37C1-L46C59

I can see why an empty constructor can happen here. But I though setting Single Instance prevented this? I believe constructor gets called a second time when returning from background which crashes the app. But with single instance set it will not call constructor. I use that specifically to solve issues like this. If it does not get called a second time which it should not be when returning from background this is is unnecessary? Or am I wrong?

[brminnick]

But I though setting Single Instance prevented this? I believe constructor gets called a second time when returning from background which crashes the app. But with single instance set it will not call constructor. I use that specifically to solve issues like this. If it does not get called a second time which it should not be when returning from background this is is unnecessary? Or am I wrong?

I wish I knew the answer to this.

My vote is to keep the constructor; the code includes a link to the discussion we've had around it. We can always remove it in a future PR.

[bijington]

Thanks all for the detail and effort. This looks great!