Add TGVOIP_DISABLE_PULSEAUDIO directive

[reagentoo]

Add possibility to build libtgvoip if there are no pulseaudio in the system.

[stek29]

IMHO it'd be better to make a "TGVOIP_WITHOUT_PULSEAUDIO" or something like that to avoid breaking current behavior

[grishka]

You only need the headers for libpulse to build it and you only need them during build time. You have to have libpulse headers anyway to build OpenAL which Telegram Desktop depends on. BTW, there's no runtime dependency on libpulse, it tries to dynamically load and then initialize PulseAudio or ALSA (in that order).

[reagentoo]

@grishka, However, this is an extra dependency on the package. Put yourself in the place of the package developer. Please look at the related request for Telegram: https://github.com/telegramdesktop/tdesktop/pull/3778

[msva]

@grishka Although, I myself use pulseaudio-enabled version, it was me, who initiated the work on no-pulseaudio builds, since: 1) there was many and many problems due to pulseaudio that people reported to me (as maintainer). And it was only complete disabling pulseaudio what helped, for example, with issue about freezing on the tdesktop start for some people. 2) there is many distributions where headers and binaries aren't splitted to separate packages, so people will anyway need to install PA, which they'd like to avoid. 3) there is also PA-disabling PR into tdesktop, so, I bet, giving people more choice (to not use PA where they can avoid it if they want to) is good.

[msva]

by the way, @john-preston just merged such PR in tdesktop

[reagentoo]

@grishka Please, give a final decision.

[stek29]

@reagentoo Didn't he?

[grishka]

@reagentoo @msva this isn't exactly "such" PR because it's about GTK and, if I understand correctly, about the library dependency, not the headers.

That being said, I would merge a PR that adds the option to completely disable PulseAudio support (not the other way around) if TDesktop merges one first since it makes little sense to have such an option in libtgvoip but not in the only Linux app that depends on it.

[Civil]

IMHO:

1. In CMake it's more idiomatic to do 'USE_PULSE' with defaults for "True", than to "WITHOUT", "DISABLE", etc. Correct me if I'm wrong, but gyp is used to generate CMake files, so it should generate as idiomatic output as possible
2. It makes more sense to make Pulse optional here and than push TDesktop developers to make it optional on their side, otherwise there'll be high chances to get a "No" in response with the reasoning "tgvoip doesn't support that". Also correct me if I'm wrong, but tdesktop doesn't use PulseAudio directly. At least grep -ri pulse * 2>&1 | egrep -v '(xml|txt|md|json):' over tdesktop git tree doesn't return anything
3. From the maintainer's point of view it makes more sense to make pulse optional in both places in parallel - for now current state means that maintainer will need to have 2 different patches - one for telegram, second one for tgvoip. If you merge this PR, maintainer will need to maintain only one patch (and only until it's merged to TDesktop).
4. OpenAL doesn't strictly require PulseAudio and pulse headers. It's compile-time switch.
5. In Gentoo if you want to have pulse headers, you need to have pulseaudio compiled and installed, this is what's ideally should be avoided. So it's about both headers and libraries.

[reagentoo]

@grishka Changed to "TGVOIP_DISABLE_PULSEAUDIO". Should I edit gyp-file?

[grishka]

@Civil

Correct me if I'm wrong, but gyp is used to generate CMake files, so it should generate as idiomatic output as possible

Have you ever seen those files? They don't really look like they're meant for humans to look at or edit. :)

Also, I don't want the default to be the opposite of what is used for official TDesktop builds.

Also correct me if I'm wrong, but tdesktop doesn't use PulseAudio directly.

TDesktop outsources all its audio to OpenAL and doesn't use any OS-specific audio APIs directly. OpenAL, in turn, uses PulseAudio the exact same way I do, by dynamically loading the library in runtime and using the headers to avoid copy-pasting a bunch of definitions from them.

OpenAL doesn't strictly require PulseAudio and pulse headers. It's compile-time switch.

TDesktop doesn't use that switch for now, you're free to submit a PR to add an option to use it.

In Gentoo

Can't say for the TDesktop dev, but I personally haven't tested the build process on distributions other than Ubuntu that the build manual is written for, so as it always is in the Linux world, YMMV depending on your particular setup.

[reagentoo]

It seems that PR is closed now. The last changes left here https://github.com/reagentoo/libtgvoip/commit/e26a86ed3b4ee4683c1a170e6bcec794bfffa608

[Civil]

@grishka you ignored main point of my message. So let me rephrase:

There is much higher chances to go from bottom to top in terms of making pulse optional. So that makes more sense to first make it optional here and then convince tdesktop developers to make it optional. That's the reason why this PR should be merged BEFORE opening PR in tdesktop.

Can't say for the TDesktop dev, but I personally haven't tested the build process on distributions other than Ubuntu that the build manual is written for, so as it always is in the Linux world, YMMV depending on your particular setup.

So that's why we are here, discussing this PR. Gentoo allows you to configure software. Currently it's impossible to do without patches (that needs to be mainted by package maintainers). Also that's the reason why headers must be optional. If they are not, pulseaudio will be a build-time dependency for this package and that's wrong.

Also currently both libtgvoip and tdesktop builds ok in Gentoo, but package maintainers forced to maintain bunch of patches to make pulseaudio optional, etc, etc, etc.