xamarin / xamarin-macios

.NET for iOS, Mac Catalyst, macOS, and tvOS provide open-source bindings of the Apple SDKs for use with .NET managed languages such as C#
Other
2.49k stars 515 forks source link

AVAssetReaderOutput.MediaType returns string and not smart enum #16318

Open Cheesebaron opened 2 years ago

Cheesebaron commented 2 years ago

Steps to Reproduce

In Xamarin.iOS we could check for MediaType on a AVAssetTrack like so:

if (track.MediaType == AVMediaType.Video)
   // do something

AVMediaType is no longer available in net6.0-ios and seems like it has been replaced with the enum AVMediaTypes. However, using that for comparison does not work as MediaType does not directly map into that enum.

This example doesn't work in net6.0-ios as MediaType would return "vide" and not "Video".

if (track.MediaType == AVMediaTypes.Video.ToString())
  // do something

Expected Behavior

AVMediaType static class being available for easy comparison with AVAssetTrack MediaType

Actual Behavior

Not available and replaced with AVMediaTypes enum

Environment

Version information ``` net6.0-ios Installed Workload Ids Manifest Version Installation Source --------------------------------------------------------------------- macos 12.3.454/6.0.400 SDK 6.0.400 ios 15.4.454/6.0.400 SDK 6.0.400 maccatalyst 15.4.454/6.0.400 SDK 6.0.400 maui 6.0.540/6.0.400 SDK 6.0.400 tvos 15.4.454/6.0.400 SDK 6.0.400 android 32.0.465/6.0.400 SDK 6.0.400 android-33 32.0.465/6.0.400 SDK 6.0.400 ```

Build Logs

Example Project (If Possible)

Cheesebaron commented 2 years ago

Seems like it is possible to use GetConstant() extension method on AVMediaTypes enum are there more of these breaking changes?

chamons commented 2 years ago

This might be because https://github.com/xamarin/xamarin-macios/blob/main/src/avfoundation.cs#L3823-L3824 was not updated to return the smart enum.

I'm looking into where this change was made exactly.

Using GetConstant on the smart enum to get the value for API usages that are bound as string is correct.

chamons commented 2 years ago

Yep, it was was done here.

And a number of tests were updated like this:

image

I'm going to leave this open to suggest adding smart enum support to that API. I'm not sure we can do that in an API safe way, thoughts @rolfbjarne ?

rolfbjarne commented 2 years ago

We can't do this in an API-compatible way (unless we expose using a different name, which we could certainly do).

This also seems to affect: