pinterf / AviSynthPlus

AviSynth with improvements
http://avs-plus.net
208 stars 24 forks source link

DirectShowSource: Sensible defaults for CMake baseclasses lib, removed dx include path #9

Open ignus2 opened 7 years ago

ignus2 commented 7 years ago

It seems to me that the DX includes are not needed to build DirectShowSource (dshow headers are in the platform sdk, not in dx). Also added defaults for baseclasses lib in case you just build it in it's original location. I don't know if it's OK or not like this, ideas?

Also this brought up an issue that was probably there before: CMake configure needs to be run twice for CMAKE_GENERATOR_TOOLSET to take effect. If you configure only once and open the solution, the projects will have "vs140" toolset instead of "vs140_xp". If you hit configure again, it gets fixed. The only report of this I found was here, but no solution: http://stackoverflow.com/questions/25302502/have-cmake-select-msvc-platform-toolset-explicitly

qyot27 commented 7 years ago

If the DirectX headers really aren't necessary, then they definitely should be removed. Is there any indication that the DirectShow headers rely on the DirectX headers silently, and that's why they're there? In any case, this long-standing issue of DirectShowSource requiring outside headers/libs - be it Platform SDK or DirectX - is why when I expanded the plugin building options in my mingwfix branch, I made DirectShowSource opt-in rather than opt-out. If a user doesn't want to build DirectShowSource or install the Platform SDK, they shouldn't have to resort to editing the CMakeLists to get around that.

I think the issue with the CMAKE_GENERATOR_TOOLSET may not have been caught earlier because the cmake builder tool circumvents it, and that most times people are building AviSynth+ with the command line, not the MSVC IDE (or they just don't care about XP support and overlooked the fact that that happens). The cmake build tool probably re-invokes the configure step before it starts building, which does the same thing as running the cmake step twice and then using the IDE. The way to actually get around it would be to not use CMAKE_GENERATOR_TOOLSET, but to have an option like WINXP_SUPPORT which sets all the necessary variables**. It's likely some kind of a bug in CMake that the TOOLSET doesn't take after one round of configuring.

**From the VS Command Prompt itself:

set INCLUDE=C:\Program Files\Microsoft SDKs\Windows\7.1A\Include;%INCLUDE%
set PATH=C:\Program Files\Microsoft SDKs\Windows\7.1A\Bin;%PATH%
set LIB=C:\Program Files\Microsoft SDKs\Windows\7.1A\Lib;%LIB%
set LINK=/SUBSYSTEM:CONSOLE,"5.01"
set CL=/D_USING_V110_SDK71_

From MSys2:

INCLUDE="C:\Program Files\Microsoft SDKs\Windows\7.1A\Include;$INCLUDE"
PATH="C:\Program Files\Microsoft SDKs\Windows\7.1A\Bin;$PATH"
LIB="C:\Program Files\Microsoft SDKs\Windows\7.1A\Lib;$LIB"
LINK="/SUBSYSTEM:CONSOLE,\"5.01\""
CL="/D_USING_V110_SDK71_"

The above obviously for 32-bit hosts and 32-bit targets.

ignus2 commented 7 years ago

No, the dshow headers have long been a part of the platform sdk (maybe it was earlier in the DX SDK), but now it has nothing to do with directx (other than it's name being "direct" :) ), and doesn't rely on it. But try to compile and you'll see (at least, it compiled for me).

About the second: well I'm the exception then, I build with VS2015 with the generated solution files :) For a newcomer, that seemed the easiest, and it just worked. Anyway, I don't see any easy solution, so I think it just have to be mentioned in the README that

About building DirectShowSource with mingw, the baseclasses must be tweaked, but seems doable: http://www.step.polymtl.ca/~guardia/programming.php http://classic.makesweet.com/bozo/2008/01/24/compiling-directshow-with-mingw-on-linux/

One way to not require the Windows 7.1 SDK at all would be to extract the baseclasses sources from the SDK and copy it under the DirectShowSource plugin sources dir, fix it up there for mingw, and make it part of the build process.

qyot27 commented 7 years ago

Integrating the baseclasses into the repo is a non-starter, IMO (classic AviSynth did that between 2.6.0 and 2.6.1 and I cringed). It's not just bad from an organizational standpoint, there's no telling what kind of license nightmare that could end up causing.

Not to mention that we don't support the old MinGW (which is what both those links are talking about), only MinGW-w64 - and they may or may not already include the rough framework for getting DirectShow applications running without Microsoft-provided headers/libs, although not in the same directory structure: https://sourceforge.net/p/mingw-w64/feature-requests/69/

MinGW-w64 5.0.0 was released just a couple weeks ago, so I have yet to test with it for anything yet.

(Although I really wasn't mentioning the branch/pull request for the mingw stuff; the patch that adds the ability to disable individual plugins from the build configuration and defaults DSS to off is true for MSVC builds too.)

ignus2 commented 7 years ago

Sorry for being a bit late with the reply. In general, there would be nothing wrong with integrating the baseclasses and tweaking it as necessary to compile with mingw-w64 (it's "sample code" after all), if the license would allow that. Which it tries to do in general (go ahead and read it), however there are more than one points under the "Distribution Requirements/Restrictions" sections which are no-gos. So because of this, the idea to integrate and tweak the baseclasses can be ditched.

I wouldn't necessarily disable DSS by default, it compiles and works fine and I believe it is useful. And it "only" needs the 7.1 Platform SDK, the DX SDK is not required.

I'll try to see if the baseclasses can be built with mingw-w64, and if so, those instructions can also be provided for people who want to build with mingw.

qyot27 commented 7 years ago

No plugins can be built with MinGW-w64 right now anyway, so whether DSS can doesn't really matter. GCC support is very experimental as it is, so it's not an immediate concern.

The reason DSS should be disabled by default is because it requires installing more than just Visual Studio, and if you do not have that extra Platform SDK (and currently DirectX, even if unneeded) the build process will fail if the user doesn't go in and edit CMakeLists.txt. It should absolutely not be necessary to do that, and it also should absolutely not fail if those extra things are missing. The only way to do that is to make DSS optional, and let users who really want it do the legwork and enable it if they want to, but not drag everyone else along, which is exactly what I did in that branch. Especially since DSS should only ever be used as a last resort for source input.