search

CommunityToolkit / Maui

The .NET MAUI Community Toolkit is a community-created library that contains .NET MAUI Extensions, Advanced UI/UX Controls, and Behaviors to help make your life as a .NET MAUI developer easier
https://learn.microsoft.com/dotnet/communitytoolkit/maui
MIT License
2.1k stars 340 forks source link

[Proposal] Add new MediaElement constructor to allow creation of ExoPlayer as TextureView in Android (which will then allow overlaying, opacity, transparency, etc.) #1891

Open jonmdev opened 4 weeks ago

jonmdev commented 4 weeks ago

Feature name

Add TextureView Option to Android ExoPlayer

Link to discussion

https://github.com/CommunityToolkit/Maui/issues/1773

Progress tracker

Summary

ExoPlayer is the Google recommended video player in Android which is used in MediaElement.

ExoPlayer automatically constructs with the parameter surface_type as SurfaceView. This type of view "punches a hole" through the view hierarchy for efficiency. However, this also prevents layering/overlap of videos or usage of transparency, opacity, etc.

The alternative surface_type offered by ExoPlayer is TextureView which allows overlapping videos, transparency, opacity, etc. at a mild cost to efficiency.

Currently MediaElement only constructs as SurfaceView and there is no constructor to allow us to implement the TextureView constructor instead.

Adding a new MediaElement constructor with this option will solve this without much work and unlock many features and solutions in Android.

Motivation

As discussed here: https://github.com/CommunityToolkit/Maui/issues/1773

TextureView will fix issues like overlapping videos "punching through" one another inappropriately (eg. split screen, picture in picture, or otherwise overlapping displays).

It will also allow implementation of solutions for video transparency as per: https://medium.com/go-electra/unlock-transparency-in-videos-on-android-5dc43776cc72

Detailed Design

A TextureView ExoPlayer is constructed with the following approach in .NET:

1) Save an XML file with the surface_type declared as texture_view inside it to Android Resources folder, eg. Resources\layout with a name like "textureview.xml"

<?xml version="1.0" encoding="utf-8" ?> 
<com.google.android.exoplayer2.ui.StyledPlayerView 
    xmlns:android="http://schemas.android.com/apk/res/android"
    xmlns:app="http://schemas.android.com/apk/res-auto"
    android:id="@+id/testname"
    app:surface_type="texture_view" />

2) Load the XML resource and construct the ExoPlayer with it:

XmlReader xmlResource = this.Resources.GetXml(Resource.Layout.textureview);
xmlResource.Read();
Android.Util.IAttributeSet attributes = Android.Util.Xml.AsAttributeSet(xmlResource);
Com.Google.Android.Exoplayer2.UI.StyledPlayerView styledPlayerView = new(this, attributes); 
xmlResource.Dispose();

System.Diagnostics.Debug.WriteLine("SURFACE TYPE " + styledPlayerView.VideoSurfaceView.GetType()); 
//default is SurfaceView, will return TextureView with this attribute correctly fed into the constructor

IMPLEMENTATION INTO COMMUNITY TOOLKIT:

1) PlayerView Constructor

The StyledPlayerView is constructed here: https://github.com/CommunityToolkit/Maui/blob/ee03326ab43f35f90d27d92a37b9df06d6663f27/src/CommunityToolkit.Maui.MediaElement/Views/MediaManager.android.cs#L43C1-L43C57

PlayerView = new StyledPlayerView(MauiContext.Context)

This just needs to be changed to:

PlayerView = new StyledPlayerView(MauiContext.Context, attributes) //StyledPlayerView has optional attributes parameter

Where attributes is the attributes file constructed above in cases where TextureView is desired, optionally taken by this constructor.

2) MediaElement Constructor

MediaElement currently only has one constructor which takes no arguments. An extra new constructor would need to be added to pass the argument for the TextureView through when it is wanted.

One could either (1) add a simple enum for the AndroidSurfaceType, and or (2) wrap this inside a class called MediaElementBuildParameters or something of that nature which we instantiate to parameterize and set inside.

eg.

public enum AndroidSurfaceType {
    SurfaceView,
    TextureView
}

and/or then:

