Skip to content

Conversation

ne0rrmatrix
Copy link
Contributor

@ne0rrmatrix ne0rrmatrix commented May 7, 2025

  • Feature/Proposal

Description of Change

Updated MediaElement to include AndroidForegroundServiceEnabled property for better control over media playback on Android. Adjusted related methods and classes to handle this new property, ensuring the foreground service is utilized appropriately during media playback. Removed unnecessary properties and streamlined the popup media element configuration.

Linked Issues

PR Checklist

Additional information

This PR is for the community to see how making android service optional could be implemented.
Builder Options

.UseMauiCommunityToolkitMediaElement(static options =>
{
				options.SetDefaultAndroidViewType(AndroidViewType.TextureView);
				options.SetDefaultAndroidForegroundService(true);
})

MediaElement.shared.cs

internal bool IsAndroidForegroundServiceEnabled { get; init; } = MediaElementOptions.IsAndroidForegroundServiceEnabled;

MediaElementOptions

/// <summary>
/// Set Android Foreground Service for MediaElement on construction
/// </summary>
internal static bool IsAndroidForegroundServiceEnabled { get; private set; } = true;

/// <summary>
/// Set Android Foreground Service for MediaElement on construction
/// </summary>
/// <param name="androidForegroundServiceEnabled"></param>
public void SetDefaultAndroidForegroundService(bool androidForegroundServiceEnabled) => IsAndroidForegroundServiceEnabled = androidForegroundServiceEnabled;

Updated MediaElement to include AndroidForegroundServiceEnabled property for better control over media playback on Android. Adjusted related methods and classes to handle this new property, ensuring the foreground service is utilized appropriately during media playback. Removed unnecessary properties and streamlined the popup media element configuration.
@ne0rrmatrix ne0rrmatrix added do not merge Do not merge this PR 📽️ MediaElement Issue/PR that has to do with MediaElement labels May 7, 2025
Copy link
Contributor

@bijington bijington left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the general approach looks good. I am surprised it takes such little code change.

I think we should add an Is prefix to the property to follow typical Boolean naming conventions (e.g. IsAndroidForegroundServiceEnabled)

I understand that you wanted to have Android as the initial prefix here. Why did we do this for the AndroidViewType property in stead of making it a property that only exists for Android? Was it to simplify the code that needs to be written?

Renamed `AndroidForegroundServiceEnabled` to `IsAndroidForegroundServiceEnabled` across multiple files, including XAML, C#, and shared code. Updated the default value for the foreground service option to true in `MediaElementOptions`. Added unit tests to verify the default behavior and the ability to disable the foreground service setting. These changes improve clarity and maintainability by standardizing the naming convention.
@ne0rrmatrix
Copy link
Contributor Author

ne0rrmatrix commented May 7, 2025

I understand that you wanted to have Android as the initial prefix here. Why did we do this for the AndroidViewType property in stead of making it a property that only exists for Android? Was it to simplify the code that needs to be written?

I honestly have no idea how or when we came up with that. I am simply using the same convention used in texture view as it is has already been established and keeping it the same made sense to me.

edit: I think it was so that we can use it in xaml.

@mrucker
Copy link

mrucker commented May 7, 2025

Do these code changes also remove the foreground service permission from the Android manifest file? That is the real problem, because that is what Google checks when you submit to the app store.

@Elinares-82
Copy link

Hi all, we just got rejected out app since the MediaElement control added this permission. We only use this control to display a video in the Splash screen. When can we get this new CommunityToolkit nuget version? In the meantime, we will appeal the rejection, however, this is just to see if they understand the situation. I would appreciate it if we could move a little faster. Thanks

@mrucker
Copy link

mrucker commented May 7, 2025

@Elinares-82 there's an easy work around for this in the meantime. You just need to remove like 3 lines of code from the MAUI project, build it, and then include the package you just built in your app instead of the Nuget package. It took me like an hour to get a version in my app without the foreground permission requirement.

@Elinares-82
Copy link

Elinares-82 commented May 7, 2025

@Elinares-82 there's an easy work around for this in the meantime. You just need to remove like 3 lines of code from the MAUI project, build it, and then include the package you just built in your app instead of the Nuget package. It took me like an hour to get a version in my app without the foreground permission requirement.

Thanks!, I will give it a try.

@bijington
Copy link
Contributor

Hi all, we just got rejected out app since the MediaElement control added this permission. We only use this control to display a video in the Splash screen. When can we get this new CommunityToolkit nuget version? In the meantime, we will appeal the rejection, however, this is just to see if they understand the situation. I would appreciate it if we could move a little faster. Thanks

I think a PR opened within a few hours is moving pretty quickly. If you would like things to move more quickly please free to get involved and help out

@Elinares-82
Copy link

Hi all, we just got rejected out app since the MediaElement control added this permission. We only use this control to display a video in the Splash screen. When can we get this new CommunityToolkit nuget version? In the meantime, we will appeal the rejection, however, this is just to see if they understand the situation. I would appreciate it if we could move a little faster. Thanks

I think a PR opened within a few hours is moving pretty quickly. If you would like things to move more quickly please free to get involved and help out

Sure, give me the permissions and I click on Approve button. The Play Store is rejecting our apps since you decided to add permissions by default, why? Let the developers add or not the permissions.

Copy link
Member

@pictos pictos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that you added the property inside the MediaElement, how is the behavior if I have two media elements in different pages, one setting foreground to true and other to false, what will be the behavior? I didn't see code to tear down the ForegroundService if it changes to false in fly.

But I think it should only be something that you set on AppBuilder and only once

@Elinares-82
Copy link

That I could see is prevent to start the service here
image
I'm taking the @ne0rrmatrix branch a create an assembly in the meantime, since we need our app get approved ASAP.

@ne0rrmatrix
Copy link
Contributor Author

I see that you added the property inside the MediaElement, how is the behavior if I have two media elements in different pages, one setting foreground to true and other to false, what will be the behavior? I didn't see code to tear down the ForegroundService if it changes to false in fly.

But I think it should only be something that you set on AppBuilder and only once

The behavior if one media element is using the service and the other is not is that one will have access to the service and the other will not. This PR was meant to allow input and let ppl see how it could be done. It is marked as draft and I explicitly created it so I could gather feedback and make changes. I was hoping for this exact sort of input.

Do we want to have it as init only and apply to any media elements that launch? Or do we want to allow each media element the option at runtime to decide?

If one media element is already using the service and the second one is not using it it will not tear down the service. I do not believe that is possible without tearing down the media element using the service. The service is tightly integrated with media session and is as close to default implementation as I could create based on platform limitations. The only thing I could do to make it more like the google example is to move the Exoplayer Builder into the service itself. That was the original goal but do to various factors in the original media2 to media3 PR it never happened.

I would prefer to keep this as simple as possible for maintainability while allowing developers the choice to do what they want. My goal here is to add the option to opt out of the service at runtime. Do you want it set in builder only? Or does anyone have any better suggestions? I am not looking at adding anything complex or doing something new. I simply want to add developer choice.

Anyone who has an opinion please speak up. I still think of this as being in design phase. This PR was purely to allow ppl to see the idea I had for design.

@Elinares-82
Copy link

Do we want to have it as init only and apply to any media elements that launch? Or do we want to allow each media element the option at runtime to decide?

What if we can have both?, We can have an initial flag to disable it for all the media elements, and force with the new IsAndroidForegroundServiceEnabled to start individually? I do not know if this make sense.

I the other hand, I know that each team have processes, I don't like to sound rude at all. I'd just like to know when this version might be coming out.

Thanks,

@mrucker
Copy link

mrucker commented May 7, 2025

I was reading back through old discussions because I've been confused. I think there are two different issues being discussed. I'm going to call them issue 1 and issue 2 here.

Issue 1 -- MediaElement should only turn on a foreground service if it is requested
Issue 2 -- MediaElement should only add foreground permissions to the Android Manifest if requested

I think this PR will likely fix what I call issue 1, but it won't fix what I call issue 2.

@ne0rrmatrix Is this PR intended to fix both issue 1 and 2? Or are you only trying to fix issue 1?

@Elinares-82 I think you have an issue 2 problem (which is what I had when I joined these threads several months ago). Unless ne0rrmatrix says otherwise, I don't think this PR is going to fix your issue.

@ne0rrmatrix
Copy link
Contributor Author

@mrucker ty for the clarification. I will update the PR to address both issues. I appreciate all the help and input.

@mrucker
Copy link

mrucker commented May 7, 2025

