fossmium / OneDrive-Cloud-Player

OneDrive Cloud Player is a media player dedicated for streaming files directly from OneDrive.
GNU General Public License v2.0
60 stars 7 forks source link

Reenable x86; small .csproj fixes #78

Closed JohannesKauffmann closed 2 years ago

JohannesKauffmann commented 2 years ago
JohannesKauffmann commented 2 years ago

It seems like our own built LibVLC nupkg has x86 disabled. We probably should have included whatever changes we did to libvlc-nuget to disable x86 in this repository.

Two possible solutions:

The second option might seem easier, however, if we go with the first option, we can provide our users with a more up-to-date version of LibVLC. Additionally, if we ever want to include ARM64 support, we have to build ourselves anyway (and document modifications to libvlc-nuget properly), because it doesn't seem like the maintainer will ever get to it.

TimGels commented 2 years ago

It seems like our own built LibVLC nupkg has x86 disabled. We probably should have included whatever changes we did to libvlc-nuget to disable x86 in this repository.

Two possible solutions:

* Update our nupkg to also include x86. While I might be able to retrieve the LibVLC version we used to build, I'm not sure I can do the same for the necessary UWP patches applied at build time, since at that time these patches moved from [vlc-winrt](https://code.videolan.org/videolan/vlc-winrt/-/tree/master/libvlc/patches) to [libvlc-nuget](https://code.videolan.org/videolan/libvlc-nuget/-/tree/master/patches). Additionally, the build script for the official UWP nupkg's was switched from vlc-winrt to the official VLC 3.0.x script. Which is to say: I cannot guarantee that the x86 version will be the same as our current x64 and ARM builds, so when we do this, we should probably use the most LibVLC verison.

* Use the official 3.3.2 UWP nupkg from Nuget instead, which includes almost a year-old LibVLC.

The second option might seem easier, however, if we go with the first option, we can provide our users with a more up-to-date version of LibVLC. Additionally, if we ever want to include ARM64 support, we have to build ourselves anyway (and document modifications to libvlc-nuget properly), because it doesn't seem like the maintainer will ever get to it.

Does the second solution not give us problems like the audio issue because of the old libvlc version?

JohannesKauffmann commented 2 years ago

For the second option, normally it would also be possible to simply grab the UWP archives created by the libvlc-nuget nightly CI. These include i686, x86_64 and armv7. However, since that CI has the version hardcoded to 3.0.16, it currently doesn't upload artifacts because no files match that version (VLC is at 3.0.17.4 already).

JohannesKauffmann commented 2 years ago

Does the second solution not give us problems like the audio issue because of the old libvlc version?

It shouldn't, as it was published right after the fix for #24 was merged. But at this point, it's an older version of LibVLC.

TimGels commented 2 years ago

I personally rather use a more up to date LibVLC without support for x86. 32 bit seems to be rarely used anyway these days. What is your view on this?

JohannesKauffmann commented 2 years ago

At the moment I think 1) the official 3.2.2 nupkg and 2) our build without x86 are both as much out of date as the other, so if we want to use a more up-to-date LibVLC, we need to rebuild either way. If you're already rebuilding, it is trivial to add x86 support. It is easier not to hack libvlc-nuget to exclude x86 and just use its default config, which is to include x86.

I don't see the (lack of) populairity of 32bit as a reason to leave it out.

TimGels commented 2 years ago

At the moment I think 1) the official 3.2.2 nupkg and 2) our build without x86 are both as much out of date as the other, so if we want to use a more up-to-date LibVLC, we need to rebuild either way. If you're already rebuilding, it is trivial to add x86 support. It is easier not to hack libvlc-nuget to exclude x86 and just use its default config, which is to include x86.

I don't see the (lack of) populairity of 32bit as a reason to leave it out.

If including 32 bit support does not add to the complexity or time when rebuilding LibVLC, I agree that we might as well include it.

JohannesKauffmann commented 2 years ago

Blocked by #80. After that, the new LibVLC version with x86 will be pulled automatically.

JohannesKauffmann commented 2 years ago

This should work now out-of-the-box.

TimGels commented 2 years ago

It also seems to pull the custom nuget perfectly fine at build time after having deleted the old libvlcsharp uwp nuget package from my system.

JohannesKauffmann commented 2 years ago

Perfect.