public class MediaElementBuildParameters {
    public AndroidSurfaceType androidSurfaceType = AndroidSurfaceType.SurfaceView;
    //can add future constructor parameters here
}

Then we would create MediaElement using this new constructor when wanted (old constructor would still also be available for typical usage) as either:

MediaElement simpleOption = new(AndroidSurfaceType.TextureView);
MediaElement betterOption = new(new MediaElementBuildParameters(){ androidSurfaceType = AndroidSurfaceType.TextureView });

If we anticipate there may be other construction related parameters that may come up over time, having the class to pass in (MediaElementBuildParameters) containing the enum (AndroidSurfaceType) makes more sense as it then keeps the API generalizable. Further construction parameters could be added to MediaElementBuildParameters for other purposes if needed down the line.

This input would then need to be passed along through to the StyledPlayerView constructor of course as well so it can happen.

Usage Syntax

As noted above for C#.

I do not program Maui in XAML (I only use C#) so I am not familiar with how it actually works but I presume it would be simple to extrapolate from what I wrote above if you understand XAML.

Drawbacks

I cannot think of any negative issues from this solution. It would take very few lines of code and it only expands the usability so we can solve currently unsolvable problems.

Otherwise users need to build their own video players from scratch to solve this which is the nightmare I am looking at otherwise. It makes no good sense to re-engineer the whole MediaElement just for one missing feature when it can be added so easily with so few lines of code.

Alternatives

There is no alternative except to tell all Maui users they must build their own video player from scratch if they want to use the full features of Android video playback.

Unresolved Questions

It is debatable whether it makes sense to just have an enum as the argument for the constructor and/or a class that contains the parameters (including the enum) for construction.

Having a "build parameters" class containing the "surface type" enum rather than just the enum alonewould open up possibilities for easily solving any other constructor related issues that might come up down the line perhaps. If it was me, for that reason, that's how I'd probably personally do it. But either will work fine.

brminnick commented 4 weeks ago

This all makes sense and looks good to me! I'll add it to the agenda for our next standup to bring it to the group.

I'm no Android expert, so I have a few curious questions:

jonmdev commented 4 weeks ago

This all makes sense and looks good to me! I'll add it to the agenda for our next standup to bring it to the group.

I'm no Android expert, so I have a few curious questions:

Is TextureView better than SurfaceView in every way? Should we make TextureView the default implementation? Is there any reason to keep SurfaceView after implementing TextureView?

Great! Thanks. SurfaceView is the intentional default of ExoPlayer because it is more efficient for playback. In order for TextureView to have the opacity and overlapping behaviors, it functions behind the scenes by essentially copying the video playback onto a regular view. This lets it behave like a regular view but at the cost of copying the data each frame.

So Android recommends using SurfaceView in all typical circumstances, and TextureView only when overlapping or opacity or other features are needed. This is discussed a bit more in these threads and articles for example:

So it would be most desirable to keep SurfaceView as default. But adding the TextureView option takes nothing away from anyone, and as noted requires very little code from what I can see to make work with the already existing system design.

Adding the optional constructor to specify building as TextureView will only increase the feature-set in Android and allow us to do a lot of things we can't now. For example, I am stuck because I need overlapping videos which won't currently cooperate under SurfaceView. It will be quite brutal to build an entire new VideoPlayer class when this one is already running well and just needs a few lines of code to add this option.

I am happy to do any further work if needed to solve any issues but I think it should be pretty straightforward. Thanks again for your support on this.

ne0rrmatrix commented 4 weeks ago

This is a great idea. I fully support it! @jonmdev has a good idea and I am happy to help if any is needed. I agree that keeping surfaceview as default is recommended. Having an enum seems like a good idea as we may want to depending on circumstances have more options in the future. Not sure what but keeping our options open is a good idea.

jonmdev commented 4 weeks ago

As a further update, while I believe I have been able to think this proposal through as far as I can, I do not have the expertise and knowledge to manage the actual implementation. I have never contributed directly to a project or library like this and I am not a high level pro. Eg. I have never built a NuGet package or tested one.

So I would hope someone from the project with more experience than me could try the implementation.

But I am not trying to be lazy, so I have written the most detailed workflow I can think through for the implementation here which follows.

Proposed Implementation Steps:

1) Add TextureView XML File