(Also, I seem to be the one to blame for the confusion. Looking back through the discussion history, I hijacked discussion #2225, which was initially about issue 1, and started talking about issue 2 because I didn't understand the difference at the time. Sorry everyone.)

@Elinares-82
Copy link

Elinares-82 commented May 7, 2025

MediaElementOptions

You are totally right; I think this will not fix my issue. Google rejected our app since in the Manifest are several permissions like FOREGROUND_SERVICE_MEDIA_PLAYBACK and FOREGROUND_SERVICE, but we just display a promo video in the splash screen. We are stuck at this point. The only way that I can see is use an old version or use an alternative like Blazor component.

@ne0rrmatrix
Copy link
Contributor Author

Updating the service to address both issues will require the developer to add permissions manually to manifest and will only allow the service to run if permissions and foreground service are turned on. My questions is how do we want to deal with that. The reason we set the permissions ourselves was to fix the issue with constant bug reports where the solution was to simply add the service and intent filter to the manifest.

@mrucker
Copy link

mrucker commented May 7, 2025

Updating the service to address both issues will require the developer to add permissions manually to manifest and will only allow the service to run if permissions and foreground service are turned on. My questions is how do we want to deal with that. The reason we set the permissions ourselves was to fix the issue with constant bug reports where the solution was to simply add the service and intent filter to the manifest.

Yes... That's kind of the problem we never solved in an old discussion about it... Which is why I've never offered my own PR... There didn't seem to be a great solution.

@Elinares-82
Copy link

Updating the service to address both issues will require the developer to add permissions manually to manifest and will only allow the service to run if permissions and foreground service are turned on. My questions is how do we want to deal with that. The reason we set the permissions ourselves was to fix the issue with constant bug reports where the solution was to simply add the service and intent filter to the manifest.

I think, if this is documented all of us have to read and follow the guidance. For me is normal to add the permissions that the controls need. But, is there a possibility to disabled to add the permissions to the Manifest if the new Flag is turn it on? In a complex scenario use a project property to disable this in the source generator (I'm assuming that these permissions are being added using a source generator)

@mrucker
Copy link

mrucker commented May 7, 2025

@Elinares-82

You are totally right; I think this will not fix my issue. Google rejected our app since in the Manifest are several permissions like FOREGROUND_SERVICE_MEDIA_PLAYBACK and FOREGROUND_SERVICE, but we just display a promo video in the splash screen. We are stuck at this point. The only way that I can see is use an old version or use an alternative like Blazor component.

It is easy to remove the permission requirements if you are able to build the MediaElement yourself and create your own Nuget Package. It's not ideal, but if you're really up against a wall it will allow you to release your app. I'd send you my team's custom Nuget package we've been using to remove this permission, but handing out code to strangers on the internet doesn't seem like a best practice 😄.

@pictos
Copy link
Member

pictos commented May 7, 2025

MediaElementOptions

You are totally right; I think this will not fix my issue. Google rejected our app since in the Manifest are several permissions like FOREGROUND_SERVICE_MEDIA_PLAYBACK and FOREGROUND_SERVICE, but we just display a promo video in the splash screen. We are stuck at this point. The only way that I can see is use an old version or use an alternative like Blazor component.

@Elinares-82 there are tools that you allow you to edit the manifest on android app, so when you build your project and it generates the apk or aab you can unzip it, remove the permissions that you don't want and zip it again to ship into stores. I already did that for many apps that I worked and never had issues with that, and since are tools for it, I would say, isn't something unusual

@Elinares-82
Copy link

@Elinares-82

You are totally right; I think this will not fix my issue. Google rejected our app since in the Manifest are several permissions like FOREGROUND_SERVICE_MEDIA_PLAYBACK and FOREGROUND_SERVICE, but we just display a promo video in the splash screen. We are stuck at this point. The only way that I can see is use an old version or use an alternative like Blazor component.

It is easy to remove the permission requirements if you are able to build the MediaElement yourself and create your own Nuget Package. It's not ideal, but if you're really up against it will allow you to release your app. I'd send you my team's custom Nuget package we've been using to remove this permission, but handing out code to strangers on the internet doesn't seem like a best practice 😄.

I think these are the lines that I have to comment out. I'm trying to use the generated "custom" DLL but I'm getting several compiler errors so I will try the @pictos suggestion.

image

Thanks to all for your help and recommendations.

@ne0rrmatrix
Copy link
Contributor Author

Give me an hour or two and I will update this PR to address issues brought up. I have an idea of how we can do this and have everyone get what they want.

@mrucker
Copy link

mrucker commented May 7, 2025

@Elinares-82 Those two and I think you'd also need to get these two (it's been a while since I made the change)

image

- Added a new service for media playback in `AndroidManifest.xml` with intent filters for media button actions and media session service.
- Changed default value of `AndroidForegroundServiceEnabled` to `false` in `MediaElementOptions.shared.cs`.
- Removed unnecessary permission attributes from `MauiMediaElement.android.cs`.
- Updated test cases in `AppBuilderExtensionsTests.cs` to reflect the new default value for `AndroidForegroundServiceEnabled`.
@Elinares-82
Copy link

@Elinares-82 this issue with scroll view and keyboard is reported on Maui repo? If don't, do you mind to it?

@pictos sure, I'll check is this issue was already reported, in case not, I'll do it!

@Elinares-82
Copy link

Finally, our apps were approved by Google. Thanks. Now I'm reviewing the issue regarding the Scroll and Microsoft.Maui.Controls 9.0.50 and 60. and log it into the repo.

@Elinares-82
Copy link

Finally, our apps were approved by Google. Thanks. Now I'm reviewing the issue regarding the Scroll and Microsoft.Maui.Controls 9.0.50 and 60. and log it into the repo.

The issue is already logged

dotnet/maui#19014

2023 almost 2 years ago.

@ne0rrmatrix
Copy link
Contributor Author

ne0rrmatrix commented Jun 4, 2025

I am thinking we should switch to using just a builder option with no ability to toggle the service off and on. ATM switching back and forth does work but the images are not loading after turning off service and then turning it back on. Can see this bug live by loading popup in media element page. In this PR I have the service disabled when you load the popup to test this out. I will look at why the image is not loading. This may be caused by having two media elements loaded where one has it turned off and the other has it on. I will investigate the behavior.

Does anyone need or want the ability to switch at runtime? Or is a builder option ok? This PR currently has both. We could end up with issues when using multiple media elements where one is turned on and one is off? How do we want to handle that? It would be much simpler to have just the builder and no runtime switch. This would simplify implementation and make it easier to maintain. My bad for overcomplicating things.

@ne0rrmatrix ne0rrmatrix added the needs discussion Discuss it on the next Monthly standup label Jun 4, 2025
@bijington
Copy link
Contributor

I say just go with the builder option and keep things simple

@JoacimWall
Copy link

JoacimWall commented Jun 23, 2025

Hi
is this fixed now and then in what version? and are they instruction on how to disable foreground service?
Regards Joacim

@bijington
Copy link
Contributor

@JoacimWall this hasn't merged yet so is not yet available.

@ne0rrmatrix where are we with this? I think we have a solution but it is breaking? Do you think there is anything we could mark as Obsolete in an upcoming release that would indicate to developers they needed to opt-in to the foreground service in future releases? I am suspecting it wouldn't be easy given the developer didn't have to call any foreground specific code?

@ne0rrmatrix
Copy link
Contributor Author

The main changes are having to add service registration and permissions to Android manifest. We had removed that and added it directly. With this PR developers will be required to do so again. Also they need to opt out of service. i don't see how to use obsolete as intended with how this works.

Updated the `CreateMauiApp` method to enable the Android foreground service for `MediaElement`. Removed the `IsAndroidForegroundServiceEnabled` property from the XAML definition and set it to `false` in the `DisplayPopup` method. Changed the access level of the `IsAndroidForegroundServiceEnabled` property in `MediaElement.shared.cs` from `public` to `internal` to restrict its visibility.
@ne0rrmatrix ne0rrmatrix marked this pull request as ready for review July 3, 2025 18:24
@ne0rrmatrix ne0rrmatrix removed do not merge Do not merge this PR needs discussion Discuss it on the next Monthly standup labels Jul 3, 2025
@Copilot Copilot AI review requested due to automatic review settings July 30, 2025 22:53
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for making Android foreground service optional in MediaElement, providing developers with better control over media playback behavior on Android. The change introduces a new configuration option that allows disabling the foreground service when it's not needed, which can be useful for applications that don't require background media playback.

  • Adds IsAndroidForegroundServiceEnabled property to control foreground service usage
  • Updates MediaManager to conditionally start service based on the new property
  • Provides builder configuration method to set the default behavior

Reviewed Changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
AppBuilderExtensionsTests.cs Adds unit tests for the new Android foreground service configuration
MediaManager.android.cs Updates service creation logic to respect the foreground service enabled flag
MauiMediaElement.android.cs Removes hardcoded Android permissions that are now conditionally needed
MediaElementOptions.shared.cs Adds configuration option for Android foreground service
MediaElement.shared.cs Adds internal property to track foreground service enablement
MediaElementHandler.android.cs Passes the foreground service flag to platform view creation
AndroidManifest.xml Moves Android permissions to sample manifest
MauiProgram.cs Demonstrates usage of the new configuration option
Comments suppressed due to low confidence (1)

src/CommunityToolkit.Maui.MediaElement/Views/MediaManager.android.cs:24

  • The field name 'androidForegroundServiceEnabled' should be renamed to follow proper naming conventions. Consider using 'isAndroidForegroundServiceEnabled' or 'AndroidForegroundServiceEnabled' to be consistent with the property naming pattern used elsewhere in the codebase.
	bool androidForegroundServiceEnabled;

@Elinares-82
Copy link

Hi all, Does this improvement have a ETA? I built my own version of this component as @mrucker suggested, however, the idea is to have the official nuget.

Regards,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📽️ MediaElement Issue/PR that has to do with MediaElement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Proposal] Add MediaElement Foreground Service to Media Options
7 participants