libxmp / xmp-cli

Command-line mod player using libxmp
GNU General Public License v2.0
75 stars 22 forks source link

Fix NetBSD driver compile error and OSS link error. #53

Closed AliceLR closed 2 years ago

AliceLR commented 2 years ago

I found two errors preventing NetBSD builds from working while testing #51:

My VM is silent but plays back girl_from_mars.xm at the correct rate, so presumably the NetBSD driver works It works, just had to switch the VM from AC'97 to Intel HD.

ccawley2011 commented 2 years ago

This might be a good time to backport some of the NetBSD and Solaris fixes from pkgsrc:

There are also a couple of patches in the OpenBSD ports tree that might be worth backporting:

AliceLR commented 2 years ago

The two live OpenBSD patches look like minor build system tweaks and I don't know how much they've been compatibility tested outside of OpenBSD (probably not...).

The NetBSD/SunOS patches look more interesting:

sezero commented 2 years ago

The NetBSD/SunOS patches look more interesting:

Increasing the maximum sample rate in xmp and libxmp to 192k is reasonable IMO and has been requested elsewhere. In libxmp this is in xmp.h so probably a minor version bump. Increasing this increases the maximum frame size, which means the mixer will allocate something like 4 times as much RAM for the frame buffers. This is the only tangible negative change this will cause.

The operative word is that it will have a negative change. I wonder what @cmatsuoka would think about it

Minor SunOS compatibility tweak in common.h. I need to push someone's KallistiOS patch to this file anyway so I'll just group these together.

That one should now be applied to configury and cmake'ry

The exact same rvalue fix as in this patch.

That one is indeed needed

Their patch also changes the NetBSD sound device name. I'll try to figure out what this is about and open another PR if needed.

Don't know about that one

* They also change the OSS sound device name. I don't know how portable this is (@sezero ?) but it might be why OSS can't initialize on my end. http://cvsweb.netbsd.org/bsdweb.cgi/pkgsrc/audio/xmp/patches/patch-src_sound_oss.c?rev=1.1&content-type=text/x-cvsweb-markup

I googled DEVOSSAUDIO and the only results I found define that macro in a makefile or something, so it doesn't make sense to me.

AliceLR commented 2 years ago

I did manage to find that DEVOSSAUDIO is defined by the pkgsrc Makefile for xmp, which in turn comes from a pkgsrc Makefile fragment that defines it as /dev/audio. DEVOSSAUDIO looks like a standard pkgsrc define so probably xmp can patch in /dev/audio when it's not defined for NetBSD/Solaris.

diff --git a/src/sound_oss.c b/src/sound_oss.c
index d38b7c75..22d29eb2 100644
--- a/src/sound_oss.c
+++ b/src/sound_oss.c
@@ -109,7 +109,16 @@ static void setaudio(int *rate, int *format)
 static int init(struct options *options)
 {
    char **parm = options->driver_parm;
-   static const char *dev_audio[] = { "/dev/dsp", "/dev/sound/dsp" };
+   static const char *dev_audio[] = {
+#ifdef DEVOSSAUDIO
+       DEVOSSAUDIO, /* pkgsrc */
+#endif
+#if defined(__NetBSD__) || defined(__sun)
+       "/dev/audio",
+#endif
+       "/dev/dsp",
+       "/dev/sound/dsp"
+   };
    audio_buf_info info;
    static char buf[80];
    int i;
AliceLR commented 2 years ago

Re: the device change to the NetBSD driver, it seems like their version might be preferable: https://man.netbsd.org/audio.4

When /dev/audio is opened, it automatically sets the track to manipulate
monaural 8-bit mu-law 8000Hz.  When /dev/sound is opened, it maintains
the audio format and pause/unpause state of the most recently opened
track.  In all other respects /dev/audio and /dev/sound are identical.

edit: the OSS, NetBSD, and Solaris drivers all look pretty much like copypaste of each other.

sezero commented 2 years ago

/dev/dsp is supposed to be OSS standart, so I think /dev/audio should come after all others.

Re: the device change to the NetBSD driver, it seems like their version might be preferable: https://man.netbsd.org/audio.4

Didn't know about that, though