An XML file like "textureview.xml" must be added to Android Resources folder in the Community Toolkit project such that it will be packed into the NuGet with everything and still accessible to the following Community Toolkit code. File would contain:

<?xml version="1.0" encoding="utf-8" ?> 
<com.google.android.exoplayer2.ui.StyledPlayerView 
    xmlns:android="http://schemas.android.com/apk/res/android"
    xmlns:app="http://schemas.android.com/apk/res-auto"
    app:surface_type="texture_view" />

2) Add BuildAttributes Class

Community Toolkit namespace would need the following enum and class added to it:

public enum AndroidSurfaceType {
    SurfaceView,
    TextureView
}

public class MediaElementBuildParameters {
    public AndroidSurfaceType androidSurfaceType = AndroidSurfaceType.SurfaceView;
    //using class like this to wrap the enum allows us to add future constructor parameters later without changing MediaElement constructor API
}

3) Add MediaElement Constructor

So far as I can tell, there is no MediaElement constructor currently. One could be added with an optional parameter input like:

https://github.com/CommunityToolkit/Maui/blob/567eee2d1a1b2a3d3d6eba014b877027eaf31623/src/CommunityToolkit.Maui.MediaElement/MediaElement.shared.cs

private MediaElementBuildParameters buildParameters = null;

public MediaElement(MediaElementBuildParameters buildParameters = null) {
    // or public MediaElement(MediaElementBuildParameters? buildParameters) {
    this.buildParameters = buildParameters; //save this parameter for when handler exists and pass it in there when you can
}

4) Add the Android TextureView constructor

The Android ExoPlayer StyledPlayerView is constructed in the MediaManager: https://github.com/CommunityToolkit/Maui/blob/ee03326ab43f35f90d27d92a37b9df06d6663f27/src/CommunityToolkit.Maui.MediaElement/Views/MediaManager.android.cs

PlayerView = new StyledPlayerView(MauiContext.Context)

We would need something like instead (presuming buildParameters is passed through to here also first):

if (buildParameters !=null && buildParameters.androidSurfaceType == AndroidSurfaceType.textureView) {

    //BUILD THE TEXTUREVIEW EXOPLAYER
    System.Xml.XmlReader xmlResource = this.Resources.GetXml(Resource.Layout.textureview); //or whatever address the XML file is at
    xmlResource.Read();
    Android.Util.IAttributeSet attributes = Android.Util.Xml.AsAttributeSet(xmlResource);
    PlayerView = new StyledPlayerView(MauiContext.Context, attributes) //StyledPlayerView has optional attributes parameter
    System.Diagnostics.Debug.WriteLine("SURFACE TYPE " + styledPlayerView.VideoSurfaceView.GetType());  //just to see if working, should return TextureView here
    xmlResource.Dispose();

}
else {
    PlayerView = new StyledPlayerView(MauiContext.Context) //no attributes needed for SurfaceView as that is default
}

5) Re-Map Events? Likely not needed...

As per this reference there are slightly different events for SurfaceView and TextureView: https://medium.com/androiddevelopers/android-hdr-migrating-from-textureview-to-surfaceview-part-1-how-to-migrate-6bfd7f4b970e

onSurfaceTextureAvailable() == surfaceCreated() onSurfaceTextureDestroyed() == surfaceDestroyed() onSurfaceTextureSizeChanged() is similar to surfaceChanged()

However searching the Maui code I don't see any of these events even used so probably nothing needs to be done regarding this.

6) Close the Gaps

The only gaps I'm aware of then would be how to: (1) Send the build attributes from MediaElement into the MediaElementHandler, and then (2) Send them from the MediaElementHandler into the MediaManager where they would be used as above.

For (2), as far as I can see, MediaManager is created in the MediaElementHandler with code like:

mediaManager ??= new(MauiContext ?? throw new InvalidOperationException($"{nameof(MauiContext)} cannot be null"),
                                VirtualView,
                                Dispatcher.GetForCurrentThread() ?? throw new InvalidOperationException($"{nameof(IDispatcher)} cannot be null"));

        var (_, playerView) = mediaManager.CreatePlatformView();

