-
Notifications
You must be signed in to change notification settings - Fork 468
Android service update #2897
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
base: main
Are you sure you want to change the base?
Android service update #2897
Conversation
Refactored `MediaElement` implementation for Android to replace the bound service (`MediaControlsService`) with `MediaSession` and `MediaController`, simplifying the architecture and aligning with modern Android media playback practices. - Removed `BoundServiceBinder` and `BoundServiceConnection` classes. - Updated `MediaControlsService` to use `ExoPlayer` and `MediaSession`. - Simplified notification management with `NotificationCompat`. - Refactored `MediaManager` to remove session and bound service logic. - Added `CreateMediaController` for asynchronous `MediaController` setup. - Updated `MauiMediaElement` to support `MediaController` integration. - Removed legacy code, unused imports, and redundant methods.
…gating a second time to same item and now the image does display in notifications. It did not previously. This affects load times. It is now faster.
src/CommunityToolkit.Maui.MediaElement/Services/MediaControlsService.android.cs
Outdated
Show resolved
Hide resolved
- Updated `textureview.xml` to use black backgrounds for better consistency. - Replaced `RelativeLayout` with `FrameLayout` in `MauiMediaElement.android.cs` for improved layout management. - Refactored `MediaControlsService.android.cs`: - Replaced `static readonly` fields with `const` for immutability. - Added audio attributes for better playback handling. - Updated track selector to fallback to system language. - Added handling for audio interruptions (e.g., unplugging headphones). - Added `OnTracksChanged` in `MediaManager.android.cs` to manage subtitle button visibility. - Removed unused imports and redundant constants for cleaner code. - Improved buffering strategies for smoother playback.
|
The issue with transparent or white background has been fixed! So this is now ready for review :) |
There was a problem hiding this 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 modernizes the Android MediaElement implementation by replacing the deprecated service architecture with Google's recommended MediaSessionService approach. The changes migrate from using ExoPlayer directly to using MediaController with MediaSessionService, implementing proper lifecycle management and unified media notifications.
Key changes include:
- Replaced ExoPlayer with MediaController as the platform media element
- Implemented MediaSessionService pattern for background media playback
- Unified media notifications using native Android methods instead of custom implementations
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| MediaManager.shared.cs | Updated global using alias from IExoPlayer to MediaController |
| MediaManager.android.cs | Complete overhaul replacing ExoPlayer with MediaController, removed service binding logic |
| MauiMediaElement.android.cs | Refactored layout from RelativeLayout to FrameLayout, added MediaController integration |
| MediaControlsService.android.cs | Converted from Service to MediaSessionService with proper notification handling |
| BoundServiceConnection.android.cs | File deleted - service binding no longer needed |
| BoundServiceBinder.android.cs | File deleted - service binding no longer needed |
| textureview.xml | Changed background colors from white/transparent to black |
| MediaElementHandler.android.cs | Added async MediaController creation and connection logic |
| AppBuilderExtensions.shared.cs | Removed MediaControlsService dependency injection |
src/CommunityToolkit.Maui.MediaElement/Views/MediaManager.android.cs
Outdated
Show resolved
Hide resolved
src/CommunityToolkit.Maui.MediaElement/Views/MediaManager.android.cs
Outdated
Show resolved
Hide resolved
src/CommunityToolkit.Maui.MediaElement/Views/MediaManager.android.cs
Outdated
Show resolved
Hide resolved
…oid.cs Good catch Co-authored-by: Copilot <[email protected]>
…oid.cs Co-authored-by: Copilot <[email protected]>
Updated the CreateMediaController method to include an optional CancellationToken parameter, allowing the operation to be canceled if needed. Modified the WaitAsync call to respect the provided cancellation token, improving the method's flexibility and robustness.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
src/CommunityToolkit.Maui.MediaElement/Views/MediaManager.android.cs
Outdated
Show resolved
Hide resolved
src/CommunityToolkit.Maui.MediaElement/Handlers/MediaElementHandler.android.cs
Show resolved
Hide resolved
src/CommunityToolkit.Maui.MediaElement/Views/MediaManager.android.cs
Outdated
Show resolved
Hide resolved
src/CommunityToolkit.Maui.MediaElement/Views/MediaManager.android.cs
Outdated
Show resolved
Hide resolved
Removed unused `CancellationTokenSource` and `HttpClient` fields from the `MediaManager` class to simplify the code. Updated the `Dispose` method to use a `using` statement for `serviceIntent` to ensure proper disposal. These changes improve resource management and align with modern C# practices.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/CommunityToolkit.Maui.MediaElement/Views/MediaManager.android.cs:262
- [nitpick] There are two methods handling playback state changes with overlapping logic:
OnPlayerStateChanged(lines 71-104) andOnPlaybackStateChanged(lines 226-262). Both methods handle state transitions for the same states (stateBuffering,stateEnded,stateReady,stateIdle), which could lead to duplicate state updates and potential race conditions.
Review whether both methods are needed. If both are callbacks from the IPlayerListener interface, ensure they don't conflict or cause duplicate state updates. Consider consolidating the logic or clearly documenting why both are necessary.
public void OnPlayerStateChanged(bool playWhenReady, int playbackState)
{
if (Player is null || MediaElement.Source is null)
{
return;
}
var newState = playbackState switch
{
stateBuffering => MediaElementState.Buffering,
stateEnded => MediaElementState.Stopped,
stateReady => playWhenReady
? MediaElementState.Playing
: MediaElementState.Paused,
stateIdle => MediaElementState.None,
_ => MediaElementState.None,
};
MediaElement.CurrentStateChanged(newState);
if (playbackState is stateReady)
{
MediaElement.Duration = TimeSpan.FromMilliseconds(
Player.Duration < 0 ? 0 : Player.Duration
);
MediaElement.Position = TimeSpan.FromMilliseconds(
Player.CurrentPosition < 0 ? 0 : Player.CurrentPosition
);
}
else if (playbackState is stateEnded)
{
MediaElement.MediaEnded();
}
}
public void OnIsPlayingChanged(bool isPlaying)
{
if (Player is null || MediaElement.Source is null)
{
return;
}
var newState = isPlaying
? MediaElementState.Playing
: MediaElementState.Paused;
MediaElement.CurrentStateChanged(newState);
}
public void OnTracksChanged(Tracks? tracks)
{
if (tracks is null || tracks.IsEmpty)
{
return;
}
if (tracks.IsTypeSupported(C.TrackTypeText))
{
PlayerView?.SetShowSubtitleButton(true);
}
else
{
PlayerView?.SetShowSubtitleButton(false);
}
}
/// <summary>
/// Creates the corresponding platform view of <see cref="MediaElement"/> on Android.
/// </summary>
/// <returns>The platform native counterpart of <see cref="MediaElement"/>.</returns>
/// <exception cref="NullReferenceException">Thrown when <see cref="Context"/> is <see langword="null"/> or when the platform view could not be created.</exception>
[MemberNotNull(nameof(PlayerView))]
public PlayerView CreatePlatformView(AndroidViewType androidViewType)
{
if (androidViewType is AndroidViewType.SurfaceView)
{
PlayerView = new PlayerView(MauiContext.Context)
{
UseController = false,
ControllerAutoShow = false,
LayoutParameters = new RelativeLayout.LayoutParams(ViewGroup.LayoutParams.MatchParent, ViewGroup.LayoutParams.MatchParent)
};
}
else if (androidViewType is AndroidViewType.TextureView)
{
if (MauiContext.Context?.Resources is null)
{
throw new InvalidOperationException("Unable to retrieve Android Resources");
}
var resources = MauiContext.Context.Resources;
var xmlResource = resources.GetXml(Microsoft.Maui.Resource.Layout.textureview);
xmlResource.Read();
var attributes = Android.Util.Xml.AsAttributeSet(xmlResource)!;
PlayerView = new PlayerView(MauiContext.Context, attributes)
{
UseController = false,
ControllerAutoShow = false,
LayoutParameters = new RelativeLayout.LayoutParams(ViewGroup.LayoutParams.MatchParent, ViewGroup.LayoutParams.MatchParent)
};
}
else
{
throw new NotSupportedException($"{androidViewType} is not yet supported");
}
return PlayerView;
}
public async Task<AndroidX.Media3.Session.MediaController> CreateMediaController(CancellationToken cancellationToken = default)
{
var tcs = new TaskCompletionSource();
var future = new MediaController.Builder(Platform.AppContext, new SessionToken(Platform.AppContext, new ComponentName(Platform.AppContext, Java.Lang.Class.FromType(typeof(MediaControlsService))))).BuildAsync();
future?.AddListener(new Runnable(() =>
{
try
{
var result = future.Get() ?? throw new InvalidOperationException("MediaController.Builder.BuildAsync().Get() returned null");
if (result is MediaController mc)
{
Player = mc;
Player.AddListener(this);
if (PlayerView is null)
{
throw new InvalidOperationException($"{nameof(PlayerView)} cannot be null");
}
PlayerView.SetBackgroundColor(Android.Graphics.Color.Black);
PlayerView.Player = Player;
using var intent = new Intent(Android.App.Application.Context, typeof(MediaControlsService));
Android.App.Application.Context.StartForegroundService(intent);
tcs.SetResult();
}
else
{
tcs.SetException(new InvalidOperationException("MediaController.Builder.BuildAsync().Get() did not return a MediaController"));
}
}
catch (Exception ex)
{
Trace.WriteLine($"Error creating MediaController: {ex}");
tcs.SetException(ex);
}
}), ContextCompat.GetMainExecutor(Platform.AppContext));
await tcs.Task.WaitAsync(cancellationToken);
return Player ?? throw new InvalidOperationException("MediaController is null");
}
/// <summary>
/// Occurs when ExoPlayer changes the playback state.
/// </summary>
/// <paramref name="playbackState">The state that the player has transitioned to.</paramref>
/// <remarks>
/// This is part of the <see cref="IPlayerListener"/> implementation.
/// While this method does not seem to have any references, it's invoked at runtime.
/// </remarks>
public void OnPlaybackStateChanged(int playbackState)
{
if (MediaElement.Source is null)
{
return;
}
MediaElementState newState = MediaElement.CurrentState;
switch (playbackState)
{
case stateBuffering:
newState = MediaElementState.Buffering;
break;
case stateEnded:
newState = MediaElementState.Stopped;
MediaElement.MediaEnded();
break;
case stateReady:
seekToTaskCompletionSource?.TrySetResult();
// Update duration and position when ready
if (Player is not null)
{
MediaElement.Duration = TimeSpan.FromMilliseconds(
Player.Duration < 0 ? 0 : Player.Duration
);
MediaElement.Position = TimeSpan.FromMilliseconds(
Player.CurrentPosition < 0 ? 0 : Player.CurrentPosition
);
}
break;
case stateIdle:
newState = MediaElementState.None;
break;
}
MediaElement.CurrentStateChanged(newState);
}
src/CommunityToolkit.Maui.MediaElement/Services/MediaControlsService.android.cs
Show resolved
Hide resolved
|
I can confirm this PR fixes some really buggy behaviour in the Android playback state changes. I've been testing it intensively over the last couple of days and I haven't come across any negative side effects. |
…oid.cs Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
src/CommunityToolkit.Maui.MediaElement/Services/MediaControlsService.android.cs
Show resolved
Hide resolved
src/CommunityToolkit.Maui.MediaElement/Services/MediaControlsService.android.cs
Show resolved
Hide resolved
src/CommunityToolkit.Maui.MediaElement/Services/MediaControlsService.android.cs
Show resolved
Hide resolved
src/CommunityToolkit.Maui.MediaElement/Services/MediaControlsService.android.cs
Show resolved
Hide resolved
src/CommunityToolkit.Maui.MediaElement/Services/MediaControlsService.android.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey James! It looks like this PR is causing a breaking change when multiple MediaElements are on screen at the same time.
Here's the problems I found using the MediaElement in CarouselView Page:
- Wrong video plays, e.g. BuckBunny is shown instead of Elephants Dream
- Pausing/Playing one video pauses/plays all other videos
That is an issue that will be difficult to resolve. The MediaSessionService is not intended to be run as anything but a singleton and explicitly states in documentation that it works that way by design. I have looked at stack overflow, github discussions, and at github bug reports. They all have one thing in common. All the response from google employees agree that multiple players playing different streams is not something they will comment on or provide support for. They recommend strongly that you do not do that. Having multiple video players takes up limited resources and I have noticed that all major streaming platforms tend to use essentially the same flow for how video's are shown. The actual player is almost always on its own page. Twitch, youtube, netflix, prime, etc all use a separate page to playback content. I do not believe we should support multiple instances of the player going further. I am happy to try and implement this but it looks like it will take a long time for me to figure this out. It has taken me almost 2 years go get this far. It is unlikely I can do this in a reasonable period of time. This will require me to create a solution that is not supported by the library and was never intended. I will keep trying this weekend but if I do not have any luck I will most likely wait for feedback from the community with suggestions on how me might do this. The current recommendation from https://developer.android.com/media/media3/session/control-playback is to not use a Do we want to consider leaving it as is if I can't figure it out? Try to figure out some workarounds for bugs and issues that crop up from unsupported use of media3? Or do we want to consider removing |


Description of Change
Switch from using default service implementation to using the recommended by Google. Player and
MediaSessioninMediaSessionServicewithMediaControllerin Activity. The deprecated API's for notifications have been completely removed and are no longer needed.No change in features with respect to media notifications. The rich media notifications have been significantly overhauled and following best principles. The issue with notification getting stuck and not being removed when leaving page are fixed. When you stop the player the notifications will be removed automatically. If you navigate away from page the notifications will also be removed. It you force close the app by swiping up it will close the notifications and end playback.
The new changes include using a
MediaControlleras an interface to theMediaSessionServicewhich lives in the Activity and fully replaces the player functionality as far asMediaManageris concerned.Deprecated API's like
PlayerNotificationsand custom http handlers to make web requests to get artwork have been removed and replaced with native methods that do the same thing. The Media Notifications are now unified under a single class and none of the API's are deprecated. This has backwards support and is tested from API 26 to 35.Breaking Changes:
The downside to this is possible a deal breaker and is a breaking change. Currently MediaElement supports as many concurrent players as you the developers chooses to use. With this change it will not longer support this. It will only support a single instance per app. With how the service model and integration between the player, player view and mediaSession works it is not really possible to have more than one Rich Media Session at the same time. Imagine your notification tray having a new item for every video in your collection?
This is a big issue. It is an ongoing issue for Windows, MacOS and iOS. All 3 of those devices have current and ongoing issues with CollectionViews, Grids, and Carousel Views. I do not believe it should ever have been intended to have this functionality and we should work towards creating sample pages where developers can effectively use those views and have a media element. But we need to keep to a single, reusable instance. Having multiple simultaneous players should not be supported and is not needed if the pages are designed correctly.
PR Checklist
approved(bug) orChampioned(feature/proposal)mainat time of PRAdditional information