Maui

Maui copied to clipboard

Is there an existing issue for this?

• [X] I have searched the existing issues

Did you read the "Reporting a bug" section on Contributing file?

• [X] I have read the "Reporting a bug" section on Contributing file: https://github.com/CommunityToolkit/Maui/blob/main/CONTRIBUTING.md#reporting-a-bug

Current Behavior

The values of MediaWidth and MediaHeight are always 0, or at least start as 0 if there is some non-obvious way to get them to get set to the dimensions of the media. The MediaOpened and SizeChanging events get triggered, but the properties are always 0 in those as well.

Similarly, the MediaElement itself is not visible (Height=.36...) unless the HeightRequest is explicitly set or maybe some other non-obvious conditions have to be met.

Expected Behavior

The MediaWidth and MediaHeight properties should reflect the dimensions of the media the source points to.

Also, in my opinion, if there is no HeightRequest set then the MediaElement should size itself to the same aspect ratio as the media based on however it gets its width, at least assuming the Aspect property is AspectFit or AspectFill. At the very least it should probably just make itself square. And that could work even if the Aspect property was set to Fill.

In other words, I think that by default the MediaElement should display the media set in the Source property. But even if that isn't possible, it would help for the two properties above to reflect the dimensions so that I can calculate it myself (this is what I had to do in Xamarin).

Steps To Reproduce

1. Open and solution from reproduction repository.
2. Run the MediaElement project.
3. Enter the URL for a video file in the Entry below the race car. The video used in a lot of MediaElement examples can be used: https://commondatastorage.googleapis.com/gtv-videos-bucket/sample/BigBuckBunny.mp4
4. The State should display Paused, since the video isn't playing (and you can't play it because it isn't visible).
5. MediaWidth and MediaHeight both display 0.

Link to public reproduction project repository

https://github.com/bzd3y/ReproductionProject

Environment

- .NET MAUI CommunityToolkit: 3.0.1
- OS: Android Api 34. I haven't tried on iOS.
- .NET MAUI: 8.0.3

Anything else?

No response

Not sure how we want to handle this. There are many controls where default values are not set. Getting media info varies between devices and would require someone to write an API and implementation. Setting height and width from media height and width would require a bit of work. This might be best to move to a discussion as feature request? @vhugogarcia

Thanks @ne0rrmatrix for the feedback.

By running a quick search of the properties MediaWidth and MediaHeight in this repository, I noticed that there are 2 bindable properties there for those, however, they are never used at all. If they are not needed at all, we maybe can remove them. https://github.com/search?q=repo%3ACommunityToolkit%2FMaui%20MediaWidth&type=code

@jfversluis what do you think about those 2 properties, maybe they were migrated in the past, however, on this new version they are not required anymore because we are using the native HeightRequest and WidthRequest properties to set the size of the video.

cc: @brminnick

@vhugogarcia Those properties appear to duplicate existing properties that do the same thing. MediaElement.HeightRequest is a VisualElement that is a BindableProperty that sets the height. Same goes for width. Without a compelling reason I think we should indeed remove them.

I think we're confusing two things here. Yes we have HeightRequest and WidthRequest but isn't that for the control? The MediaWidth and MediaHeight should tell me about the dimensions of the media (when applicable, obviously not for audio only). So my MediaElement control would be 100x100 but my media can be 1080x1920. I still think that is useful information?

Looks like we (probably me...) forgot to just hook that up? Because it seems like in XCT we did actually retrieve it: https://github.com/xamarin/XamarinCommunityToolkit/blob/main/src/CommunityToolkit/Xamarin.CommunityToolkit/Views/MediaElement/Android/FormsVideoView.android.cs#L37

I think we're confusing two things here. Yes we have HeightRequest and WidthRequest but isn't that for the control? The MediaWidth and MediaHeight should tell me about the dimensions of the media (when applicable, obviously not for audio only). So my MediaElement control would be 100x100 but my media can be 1080x1920. I still think that is useful information?

Looks like we (probably me...) forgot to just hook that up? Because it seems like in XCT we did actually retrieve it: https://github.com/xamarin/XamarinCommunityToolkit/blob/main/src/CommunityToolkit/Xamarin.CommunityToolkit/Views/MediaElement/Android/FormsVideoView.android.cs#L37

Yes it is not hooked up at all. I was looking at it after @vhugogarcia mentioned it. There was a request for this info in another discussion/bug report/PR.

Good catch @jfversluis! thanks for sharing more insights.

The metadata of the video can be extracted in C# directly or by using a package like: https://github.com/CryShana/CryMedia So, that metadata can be useful for the developer if needed and even a lot more metadata. There are several ways to get the metadata crossplatform from a given video these days.

If we extract the metadata from directly from each platform, we will be ending up adding/supporting a lot of metadata information just to cover the requests. So, I personally believe we should remove those properties to avoid confusion, and focus only on the Media Element core functionality.

However, I am happy to what you all define as next steps 😃

What do you think @jfversluis?

I don't think we need to overdo it, I do would like to match what was available in Xamarin at the very least.

Looking at the linked Xamarin Community Toolkit code we should be able to do it without any extra dependencies and it doesn't seem very complicated...?

Tried to put some code together which should make this work in a draft PR, will look at finalizing this tomorrow hopefully or somewhere this week!

Tried to put some code together which should make this work in a draft PR, will look at finalizing this tomorrow hopefully or somewhere this week!

Fantastic! Thanks @jfversluis for the help! 😃