https://github.com/CommunityToolkit/Maui/blob/567eee2d1a1b2a3d3d6eba014b877027eaf31623/src/CommunityToolkit.Maui.MediaElement/Handlers/MediaElementHandler.android.cs

Thus here one could simply add a new function in the shared MediaManager class to pass them in like:

private MediaElementBuildAttributes buildAttributes;
public void setMediaElementBuildAttributes(MediaElementBuildAttributes buildAttributes){
    this.buildAttributes = buildAttributes;
}

Thus one could change the above code to:

mediaManager ??= new(MauiContext ?? throw new InvalidOperationException($"{nameof(MauiContext)} cannot be null"),
                                VirtualView,
                                Dispatcher.GetForCurrentThread() ?? throw new InvalidOperationException($"{nameof(IDispatcher)} cannot be null"));

    //pass in Build Attributes before "CreatePlatformView":
    mediaManager.setBuildAttributes(buildAttributes); //just pass in the build attributes for use in the platforms if needed

        var (_, playerView) = mediaManager.CreatePlatformView();

The last gap would then be getting the buildAttributes from the MediaElement into the MediaElementHandler. I have at this point lost the forest for the trees, but I presume this would be trivial for someone who understands better how the handlers are created (I am not that person I suppose).

Thoughts?

That is unfortunately the best I can do and I would have to leave the implementation to someone better than me. If anyone tackles this and runs into any walls I'm happy to try to research find solutions to get through, but I suspect this will work.

ne0rrmatrix commented 4 weeks ago

I am happy to start working on this. We will need to create a sample to demonstrate how this works and also update the docs when we do the pull request. If you are willing to work with me and help me with the sample and testing out the project I am happy to create the code. We can get started once this is approved. I will in the meantime try and figure out a way to make this work. I have done work on this before and think I may have a way to avoid having to write any xaml.

jonmdev commented 4 weeks ago

I am happy to start working on this. We will need to create a sample to demonstrate how this works and also update the docs when we do the pull request. If you are willing to work with me and help me with the sample and testing out the project I am happy to create the code. We can get started once this is approved. I will in the meantime try and figure out a way to make this work. I have done work on this before and think I may have a way to avoid having to write any xaml.

Sure absolutely. Please let me know what to do. At least in C# it is very little code needed so I suspect it will all go smoothly.

ne0rrmatrix commented 3 weeks ago

@jonmdev have you joined the community discord? BTW I have a sample with texture viewer working and I am in the process of testing various stuff with it. I will be playing around with it today. My current implementation only allows setting it at launch of media element which I want to switch dynamically but that is for later in the process. I just want to verify that it is actually texture view and would appreciate some ideas to showcase and test if this is working. I was able to take what you had and essentially get it down to 4 lines of code which is easier to use than adding a ton of xml and rewriting the handler implementation which I don't really want to mess with at this time.

ne0rrmatrix commented 3 weeks ago

How does this look for an API

/// <summary>
/// Backing store for the <see cref="AndroidSurface"/> property.
/// </summary>
public static readonly BindableProperty AndroidSurfaceProperty =
    BindableProperty.Create(nameof(AndroidSurface), typeof(AndroidSurfaceType), typeof(MediaElement), AndroidSurfaceType.SurfaceView);

/// <summary>
/// Backing store for the <see cref="AndroidColorHEX"/> property.
/// </summary>
public static readonly BindableProperty AndroidColorHEXProperty =
    BindableProperty.Create(nameof(AndroidColorHEX), typeof(string), typeof(MediaElement), "#000000");

Does setting the color with alpha look like the sort of thing you want to do? Or should we also be setting the player view alpha too as we can also set the transparency of the controls? That would be separate but can be done.

Here is the enum for surface type:

namespace CommunityToolkit.Maui.Core.Primitives;
/// <summary>
/// Android surface type for the <see cref="IMediaElement"/>.
/// </summary>
public enum AndroidSurfaceType
{
    /// <summary>
    /// A <see cref="SurfaceView"/> is used for rendering the media.
    /// </summary>
    SurfaceView,

    /// <summary>
    /// A <see cref="TextureView"/> is used for rendering the media.
    /// </summary>
    TextureView,
}
jonmdev commented 3 weeks ago

