deps.ffmpeg: Enable PDBs for FFmpeg builds on Windows

[RytoEX]

Description

Enable PDBs for FFmpeg builds on Windows if the Configuration is set to Debug or RelWithDebInfo.

Motivation and Context

Having PDBs for FFmpeg can be useful for debugging crashes. This was one of the original goals of moving the FFmpeg builds to be natively compiled.

How Has This Been Tested?

Tested locally about a year ago. Should probably be retested.

Types of changes

• Tweak (non-breaking change to improve existing functionality)

Checklist:

• My code has been run through clang-format.
• I have read the contributing document.
• My code is not on the master branch.
• The code has been tested.
• All commit messages are properly formatted and commits squashed where appropriate.
• I have included updates to all appropriate documentation.

[PatTheMav]

Wouldn't it make sense to always add this flag? Calling PDBs "debug"-symbols is a bit of a misleading name, as "debug" is their purpose but is not tied to the build configuration. It is perfectly fine (and indeed expected) to generate PDB symbol files in Release configuration (or particularly for that configuration) to ship stripped release binaries to users while still being able to debug/symbolicate issues.

[RytoEX]

We could. My recollection from our conversation when I first wrote this a year ago was that blanket adding -DEBUG all the time was a quick hack and that we should only add it as needed. If our current consensus is that we should always use this flag, then we can do that.

[PatTheMav]

If we currently build in RelWithDebInfo on Windows, then this change is sufficient for the goal of this PR - if we build in Release we would still get no PDBs for an obs-deps release.

Otherwise we could look into enabling PDBs for Release separately.

[RytoEX]

The default configuration for .github/actions/build-ffmpeg/action.yaml is Release and it seems that we currently specify that in .github/workflows/main.yaml and .github/workflows/scheduled.yaml as well.

[PatTheMav]

That was my hunch - so if we actually want to have PDBs generated for obs-deps releases, then the DEBUG switch needs to be probably added regardless of selected build configuration.

This will work for CMake-based projects using the MSVC build system, but I'm not sure about custom build systems like FFmpeg's.

[RytoEX]

How do we want to proceed here?

[RytoEX]

Following up a little here, if the current form is correct, it looks like I need to move the closing single-quote to the end:

Suggested change

	('--extra-ldflags=' + "'-APPCONTAINER:NO -MACHINE:${Target}'" + $(if ( $Configuration -match '(Debug|RelWithDebInfo)' ) { ' -DEBUG' }))
	('--extra-ldflags=' + "'-APPCONTAINER:NO -MACHINE:${Target}" + $(if ( $Configuration -match '(Debug|RelWithDebInfo)' ) { ' -DEBUG' }) + "'")

If --extra-ldflags='-DEBUG' is the same as /DEBUG, then we'll have to be mindful that it changes the defaults of /OPT (REF to NOREF and ICF to NOICF) and enables /INCREMENTAL. We seem mostly fine with that on obs-studio (all non-DEBUG builds use /DEBUG /OPT:REF /OPT:ICF /LTCG /INCREMENTAL:NO and all other builds use /DEBUG with its own defaults).

If we want to mimic that here, we would need something like:

Suggested change

	('--extra-ldflags=' + "'-APPCONTAINER:NO -MACHINE:${Target}'" + $(if ( $Configuration -match '(Debug|RelWithDebInfo)' ) { ' -DEBUG' }))
	('--extra-ldflags=' + "'-APPCONTAINER:NO -MACHINE:${Target} -DEBUG" + $(if ( $Configuration -match '(Release|RelWithDebInfo|MinSizeRel)' ) { ' -OPT:REF -OPT:ICF -LTCG -INCREMENTAL:NO' }) + "'")

Or we add default LDFLAGS to Setup-BuildParameters and consume them here (and everywhere else).

[PatTheMav]

I spent some time diving into these parameters for MSVC because I wanted to switch to using CMAKE_INTERPROCEDURAL_OPTIMIZATION to also enable similar optimisations for macOS and Linux and checking my pending branch it seems that I:

• Enabled CMAKE_INTERPROCEDURAL_OPTIMIZATION for "Release" and "MinSizeRel" only
• Similarly kept /OPT:REF and /OPT:ICF for "Release" and "MinSizeRel" only
• Removed /GL for non-"Debug" configurations

This fits with your findings and Microsoft's documentation that these optimisations need to be manually re-enabled if /DEBUG is specified. As you mentioned /INCREMENTAL is enabled by it as well, but the docs also state that the linker will always do a full link if either of those two optimisations are enabled, so they might cancel each other out.

CMake will by itself add /GL, /INCREMENTAL:NO, and /LTCG, as such my changes make sense as they'd be superfluous (/GL and /LTCG kind-of go hand in hand).

---

Tying all these together it seems that the flags on obs-studio are already going the right way if you have to specify them all explicitly:

• /DEBUG and /Zi (compiler flag) to get PDBs at all
• /OPT:REF and /OPT:ICF to get Release optimisations back
• /INCREMENTAL:NO just to be safe
• /LTCG and /GL (compiler flag) if whole-program optimisations are desired.

So you got the extra linker flags about right and might only need to add the compiler flags.

[RytoEX]

Strangely, I do not recall having to add /Zi to get PDBs when I last ran this. Unfortunately, I won't be able to re-run a build any time soon.

[PatTheMav]

From the documentation it seems that /DEBUG will somehow always produce PDB files regardless even if /Z7 is provided (which would keep the debug information in the object files), so for the generation of those files it seems irrelevant.

/Zi seems to imply /DEBUG, but not the other way around (which makes sense, as the compiler flag expresses the intent to get separate PDB files, which requires /DEBUG to begin with).

As I don't think we have a specific preference as to how this file is generated, just providing /DEBUG should be sufficient.