Baseflow / ExoPlayerXamarin

Xamarin bindings library for the Google ExoPlayer library
https://baseflow.com
MIT License
153 stars 67 forks source link

Add MonoAndroid support along with MAUI #145

Closed ArchangelWTF closed 1 year ago

ArchangelWTF commented 1 year ago

:sparkles: What kind of change does this PR introduce? (Bug fix, feature, docs update...)

Allows ExoPlayerXamarin to support both MonoAndroid and MAUI at the same time

:arrow_heading_down: What is the current behavior?

ExoPlayerXamarin only supports MAUI

:new: What is the new behavior (if this is a feature change)?

:boom: Does this PR introduce a breaking change?

Not that I know of.

:bug: Recommendations for testing

:memo: Links to relevant issues/docs

:thinking: Checklist before submitting

martijn00 commented 1 year ago

Hmmm, I love the effort you put in but I find it hard to pull this into the code base and a release because long term this is more difficult to support. People should move to .net6 or 7 anyways. Why would this be a good idea?

ArchangelWTF commented 1 year ago

Considering that MonoAndroid wont be End of Life until 2024 (Which means load of people will be still stuck in the migration of XF to MAUI) I'd say this is a perfect compromise where we can build for both somewhat painlessly.

I propose just supporting it for as long as ExoPlayerXamarin compiles with it (Meaning that the bindings dont get too complex for mono) I'll happily even maintain it since it wasn't too big of an issue to get it working in the way i've made it work above.

ArchangelWTF commented 1 year ago

Woops wasn't meant to press that, damn mobile phone.

sschaub commented 1 year ago

@martijn00 I hope you'll consider accepting this PR. While it's true that MAUI is the future, there are good reasons for projects not to move to it just yet. Articles such as this one indicate that MAUI is still not quite ready for prime time. In the meantime, Xamarin users who aren't ready to make the switch would benefit from having the latest ExoPlayer releases available to them.

martijn00 commented 1 year ago

I've been thinking about this. What about creating a new xamarin branch and releasing seperate nugets from there with a suffix -xamarin in the release title? Long term I wouldn't want to maintain the old stuff as it begins to break more and more.

sschaub commented 1 year ago

I can understand your concern about not wanting to maintain old stuff long term. Your idea of a separate branch and separate nugets would achieve that goal. But it would mean having to make updates to two separate branches in the short term. If you accept this PR, we would have a single branch to maintain in the short term. At some point in the future if maintenance of the Xamarin builds becomes problematic you could update the projects to be .net6 only, as they are at present, and perhaps consider making a Xamarin-only branch at that point if there is demand.

martijn00 commented 1 year ago

Yes, but the update process will be done by Github Actions, so not a problem there. Also you can see in the bindings now that there need to be separate fixes for both. I rather have 1 point of truth and backport that to the Xamarin version.

janwiebe-jump commented 1 year ago

Hi everyone. Best wishes for 2023 🎉 Is having a separate branch for the Xamarin version of ExoPlayerXamarin the way to go now?

martijn00 commented 1 year ago

@ArchangelWTF @janwiebe-jump i've done some changes to support both from 1 codebase (hopefully), i'm not sure if all the metadata changes are now correct in the xamarin branch? Could you check and if possible make a PR?

When it works like i hope it would, i will enable: https://github.com/Baseflow/ExoPlayerXamarin/blob/develop/.github/workflows/dotnet.yml#L45

janwiebe-jump commented 1 year ago

LGTM! Thanks so much!

martijn00 commented 1 year ago

For some reason dotnet pack doesn't work: MSBUILD : error MSB1003: Specify a project or solution file. The current working directory does not contain a project or solution file.

janwiebe-jump commented 1 year ago

@martijn00 I ran into this earlier. Looks like the Xamarin projects cant be built with dotnet build / pack. building with MSBuild should generate the packages, which can then be pushed with a dotnet nuget push

https://github.com/Baseflow/ExoPlayerXamarin/pull/147/files#diff-19a29f4e3c34d5224ac8726575c7750eb84080a5b97cd58439fdac6c13dda7c6

martijn00 commented 1 year ago

But i do build with msbuild. I want to use dotnet pack to re pack the already build nupkg