@jonmdev have you joined the community discord? BTW I have a sample with texture viewer working and I am in the process of testing various stuff with it. I will be playing around with it today. My current implementation only allows setting it at launch of media element which I want to switch dynamically but that is for later in the process. I just want to verify that it is actually texture view and would appreciate some ideas to showcase and test if this is working. I was able to take what you had and essentially get it down to 4 lines of code which is easier to use than adding a ton of xml and rewriting the handler implementation which I don't really want to mess with at this time.

Wow that is phenomenal. You are very fast! I am very curious how you managed to set the ExoPlayer as TextureView without loading the XML snippet for this attribute from Resources. I spent days/weeks looking for a way to do that and posted all over the Internet with no solution found.

In any case, there are two immediate ways to test if it is successfully a TextureView:

1) Debug the styledPlayerView.GetType()

By default this will return "SurfaceView" but if you build a TextureView this will then return "TextureView".

2) Use my Existing Bug Project to Test:

https://github.com/CommunityToolkit/Maui/issues/1730

In that bug report I detail a test project I made where I have two videos on screen at once (two MediaElements). Both fill the screen, but each is clipped by Maui code to show on only one half of the screen (left for one, right for other). As you see in that project, in Windows and iOS this works fine with side by side video showing.

But in Android (because of the SurfaceView) the clipping is not respected and you get just one video showing full screen.

If this can be built as TextureView, it should automatically respect the clipping and the Android build will then look the same as Windows/iOS.

What is the Discord channel? I see two linked here: https://dotnet.microsoft.com/en-us/platform/community

One for C# community and the other .NET development. Is that it?

Regarding the Hex Color etc you are mentioning, I am not sure of the utilization or purpose. As I understand it, once this is switched to TextureView, we should be able to change the background color and opacity just like we do with any other Maui view.

Ie. 

MediaElement mediaElement = new([textureviewcommand]);
mediaElement.Opacity = 0.5;
mediaElement.BackgroundColor = Colors.AliceBlue;

Please keep in mind the ExoPlayer does not support transparent video playback (ie. alpha channel) without major workarounds so don't expect that to work regardless of SurfaceView or TextureView out of the box https://medium.com/go-electra/unlock-transparency-in-videos-on-android-5dc43776cc72

I am not sure anything extra should be needed beyond that here but perhaps I am naive. Perhaps the Opacity or other MediaElement parameters will need to passed through to the styledPlayerView if they are not already, as I believe that is the true native platform view to manipulate here whether TextureView or SurfaceView.

Thanks again and I am very curious to see how you did it.

Or re: API, is your point about keeping the controls (play button etc) separately parameterized in terms of color/opacity from the video playback view (ie styledPlayerView)? If so, that is a great idea. Users may want to change the opacity of the styledPlayerView separate from the MediaElement control buttons.

I did not even think of that possibility as I am not using the controls buttons (play etc) typically in my application. I think that is a smart idea to be able to control those separately though I defer to you on how to implement it as I am not sure of the mechanics.

If it was me, and that is what you mean, I might lean toward something like:

mediaElement.Opacity = 0.5; //global opacity for controls and view
mediaElement.OpacityVideo = 0.5; //specific opacity for video player view (multiplied in effect by global, so here 0.25 in effect)
mediaElement.OpacityControls = 1; //specific opacity for video player controls (multiplied in effect by global, so here 0.5 in effect)

Thanks again.

ne0rrmatrix commented 3 weeks ago

I'm mostly basing what I am doing based on your specifications. I have no real idea what you intended so every detail helps.

jonmdev commented 3 weeks ago

I'm mostly basing what I am doing based on your specifications. I have no real idea what you intended so every detail helps.

Thanks. That is fair.

Here is a list of things I can think it should be able to do if it is working correctly (ie. behaviors I would hope for):

1) Clipping

2) Opacity

3) Playback Controls

4) Background Color

5) GetAndroidSurface()

The two main problems I am personally trying to solve are: (1) Overlapping videos not cooperating with their Maui clipping (like my bug report), and (2) Eventually implementing the above workaround for transparent video (alpha video support).

So at least from my perspective and what I would hope for, if the solution can allow the above, this would be Christmas has come early.

Does that answer what you were wondering? Hope that helps and thanks again for your efforts.

ne0rrmatrix commented 3 weeks ago

Sample of transparency and colors: Setting PlayerView color and transparency affects the controls. Setting Foreground color and transparency affects the video image.

PlayerView.SetBackgroundColor(Android.Graphics.Color.Transparent);
PlayerView.Foreground = new Android.Graphics.Drawables.ColorDrawable(Android.Graphics.Color.White);
PlayerView.Foreground.Alpha = 128;
PlayerView.Alpha = 0.50f;

image

ne0rrmatrix commented 3 weeks ago

Setting foreground color sets the image color property that sits on the foreground. Setting foreground alpha sets the transparency of color on top of image. Setting background color sets video controls color property. Setting the background alpha sets the video controls transparency.

This all assumes you are using TextureView. Otherwise it behaves as before and ignores all of the transparencies and colors. Or we could throw an error or create a log warning? How is this for API:

/// <summary>
/// Backing store for the <see cref="AndroidSurface"/> property.
/// </summary>
public static readonly BindableProperty AndroidSurfaceProperty =
    BindableProperty.Create(nameof(AndroidSurface), typeof(AndroidSurfaceType), typeof(MediaElement), AndroidSurfaceType.SurfaceView);

/// <summary>
/// Backing store for the <see cref="PlayerBackgroundColor"/> property.
/// </summary>
public static readonly BindableProperty PlayerBackgroundColorProperty =
    BindableProperty.Create(nameof(PlayerBackgroundColor), typeof(MediaElementColor), typeof(MediaElement), MediaElementColor.Default);

/// <summary>
/// Backing store for the <see cref="PlayerForegroundColor"/> property.
/// </summary>
public static readonly BindableProperty PlayerForegroundColorProperty =
    BindableProperty.Create(nameof(PlayerForegroundColor), typeof(MediaElementColor), typeof(MediaElement), MediaElementColor.Default);

/// <summary>
/// Backing store for the <see cref="PlayerAlpha"/> property.
/// </summary>
public static readonly BindableProperty PlayerAlphaProperty =
    BindableProperty.Create(nameof(PlayerAlpha), typeof(float), typeof(MediaElement), 1.0f);

/// <summary>
/// Backing store for the <see cref="PlayerForegroundAlpha"/> property.
/// </summary>
public static readonly BindableProperty PlayerForegroundAlphaProperty = 
    BindableProperty.Create(nameof(PlayerForegroundAlpha), typeof(int), typeof(MediaElement), 255);

/// <inheritdoc cref="IMediaElement.PlayerAlpha"/>
public float PlayerAlpha
{
    get => (float)GetValue(PlayerAlphaProperty);
    set => SetValue(PlayerAlphaProperty, value);
}

/// <inheritdoc cref="IMediaElement.PlayerForegroundAlpha"/>
public int PlayerForegroundAlpha
{
    get => (int)GetValue(PlayerForegroundAlphaProperty);
    set => SetValue(PlayerForegroundAlphaProperty, value);
}

/// <inheritdoc cref="IMediaElement.PlayerBackgroundColor"/>
public MediaElementColor PlayerBackgroundColor
{
    get => (MediaElementColor)GetValue(PlayerBackgroundColorProperty);
    set => SetValue(PlayerBackgroundColorProperty, value);
}

/// <inheritdoc cref="IMediaElement.PlayerForegroundAlpha"/>
public MediaElementColor PlayerForegroundColor
{
    get => (MediaElementColor)GetValue(PlayerForegroundColorProperty);
    set => SetValue(PlayerForegroundColorProperty, value);
}

/// <inheritdoc cref="IMediaElement.AndroidSurface"/>
public AndroidSurfaceType AndroidSurface
{
    get => (AndroidSurfaceType)GetValue(AndroidSurfaceProperty);
    set => SetValue(AndroidSurfaceProperty, value);
}
jonmdev commented 2 weeks ago

Setting foreground color sets the image color property that sits on the foreground. Setting foreground alpha sets the transparency of color on top of image. Setting background color sets video controls color property. Setting the background alpha sets the video controls transparency.

This all assumes you are using TextureView. Otherwise it behaves as before and ignores all of the transparencies and colors. Or we could throw an error or create a log warning?

Sample of transparency and colors: Setting PlayerView color and transparency affects the controls. Setting Foreground color and transparency affects the video image.

PlayerView.SetBackgroundColor(Android.Graphics.Color.Transparent);
PlayerView.Foreground = new Android.Graphics.Drawables.ColorDrawable(Android.Graphics.Color.White);
PlayerView.Foreground.Alpha = 128;
PlayerView.Alpha = 0.50f;

Looks great that you seem to be getting it working! A few points of my thoughts to comment on for feedback or ideas.

1) Foreground vs. Background

If the design is to be split into "foreground" and "background" where one represents the controls (play button, etc) and the other represents the video view (ie. TextureView), would it not make more sense for "background" to be the video view (TextureView) and "foreground" to be the controls (play button, etc.)?

Since the controls are seen overlaying (in front of) the video, it seems more logical they should be called "foreground" and the video (which is behind the controls) should be called "background" if that is the terminology used.

Perhaps that is what you meant?

2) API

From what I would expect, there should be two color settings - one for the background of the TextureView (PlayerView.SetBackgroundColor) and another for the color of the controls (play button etc.).

And there should be two opacity settings - one for the TextureView and another for the controls.

Based on your naming convention for the functions I would see this as:

3) Opacity vs. Alpha

Again in just semantics, I would personally go with the term "Opacity" over "Alpha". I understand Android uses the term "Alpha" but Maui goes with "Opacity" everywhere else so for consistency, I would stick with "Opacity", ie. PlayerOpacity or PlayerForegroundOpacity.

But this is not important either way. Just my thought.

4) Log Message if set alpha on SurfaceView

I would think it is better to just have a log message if you attempt to set opacity/alpha on a SurfaceView. If it is descriptive it will probably avoid unnecessary confusion and false bug reports. This message could say something like "Cannot set opacity of MediaElement in Android when it is built as SurfaceView. To set opacity, you must first configure MediaElement with mediaElement.AndroidSurface = AndroidSurfaceType.TextureView".

Avoiding any true errors (just showing log message) will allow us to switch back and forth in testing between TextureView and SurfaceView without having to comment out any other code.

5) Ensure proper nulling of setBackgroundColor

Just as an FYI again, please note that setBackgroundColor(Android.Graphics.Color.Transparent) is not the same as setBackgroundColor(null). Using Transparent will create overdraw. Whereas setBackgroundColor(null) does not overdraw. This is discussed in my prior linked bug thread I mentioned above or here for example also: https://stackoverflow.com/a/43790016/10305478

So please just make sure if the system is set to null for background color (or any relevant color) it passes this through as setBackgroundColor(null) as well to the Android View to avoid overdraw. The default configuration should be setBackgroundColor(null) as well.

6) Accessing Android StyledPlayerView?

As noted previously, Google has shown zero willpower in adding support for transparent video so far. They developed VP9 and released it 11 years ago and we still can't playback transparent video (support for alpha video channel). Thus the only workaround I know of is this one: https://medium.com/go-electra/unlock-transparency-in-videos-on-android-5dc43776cc72

In order to do that workaround, we need some access to a TextureView in order to render to it manually. This may be a bust, but I would appreciate a method to access the styledPlayerView (TextureView) to experiment with. Or otherwise, do you have any suggestion for how it can be reached by Reflection?

I think one would have to reach it by MediaElementHandler > mediaManager > PlayerView through Reflection...

7) What happens to MediaElement.Opacity?

I am just curious - what would mediaElement.Opacity end up doing (in any OS) currently or after these changes? Does this currently set opacity of both controls (play button etc) and view (video)? As mentioned, I am not using control buttons so don't know.

Would the new PlayerOpacity and PlayerForegroundOpacity be used in all OS's or just Android? ie. Would mediaElement.Opacity become redundant?

I see no problem with mediaElement.Opacity becoming unnecessary/redundant. Or it could set the opacity of both video and controls together. ie. mediaElement.Opacity = 0.5 would set both video and controls to 50% opacity in any OS. Whereas PlayerOpacity and PlayerForegroundOpacity would set the individual video or control opacities in any OS.

This change would then give 3 functions to control opacity. Or alternatively if this is more work or doesn't automatically work, we could just stop using mediaElement.Opacity. It is not too important.

8) What happens to MediaElement.BackgroundColor?

Similarly, I am then just wondering what happens to this mediaElement.BackgroundColor. Intuitively I would presume it would make sense to to have this automatically map to the same outcome as mediaElement.PlayerBackgroundColor ie. the background of the video view.

It would then be a bit redundant. I am not sure how you have to handle things from a coding perspective on this. It is not too important in my opinion what happens to mediaElement.BackgroundColor or what it does. We can just not use it altogether same as mediaElement.Opacity. Whatever works.

Thanks again for all your work. Hopefully this all at least makes sense. Please let me know if any further thoughts or points to consider.

bijington commented 2 weeks ago

Out of interest will this be necessary once we migrate away from ExoPlayer and onto Media3? I don't know the timeline for that but I just wanted to make sure we are all aware and don't do something that might be thrown away in the future

jonmdev commented 2 weeks ago

Out of interest will this be necessary once we migrate away from ExoPlayer and onto Media3? I don't know the timeline for that but I just wanted to make sure we are all aware and don't do something that might be thrown away in the future

To my knowledge, I have looked into it and Media3 doesn't change anything except the locations of the classes. Ie. There is still SurfaceView and TextureView and these work the same. ExoPlayer is just being moved around. So these still need to be accommodated to get full function in Android.

You can see Android's documentation on PlayerView and ExoPlayer here for media3 and they describe the same system for surface_type that exists now:

https://developer.android.com/media/media3/ui/playerview https://developer.android.com/media/media3/exoplayer

The surface_type attribute of PlayerView lets you set the type of surface used for video playback. Besides the values spherical_gl_surface_view (which is a special value for spherical video playback) and video_decoder_gl_surface_view (which is for video rendering using extension renderers), the allowed values are surface_view, texture_view and none. If the view is for audio playback only, none should be used to avoid having to create a surface because doing so can be expensive.

If the view is for regular video playback then surface_view or texture_view should be used. SurfaceView has a number of benefits over TextureView for video playback ...

On that note, to add one more point, if we are using MediaElement for audio (it looks like that is the intended design: "Play Audio and Video in .NET MAUI apps with the new MediaElement": https://devblogs.microsoft.com/dotnet/announcing-dotnet-maui-communitytoolkit-mediaelement/) then having the none option would be good as well for audio playback to avoid creating unnecessary SurfaceView or TextureView objects as suggested by Android in the quote above.

ie. Again, as they say:

If the view is for audio playback only, none should be used to avoid having to create a surface because doing so can be expensive.

Sorry @ne0rrmatrix 🙂 one more point to consider but then will be good to have none as SurfaceType also for audio only MediaElement option...

ne0rrmatrix commented 1 week ago

@jonmdev Yes it would be nice to have a surface type for audio. I would not recommend using none. Using audio if we use any controls it would default to SurfaceView and use that to display controls. If you do not explicitly set the surface it will default to SurfaceView and only by explicitly setting TextureView can we use it. So although it might be nice to use none it would be an issue if they also used controls and did not realize they were using a default view for media element. I do not believe it is possible to not have a View at all for exoplayer.

jonmdev commented 1 week ago

@jonmdev Yes it would be nice to have a surface type for audio. I would not recommend using none. Using audio if we use any controls it would default to SurfaceView and use that to display controls. If you do not explicitly set the surface it will default to SurfaceView and only by explicitly setting TextureView can we use it. So although it might be nice to use none it would be an issue if they also used controls and did not realize they were using a default view for media element. I do not believe it is possible to not have a View at all for exoplayer.

Thanks! If the view is necessary for it to build and add to the hierarchy and work properly in Maui, then I would not bother adding a third option. People can decide whether they want their audio 'surface' to be texture view or surface view. There may be quirks to explore once we start playing with it in real use. Being able to use either for audio might still help in some way. I would then just leave the two options only.

Hope the work is going well. I am eager and excited to try it. 🙂