Skip to content
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

Adding a MediaPlayerElement to Maui #352

Closed
wants to merge 8 commits into from

Conversation

jonx
Copy link

@jonx jonx commented Jun 18, 2024

Description of Change

This draft PR includes the implementation of a MediaPlayerElement for Maui. The changes include:

  • Added new MediaPlayerElement control to LibVCLSharp.MAUI
  • Added the sample LibVLCSharp.MAUI.Sample.MediaElement based on the Forms sample
  • Added the sample LibVLCSharp.MAUI.Sample.MediaElement.Multiple that aims at showing how to style and use two instances of the control on the same page - but it's not yet doing that. Both controls are the same style currently

Issues Resolved

  • fixes #637

Other Changes

  • The SvgTintColorBehavior.cs was meant to change the color or the SVG icons but it's not working and can be ignored

Platforms Affected

  • Core (all platforms)
  • Android

Behavioral/Visual Changes

None

Before/After Screenshots

Not applicable

Testing Procedure

Not applicable

PR Checklist

  • Rebased on top of the target branch at time of PR
  • Changes adhere to coding standard

jonx added 7 commits June 4, 2024 21:39
App life cycle management rewrite.
App state save.
Corrected the Strings.fr.resx build problem.
Forward icon was not visible because the svg icon had the wrong name.
Icons are white, until we find a better way to handle colors.
New Sample with multiple MediaElementPlayers in the same project.
@jonx
Copy link
Author

jonx commented Jun 18, 2024

The implementation is close to complete for Android but probably needs further testing.

  1. I removed the dependance to AwesomeFont and replaced it with equivalent SVG files. The problem is that I can't change their color? I tried to implement a behavior to do that - it's in the project - but it's not working
  2. I replaced Messaging center by WeakEventManager
  3. The checks in the track list popup should be more constrained. The controls there need some refinement.
  4. The lock slider has some initial drawing issues. The icon only appears once you scroll once for some reason.
  5. This is only an Android port, but I suspect getting the iOS version should be quite easy given all we have here is cross-platform.
  6. The video starts playing before the player is visible. Maybe it's related to how the player is embedded.

It's possible the issues I mention were there already in the Xamarin version.

Please review and provide feedback.

For some reason, you need to specify the complete path to the type on Windows.
@mfkl
Copy link
Member

mfkl commented Jun 19, 2024

image

Looking good so far :-)

@mfkl
Copy link
Member

mfkl commented Jun 21, 2024

I'm wondering whether it would be possible to keep the same code for XF and MAUI and just use #ifdef? Depends how much code is the same or different, besides namespaces.
Also I don't know when we will drop XF support, so going through the effort of supporting both from a single code base might not be worth it. Not sure.

Will attempt to run the iOS sample next week, keep you posted.

@jonx
Copy link
Author

jonx commented Jun 25, 2024

I guess it's possible in theory but I'm not sure how you're going to manage this. The way I see this is that it's probably less work to maintain both projects rather than having to add all the #ifdefs. Maybe there is an easy way to do such thing that I don't know of?

@mfkl mfkl changed the title Issue/637/media element for Maui 3.x - Adding a MediaPlayerElement to Maui Adding a MediaPlayerElement to Maui Jun 25, 2024
@mfkl
Copy link
Member

mfkl commented Jun 25, 2024

After some setup and deployment struggles, I managed to run the sample on an iOS device.

image

Buttons are present and working, just invisible.

@mfkl
Copy link
Member

mfkl commented Jul 3, 2024

Managed to display the play/pause icons in iOS with a hack. mfkl@66d3d52;

Still need to figure out how to include them transitively (from libvlcsharp.maui) as they were not present in the app bundle.

MAUI expects png references from XAML or C# as it translates the svgs.

The path from the generic theme appears to be from the app bundle, not the code project.

Comment on lines +19 to +33
<PropertyGroup Condition="'$(Configuration)|$(TargetFramework)|$(Platform)'=='Debug|net7.0-android|AnyCPU'">
<TreatWarningsAsErrors>False</TreatWarningsAsErrors>
</PropertyGroup>

