roc-streaming / roc-toolkit

Real-time audio streaming over the network.
https://roc-streaming.org
Mozilla Public License 2.0
1.06k stars 213 forks source link

sox and libpulse are requirement for tooling but not library #506

Closed dvzrv closed 1 year ago

dvzrv commented 1 year ago

Hi! When building pipewire with roc-toolkit support I noticed that sox is required unconditionally during build time, but it is in fact not required by libroc.so, only by the tooling executables:

/usr/bin/roc-conv
  NEEDED               libunwind.so.8
  NEEDED               libspeexdsp.so.1
  NEEDED               libpulse.so.0
  NEEDED               libsox.so.3
  NEEDED               libstdc++.so.6
  NEEDED               libm.so.6
  NEEDED               libc.so.6
/usr/bin/roc-recv
  NEEDED               libuv.so.1
  NEEDED               libunwind.so.8
  NEEDED               libopenfec.so.1
  NEEDED               libspeexdsp.so.1
  NEEDED               libpulse.so.0
  NEEDED               libsox.so.3
  NEEDED               libstdc++.so.6
  NEEDED               libm.so.6
  NEEDED               libc.so.6
/usr/bin/roc-send
  NEEDED               libuv.so.1
  NEEDED               libunwind.so.8
  NEEDED               libopenfec.so.1
  NEEDED               libspeexdsp.so.1
  NEEDED               libpulse.so.0
  NEEDED               libsox.so.3
  NEEDED               libstdc++.so.6
  NEEDED               libm.so.6
  NEEDED               libc.so.6
/usr/lib/libroc.so.0.2
  NEEDED               libuv.so.1
  NEEDED               libunwind.so.8
  NEEDED               libopenfec.so.1
  NEEDED               libspeexdsp.so.1
  NEEDED               libstdc++.so.6
  NEEDED               libm.so.6
  NEEDED               libc.so.6

It would be great if the pkg-config integration for libroc would not carry libpulse and sox. Especially the latter is known to be problematic, hasn't seen updates for years (although dozens of CVEs) and would in general be best to avoid.

gavv commented 1 year ago

Hi, thanks for report and PR (#507).

Unfortunately, the fix from PR is totally incorrect. It's a shame for our CI that only one step failed on it :)

The patch removes search for pulse and sox dependencies which are needed by roc components: tools, tests, examples.

A proper fix would be different: these dependencies still should be searched, but they should be added only to corresponding components. To achieve this, we can use subenvs.tools, subenvs.tests, etc, instead of env, when adding them.

However, I think this change is not actually necessary:

So the question is, is this problem urgent? If not, let's just keep this issue open until we get rid of libsox. If this is urgent for some reason, we can think how we can implement a temporary hotfix without breaking things and writing too much code that will be thrown away later.

dvzrv commented 1 year ago

The patch removes search for pulse and sox dependencies which are needed by roc components: tools, tests, examples.

Those are still linked correctly though:

/usr/bin/roc-conv
  NEEDED               libunwind.so.8
  NEEDED               libspeexdsp.so.1
  NEEDED               libpulse.so.0
  NEEDED               libsox.so.3
  NEEDED               libstdc++.so.6
  NEEDED               libm.so.6
  NEEDED               libc.so.6
/usr/bin/roc-recv
  NEEDED               libuv.so.1
  NEEDED               libunwind.so.8
  NEEDED               libopenfec.so.1
  NEEDED               libspeexdsp.so.1
  NEEDED               libpulse.so.0
  NEEDED               libsox.so.3
  NEEDED               libstdc++.so.6
  NEEDED               libm.so.6
  NEEDED               libc.so.6
/usr/bin/roc-send
  NEEDED               libuv.so.1
  NEEDED               libunwind.so.8
  NEEDED               libopenfec.so.1
  NEEDED               libspeexdsp.so.1
  NEEDED               libpulse.so.0
  NEEDED               libsox.so.3
  NEEDED               libstdc++.so.6
  NEEDED               libm.so.6
  NEEDED               libc.so.6

So the question is, is this problem urgent? If not, let's just keep this issue open until we get rid of libsox. If this is urgent for some reason, we can think how we can implement a temporary hotfix without breaking things and writing too much code that will be thrown away later.

Well, if the pkgconf integration pulls in sox, then I have to have it as build dependency for other things (e.g. pipewire) and that is something I would like to circumvent.

gavv commented 1 year ago

I see.

I've found a simple workaround and pushed it: d1fe81334d1fbabcede76369f3316427b234e581

Now sox and pulse are excluded from .pc file.

Note that in future versions pulse likely will be re-included there, and sox will be gone at all.

Also you can disable sox and pulse completely in both lib and tools using --disable-sox and --disable-pulseaudio.

So I'm going to close this, but feel free to reopen if I'm missing something!

gavv commented 1 year ago

The fix will be part of 0.2.3

gavv commented 1 year ago

0.2.3 is out now