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.

Sign up for GitHub

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

WinForms/WPF .NET Core 3 support for LibVLCSharp #71

Merged
merged 2 commits into from
Oct 13, 2019
Merged

WinForms/WPF .NET Core 3 support for LibVLCSharp #71

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0c8eb95
WinForms/WPF .NET Core 3 support for LibVLCSharp
dashkan Oct 8, 2019
d6dedd0
add sample
dashkan Oct 9, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions LibVLCSharp.WPF/LibVLCSharp.WPF.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,14 @@ in a WPF app. It depends on LibVLCSharp.

LibVLC needs to be installed separately, see VideoLAN.LibVLC.* packages.
</Description>
<TargetFramework>net461</TargetFramework>
<TargetFrameworks>net461;netcoreapp3.0</TargetFrameworks>
<RootNamespace>LibVLCSharp.WPF</RootNamespace>
<NeutralLanguage>en</NeutralLanguage>
<LangVersion>7.3</LangVersion>
<Version>3.2.3</Version>
<PackageId>LibVLCSharp.WPF</PackageId>
<UseWPF>true</UseWPF>
<UseWindowsForms>true</UseWindowsForms>
<Authors>VideoLAN</Authors>
<Owners>VideoLAN</Owners>
<GeneratePackageOnBuild Condition=" '$(Configuration)' == 'Release' ">true</GeneratePackageOnBuild>
Expand All @@ -31,7 +32,7 @@ LibVLC needs to be installed separately, see VideoLAN.LibVLC.* packages.
<PackageReleaseNotes>https://code.videolan.org/videolan/LibVLCSharp/blob/master/NEWS</PackageReleaseNotes>
<PackageTags>libvlc;vlc;videolan;native;c/c++;video;audio;player;media;mediaplayer;codec;ffmpeg;xamarin;graphics;ios;android;linux;windows;macos;cross-platform</PackageTags>
</PropertyGroup>
<ItemGroup>
<ItemGroup Condition="'$(TargetFramework)'=='net461'">
Copy link
Collaborator

Choose a reason for hiding this comment

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

See if we can use the windows desktop sdk and clean up that file

Copy link
Member

Choose a reason for hiding this comment

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

Yep this whole ItemGroup can be removed.

<Reference Include="WindowsFormsIntegration" />
<Reference Include="System.Windows.Forms" />
<Reference Include="WindowsBase" />
Expand All @@ -50,6 +51,6 @@ LibVLC needs to be installed separately, see VideoLAN.LibVLC.* packages.
</Page>
<Page Remove="**\App.xaml" />
<ApplicationDefinition Include="**\App.xaml" />
<Reference Include="System.Xaml" />
<Reference Include="System.Xaml" Condition="'$(TargetFramework)'=='net461'"/>
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 this can be removed as well. Need to try a WPF netcore 3 sample to make sure

</ItemGroup>
</Project>
7 changes: 4 additions & 3 deletions LibVLCSharp.WinForms/LibVLCSharp.WinForms.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk.WindowsDesktop">
Copy link
Member

Choose a reason for hiding this comment

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

Let's use MSBuild.Sdk.Extras here :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure about that. MSBuild.Sdk.Extras was great in setting up xamarin/WPF project with sdk-style csproj with VS2017. Now that VS 2019 and net core 3 are out, the Windows desktop sdk does that job for VS2019, and I guess dotnet CLI too.
I tried MSBuild.Sdk.Extras 2.x not too long ago, and I didn't manage to make it work with VS 2017 anyway. I'm in favor of migrating to VS 2019.

Copy link
Member

Choose a reason for hiding this comment

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

If we can keep support for VS2017 without effort, I say we should.

And referencing MSBuild.Sdk.Extras here makes it follow the same convention than the WPF package (i.e. Microsoft.NET.Sdk.WindowsDesktop is implicitly referenced AFAIK).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, but I fear about the "without effort" part. I'm not sure VS2017 is still officially supported by MSBuild.Sdk.Extras...

<PropertyGroup>
<Title>LibVLCSharp.WinForms</Title>
<Summary>WinForms integration for LibVLCSharp</Summary>
Expand All @@ -11,7 +11,8 @@ in a Windows Forms app. It depends on LibVLCSharp.

LibVLC needs to be installed separately, see VideoLAN.LibVLC.* packages.
</Description>
<TargetFrameworks>net40</TargetFrameworks>
<TargetFrameworks>net40;netcoreapp3.0</TargetFrameworks>
<UseWindowsForms>true</UseWindowsForms>
<RootNamespace>LibVLCSharp.WinForms</RootNamespace>
<Version>3.2.3</Version>
<PackageId>LibVLCSharp.WinForms</PackageId>
Expand All @@ -31,7 +32,7 @@ LibVLC needs to be installed separately, see VideoLAN.LibVLC.* packages.
<ItemGroup>
<ProjectReference Include="..\LibVLCSharp\LibVLCSharp.csproj" />
</ItemGroup>
<ItemGroup>
<ItemGroup Condition="'$(TargetFramework)'=='net40'">
Copy link
Member

Choose a reason for hiding this comment

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

This Reference can be removed as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry guys.

I did a quick conversion to support netcoreapp3.0.

I should have probably spent more time understanding your build process. I used VS Code / PS Core terminal to compile the project. There are a lot of prerequisite VS 2017/19 payloads which I didn't want to install (UWP, Xamarin, etc).

I was going to update the WPF sample app, but the project is still using old project format.

Let me know if you want me to update the PR and remove the conditional references.

I would love a nuget package targeting netcoreapp3.0 ASAP :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

we're on VS2017/VS4Mac 2017, but we need to make te migration anyway.
I guess we should make the migration all at once and see if any of the project has incompatibilities.

I might be able to give that a try in the weekend.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I opened dashkan#2 , but I just noticed Martin already did it...

<Reference Include="System.Windows.Forms" />
</ItemGroup>
</Project>
7 changes: 5 additions & 2 deletions Samples/LibVLCSharp.WinForms.Sample/LibVLCSharp.WinForms.Sample.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,19 @@
<PropertyGroup>
<Title>LibVLCSharp.WinForms.Sample</Title>
<OutputType>WinExe</OutputType>
<TargetFramework>net40</TargetFramework>
<TargetFrameworks>net40;netcoreapp3.0</TargetFrameworks>
<UseWindowsForms>true</UseWindowsForms>
Copy link
Member

Choose a reason for hiding this comment

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

You can remove that line, it's already here.

<RootNamespace>LibVLCSharp.WinForms.Sample</RootNamespace>
<UseWindowsForms>true</UseWindowsForms>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="VideoLAN.LibVLC.Windows" Version="3.0.8" />
</ItemGroup>
<ItemGroup Condition="'$(TargetFramework)'=='net40'">
<Reference Include="System.Windows.Forms" />
Copy link
Collaborator

Choose a reason for hiding this comment

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

I've never tried netcoreapp 3.0 projects yet, but does UseWinForms add this reference only for netcoreapp or net framework applications as well?
I'd expect microsoft to ease multi-targetting as much as possible.

Copy link
Member

Choose a reason for hiding this comment

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

Yep, unnecessary.

</ItemGroup>
<ItemGroup>
<ProjectReference Include="..\..\LibVLCSharp.WinForms\LibVLCSharp.WinForms.csproj" />
<Reference Include="System.Windows.Forms" />
</ItemGroup>
<ItemGroup>
<Compile Update="Form1.cs">
Expand Down