<PropertyGroup Condition="'$(Configuration)|$(TargetFramework)|$(Platform)'=='Debug|net6.0-ios|AnyCPU'">
<TreatWarningsAsErrors>False</TreatWarningsAsErrors>
</PropertyGroup>

<ItemGroup>
<PropertyGroup Condition="'$(Configuration)|$(TargetFramework)|$(Platform)'=='Release|net7.0-android|AnyCPU'">
<TreatWarningsAsErrors>False</TreatWarningsAsErrors>
</PropertyGroup>

<PropertyGroup Condition="'$(Configuration)|$(TargetFramework)|$(Platform)'=='Release|net6.0-ios|AnyCPU'">
<TreatWarningsAsErrors>False</TreatWarningsAsErrors>
</PropertyGroup>
Copy link
Member

Choose a reason for hiding this comment

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

I think these can be removed.

Comment on lines +39 to +46
<ItemGroup>
<AndroidResource Remove="Handlers\**" />
<Compile Remove="Handlers\**" />
<EmbeddedResource Remove="Handlers\**" />
<MauiCss Remove="Handlers\**" />
<MauiXaml Remove="Handlers\**" />
<None Remove="Handlers\**" />
</ItemGroup>
Copy link
Member

Choose a reason for hiding this comment

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

these lines can be removed too I guess

@mfkl
Copy link
Member

mfkl commented Jul 5, 2024

I'm thinking we can remove the Shared folder and namespace.

@mfkl
Copy link
Member

mfkl commented Jul 5, 2024

For the image issue on iOS, Maui is supposed to translate them into pngs during build with proper size options but there are various bugs preventing it from doing it transitively. With a BundleResource I managed to have them in the app bundle but they're not translated into pngs and iOS cannot read SVG natively...

So we might be better off sticking to FontAwesome for now, as XF was doing.

@jonx
Copy link
Author

jonx commented Jul 15, 2024

Alright, thanks for the review. I'll update the PR based on your comments and will revert back the Fontawesome removal with the hope we won't face the same issue with icons.

@TheSundayDev
Copy link

hi,
can I ask if you have an estimation in order to be live? thans a lot!

@jonx
Copy link
Author

jonx commented Jul 26, 2024

Hello, I'll update the PR with the points discussed here by the end of next week. What platform are you interested in?

@TheSundayDev
Copy link

Hi,
MAUI ios and android for now.
thanks

@jonx
Copy link
Author

jonx commented Jul 26, 2024

Alright, in this case, if you're in a hurry, my PR code is working already and not much will change before the final version on Android. We're only having this bug on iOS that is not including the button icons from the library to the main project. I'm just going to revert to using fonts like the original xamarin was doing. Meaning you can potentially start integrating it in your project. If not, stay tuned for my update next week. Then it will probably take a little longer for the VLC# people to accept the PR.

@mfkl
Copy link
Member

mfkl commented Jul 30, 2024

hi, can I ask if you have an estimation in order to be live? thans a lot!

Hi, we do not provide delivery estimate for opensource work, it is released when it's ready. If you'd like to speed up the process, you may check out the PR's branch locally and help improve it by fixing the remaining issues. Thanks.

@TheSundayDev
Copy link

hi, can I ask if you have an estimation in order to be live? thans a lot!

Hi, we do not provide delivery estimate for opensource work, it is released when it's ready. If you'd like to speed up the process, you may check out the PR's branch locally and help improve it by fixing the remaining issues. Thanks.

Hi,
I was just asking, I didn't want to push ;)

@jonx
Copy link
Author

jonx commented Sep 28, 2024

I'm closing this PR to sync my fork and create a new PR.
@TheSundayDev, you're welcome to test the new PR. Feedback welcome.

@jonx jonx closed this Sep 28, 2024
@jonx jonx deleted the issue/637/MediaElement-for-MAUI-3.x branch September 30, 2024 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants