Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ao_oss doesn't build under linux #9378

Closed
marillat opened this issue Nov 1, 2021 · 18 comments · Fixed by #9379
Closed

ao_oss doesn't build under linux #9378

marillat opened this issue Nov 1, 2021 · 18 comments · Fixed by #9379
Labels

Comments

@marillat
Copy link

marillat commented Nov 1, 2021

Debian amd64 4.2-build2020
Commit 1b2e513 in ao_oss.c is the guilty.

../audio/out/ao_oss.c: In function ‘device_descr_get’:
../audio/out/ao_oss.c:115:42: error: ‘PATH_DEV_MIXER’ undeclared (first use in this function); did you mean ‘PATH_DEV_DSP’?
  115 |     snprintf(dev_path, sizeof(dev_path), PATH_DEV_MIXER"%zu", dev_idx);
      |                                          ^~~~~~~~~~~~~~
      |                                          PATH_DEV_DSP
../audio/out/ao_oss.c:115:42: note: each undeclared identifier is reported only once for each function it appears in
../audio/out/ao_oss.c:115:56: error: expected ‘)’ before string constant
  115 |     snprintf(dev_path, sizeof(dev_path), PATH_DEV_MIXER"%zu", dev_idx);
      |             ~                                          ^~~~~
      |                                                        )

Waf: Leaving directory `/home/marillat/src/mpv-0.34.0/build'
@sfan5
Copy link
Member

sfan5 commented Nov 1, 2021

Linux does not support OSSv4 and hence cannot use ao_oss.
In fact waf should not let you enable oss because required features are missing.

@marillat
Copy link
Author

marillat commented Nov 1, 2021

But I was able to enable and build ossv4 in the previous release.

@jeeb
Copy link
Member

jeeb commented Nov 1, 2021

It was once removed, and then re-instated in a manner that was OSSv4 only for simplicity etc (to serve FreeBSD etc, where ALSA or PulseAudio etc are not alternatives).

If it is simple to fix (and it would actually work after those changes), then such a change could be pulled in, but as of right now Linux is out of scope for mpv's OSS audio output.

@marillat
Copy link
Author

marillat commented Nov 1, 2021

I must agree oss isn't really needed under Linux

@jeeb
Copy link
Member

jeeb commented Nov 1, 2021

BTW, how did you get that far in the build process? I would have thought that the configure check is rigid enough to not let you pass with standard kernel OSS?

@marillat
Copy link
Author

marillat commented Nov 1, 2021

waf isn't called with --enable-oss-audio
oss is detected if waf is able to build the test code

@jeeb
Copy link
Member

jeeb commented Nov 1, 2021

Yes, but as far as I know the autodetection check should fail? Can you provide a config.log?

@marillat
Copy link
Author

marillat commented Nov 1, 2021

config.log

@jeeb
Copy link
Member

jeeb commented Nov 1, 2021

Thanks.

Thus yes, you seem to have a <sys/soundcard.h> which does include SNDCTL_DSP_SETPLAYVOL. For me that file seems to come from glibc (?), and is just an include redirect towards <linux/soundcard.h>. Quick look through Debian's package listing seems to point towards a similar result.

On my Fedora machine doing the following gives no results so it doesn't seem to be a file included from soundcard.h, either.

grep -R "SNDCTL_DSP_SETPLAYVOL" /usr/include/*

Basically, I'm just trying to figure out what sort of check there should be so that Linux users wouldn't end up in the situation that you are in :) (the alternative is not only making it build, but also verifying that it successfully works without major changes).

@marillat
Copy link
Author

marillat commented Nov 1, 2021

linux/soundcard.h come from oss4-dev package. If I remove oss4-dev waf doesn't detect oss.

$ grep -R "SNDCTL_DSP_SETPLAYVOL" /usr/include/*
/usr/include/linux/soundcard.h:#define SNDCTL_DSP_SETPLAYVOL		__SIOWR('P', 24, int)
$ dlocate /usr/include/linux/soundcard.h        
oss4-dev: /usr/include/linux/soundcard.h
linux-libc-dev:amd64: /usr/include/linux/soundcard.h
diversion by oss4-dev from: /usr/include/linux/soundcard.h
diversion by oss4-dev to: /usr/include/linux/soundcard.h.oss3
$ sudo apt-get purge oss4-dev
then
$ grep -R "SNDCTL_DSP_SETPLAYVOL" /usr/include/*
return nothing.

@jeeb
Copy link
Member

jeeb commented Nov 1, 2021

Thanks for the info. The Debian code is in here. So yea, I guess we just need to test for PATH_DEV_MIXER at the very least.

jeeb added a commit to jeeb/mpv that referenced this issue Nov 1, 2021
Apparently Debian does package just an old enough OSSv4 that checking
for just the play volume control does work, but the mixer path
is not defined.

This properly keeps the failure to the configure step, not letting
it get all the way to the build step. And if Debian ever updates
their OSSv4 headers, then it might just end up building as-is?

Fixes mpv-player#9378
@jeeb
Copy link
Member

jeeb commented Nov 1, 2021

@marillat could you verify that #9379 properly makes the configure check fail with Debian's OSSv4?

@marillat
Copy link
Author

marillat commented Nov 1, 2021

Works fine. Thanks.

@jeeb
Copy link
Member

jeeb commented Nov 1, 2021

Also if you're interested in testing the other way,

diff --git a/audio/out/ao_oss.c b/audio/out/ao_oss.c
index 11b182e52d..04b2c75c88 100644
--- a/audio/out/ao_oss.c
+++ b/audio/out/ao_oss.c
@@ -49,6 +49,9 @@
 #endif
 
 #define PATH_DEV_DSP "/dev/dsp"
+#ifndef PATH_DEV_MIXER
+#define PATH_DEV_MIXER "/dev/mixer"
+#endif
 
 struct priv {
     int dsp_fd;

Should probably let it build, but I have no idea if OSSv4 is even possible to utilize under Debian :D .

@42147
Copy link

42147 commented Nov 2, 2021

Can confirm OSSv4 works fine on the Debian based MX Linux 19.4 and Xubuntu 18.04 distros. If at all possible, would like to keep using ao=oss when switching over to MX-21.

@marillat
Copy link
Author

marillat commented Nov 2, 2021

Also if you're interested in testing the other way,

Thanks build fine.

Should probably let it build, but I have no idea if OSSv4 is even possible to utilize under Debian :D .

I think it was working before, but I can't test myself.

@jeeb
Copy link
Member

jeeb commented Nov 2, 2021

@42147 The OSS AO was rewritten to be simpler and OSSv4-only. If you have an actual interest/requirement for this, please test building with that patch to give a data point whether we should disable it on Linux or enable it to build.

All of the people going to poke this AO as far as I know are *BSD people. So Linux is not going to receive a lot of love unless it's a common issue.

@42147
Copy link

42147 commented Nov 2, 2021

@42147 The OSS AO was rewritten to be simpler and OSSv4-only. If you have an actual interest/requirement for this, please test building with that patch to give a data point whether we should disable it on Linux or enable it to build.

Tested version 0.34 with the patch on Xubuntu 18.04. Worked great!

All of the people going to poke this AO as far as I know are *BSD people. So Linux is not going to receive a lot of love unless it's a common issue.

Yes. Unfortunately the relatively newly added OSS backend on Firefox was already limited to *BSD only. There hasn't been any development on OSSv4 for years on Linux, but it is maintained to work even on the latest kernels. It's still useful with older PCI sound cards because the developers worked on the drivers with some of the sound card manufacturers when they went proprietary for a while back in the day.

jeeb added a commit to jeeb/mpv that referenced this issue Nov 2, 2021
This fixes a mismatch between configure working and build time
failing with Linux + OSSv4, enabling compilation on Debian based
Linux systems with the ossv4-dev package.

Fixes mpv-player#9378
jeeb added a commit to jeeb/mpv that referenced this issue Nov 2, 2021
This fixes a mismatch between configure working and build time
failing with Linux + OSSv4, enabling compilation on Debian based
Linux systems with the oss4-dev package.

Fixes mpv-player#9378
jeeb added a commit to jeeb/mpv that referenced this issue Nov 6, 2021
This fixes a mismatch between configure working and build time
failing with Linux + OSSv4, enabling compilation on Debian based
Linux systems with the oss4-dev package.

Fixes mpv-player#9378
sfan5 pushed a commit that referenced this issue Nov 10, 2021
This fixes a mismatch between configure working and build time
failing with Linux + OSSv4, enabling compilation on Debian based
Linux systems with the oss4-dev package.

Fixes #9378
sfan5 pushed a commit to sfan5/mpv that referenced this issue Dec 19, 2021
This fixes a mismatch between configure working and build time
failing with Linux + OSSv4, enabling compilation on Debian based
Linux systems with the oss4-dev package.

Fixes mpv-player#9378
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants