-
Notifications
You must be signed in to change notification settings - Fork 410
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Proposal] Add new MediaElement constructor to allow creation of ExoPlayer as TextureView in Android (which will then allow overlaying, opacity, transparency, etc.) #1891
Comments
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:
|
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. |
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. |
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 FileAn 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:
2) Add BuildAttributes ClassCommunity Toolkit namespace would need the following enum and class added to it:
3) Add MediaElement ConstructorSo far as I can tell, there is no MediaElement constructor currently. One could be added with an optional parameter input like:
4) Add the Android TextureView constructorThe Android ExoPlayer StyledPlayerView is constructed in the MediaManager: https://github.com/CommunityToolkit/Maui/blob/ee03326ab43f35f90d27d92a37b9df06d6663f27/src/CommunityToolkit.Maui.MediaElement/Views/MediaManager.android.cs
We would need something like instead (presuming buildParameters is passed through to here also first):
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
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 GapsThe 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:
Thus here one could simply add a new function in the shared MediaManager class to pass them in like:
Thus one could change the above code to:
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. |
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. |
@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. |
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,
} |
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
|
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. |
Sample of transparency and colors: 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; |
Setting foreground color sets the image color property that sits on the foreground. 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? /// <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);
} |
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. BackgroundIf 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) APIFrom what I would expect, there should be two color settings - one for the background of the TextureView ( 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. AlphaAgain 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. But this is not important either way. Just my thought. 4) Log Message if set alpha on SurfaceViewI 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
|
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 https://developer.android.com/media/media3/ui/playerview
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 ie. Again, as they say:
Sorry @ne0rrmatrix 🙂 one more point to consider but then will be good to have |
@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 |
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. 🙂 |
Feature name
Add TextureView Option to Android ExoPlayer
Link to discussion
#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: #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 astexture_view
inside it to Android Resources folder, eg. Resources\layout with a name like "textureview.xml"2) Load the XML resource and construct the ExoPlayer with it:
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
This just needs to be changed to:
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 calledMediaElementBuildParameters
or something of that nature which we instantiate to parameterize and set inside.eg.
and/or then:
Then we would create MediaElement using this new constructor when wanted (old constructor would still also be available for typical usage) as either:
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 toMediaElementBuildParameters
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.
The text was updated successfully, but these errors were encountered: