Use proper platform targeting

[softworkz]

The vcxproj targets 22000. The .Dotnet projection project is referencing it, so it must target 22000 at minimum (you can see an error when building in VS when trying otherwise). Any other project which references the .Dotnet project in turn, needs to target 22000 at minimum as well. That's no different when the referencing is going through a nuget package, and that means that the nuget package needs to require 22000 accordingly. This PR sets the numbers in the nuspec files correctly.

Another conclusion from this is that there's not really a choice for the targeting versions. The required version is determined by the features which are being used in the vcxproj. A lower version cannot be used, and there's no reason to use a higher version. In turn, there's no point in making this a choice in the PS build script. Even if this would still be desired, it would have to be changed in a way that the selection from Build-FFmpegInteropX.ps1 would not only need to be set in the vcxproj, but also in the projection projection, in the nuspec files and the corresponding targets files.

This PR removes the selection from the build script and hardcodes it in the vcxproj.

[lukasf]

You are right about the nuspec file. But if you look into the build script, you will see that I explicitly set the target platform version in every MSBuild call, overriding any setting in the project files. That way, we can be sure that all the projects use the very same target version, no matter what's in project files.

Instead of removing the config option, I'd rather use a parameter in the nuspec file, so we can pass the version in and be sure that everything is aligned.

[softworkz]

That way, we can be sure that all the projects use the very same target version, no matter what's in project files.

It doesn't change the TargetPlatform in FFmpegInteropX.Dotnet.

Instead of removing the config option, I'd rather use a parameter in the nuspec file, so we can pass the version in and be sure that everything is aligned.

That's possible as well - but why? There's is no point in having a choice, because you can't get lower than 22000 and it's pointless to go higher.

[lukasf]

That way, we can be sure that all the projects use the very same target version, no matter what's in project files.

It doesn't change the TargetPlatform in FFmpegInteropX.Dotnet.

It sure does:

        MSBuild.exe $SolutionDir\FFmpegInteropX.sln `
            /restore `
            /t:FFmpegInteropX_DotNet `
            /p:Configuration=${Configuration}_${WindowsTarget} `
            /p:Platform=$Platform `
            /p:WindowsTargetPlatformVersion=$WindowsTargetPlatformVersion `
            /p:useenv=true

Are you sure we can't get lower? I don't think that we really need the 22000 sdk. We could probably go as low as 19041, maybe even lower. I have not tried. But besides that, the thing is, we sometimes ran into build problems in the past using specific tool and sdk versions. That's why we added the config options in the build file, so we can easily experiment and adjust. I won't throw them out now, they might come in handy again at a later time.

[softworkz]

It sure does:

No, that's not the right thing. What needs to be set is this:

<TargetFramework>net6.0-windows10.0.22000.0</TargetFramework>

When it is lower than the target in the vcxproj, then it fails in Visual Studio. The command line builds are just hiding this :-)

Are you sure we can't get lower? I don't think that we really need the 22000 sdk.

No we can't. There's one lower target than 22000: 20348 - which might work. But it's a rarely used SDK - I haven't seen it in any samples, so I would be careful.

We could probably go as low as 19041, maybe even lower

No, The increase is required since you added the new subtitle features (like 'Bouten' marks and the likes). Those things are not available in 19041. And that's why I'm saying that there's no reasonable choice to make.

[lukasf]

Okay, so we might have to set a different property for CS projects. I'd still rather fix this than removing it.

[softworkz]

Okay, so we might have to set a different property for CS projects. I'd still rather fix this than removing it.

It's fine of course when all related properties get set automatically.

And it would even make sense if you would enclose those parts which require a newer SDK in preprocessor conditions, then there would be at least some value in being able to specify some version.

[softworkz]

One concern that I have is that I think that consistency must not rely on the command line. Everything should be consistent in the same way, when using the Visual Studio solution.

This could be achieved by adding a common.props file and including it in all projects. It can set things like the platform toolset and the sdk versions:

<Project>
  <!-- Set SdkVersion if not already set -->
  <PropertyGroup Condition="'$(SdkVersion)' == ''">
    <SdkVersion>22000</SdkVersion>
  </PropertyGroup>

  <!-- Use SdkVersion to set other properties -->
  <PropertyGroup>
    <WindowsTargetPlatformVersion Condition="'$(WindowsTargetPlatformVersion)' == ''">10.0.$(SdkVersion).0</WindowsTargetPlatformVersion>
    <TargetPlatformVersion Condition="'$(TargetPlatformVersion)' == ''">10.$(SdkVersion).0</TargetPlatformVersion>
    <TargetFramework Condition="'$(TargetFramework)' == ''">net6.0-windows10.0.$(SdkVersion).0</TargetFramework>
  </PropertyGroup>

  <!-- Conditions for setting PlatformToolset based on Visual Studio version -->
  <!-- Visual Studio 2017 (v141) -->
  <PropertyGroup Condition="'$(VisualStudioVersion)' == '15.0' And '$(PlatformToolset)' == ''">
    <PlatformToolset>v141</PlatformToolset>
  </PropertyGroup>

  <!-- Visual Studio 2019 (v142) -->
  <PropertyGroup Condition="'$(VisualStudioVersion)' == '16.0' And '$(PlatformToolset)' == ''">
    <PlatformToolset>v142</PlatformToolset>
  </PropertyGroup>

  <!-- Visual Studio 2022 (v143) -->
  <PropertyGroup Condition="'$(VisualStudioVersion)' == '17.0' And '$(PlatformToolset)' == ''">
    <PlatformToolset>v143</PlatformToolset>
  </PropertyGroup>

</Project>

This still leaves room for overriding the values when building from the command line, while it eliminates the need for checking the VS separately.

[lukasf]

It's a good idea about the common.props file. Best would be having both, the common.props to easily align versions across all projects in VS, plus the flexibility to override this using command line build.

[lukasf]

I made a new PR which fixes the sdk versions instead of deleting the parameter from the script.

[softworkz]

It's a good idea about the common.props file. Best would be having both, the common.props to easily align versions across all projects in VS, plus the flexibility to override this using command line build.

This is perfectly possible with the props sample which I posted above. All variable assignments are checking whether the variable has a value already (from the command line), so cmd params will have precedence.

[brabebhin]

Included in #387