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

Player enhancements #223

Merged
merged 52 commits into from
Mar 31, 2024
Merged

Player enhancements #223

merged 52 commits into from
Mar 31, 2024

Conversation

winterheart
Copy link
Contributor

Huge refactor for wildmidi player intended to enable more than one playback outputs.
Now player supports -P option which controls actual output (default value depends on platform capabilities, but OpenAL output is preferred when available). Each output can be enabled or disabled on configuration and compile time. Currently there 9 outputs:

  • noout - No out, permanently enabled; just stub functions, needed for print error messages when user chooses unavailable output.
  • wave - Output to Wave file, permanently enabled.
  • openal - OpenAL output, controlled via -DWANT_OPENAL. Preferred default output if available.
  • alsa - ALSA output, controlled via -DWANT_ALSA.
  • oss - OSS output, controlled via -DWANT_OSS
  • ahi - Amiga AHI output, controlled via -DWANT_AMIGA_AHI
  • win32mm - Windows MM output, controlled via -DWANT_WIN32_MM
  • os2dart - OS/2 DART output, controlled via -DWANT_OS2DART
  • dossb - DOS SoundBlaster output, controlled via -DWANT_DOSSB

Player now can be easily extended for additional outputs and ported to another platforms, since there modular "framework" has been implemented.

open_*_output functions now accept path as output where to save file or send stream to sound device and can be defined by -o <out> option. This parameter currently required by wave, alsa, oss outputs.

Additional fixes to build system

Besides changes related to player, there also some fixes in CMake build system. Now CMake can properly generate project for multiconfiguration systems like VS MSBuild. If such system is detected, you can build any of supported build types with cmake --build <dir> --config <build_type>.

Additionally Github Actions now stores build artifacts available on Actions page (like here https://github.com/winterheart/wildmidi/actions/runs/376312898).

@raziel-
Copy link
Contributor

raziel- commented Nov 21, 2020

Awesome

@psi29a
Copy link
Member

psi29a commented Nov 23, 2020

Very cool, thoughts @sezero ?

@sezero
Copy link
Contributor

sezero commented Nov 23, 2020

Very cool, thoughts @sezero ?

I don't know. One thing that comes to my mind is why make OpenAL the default?? Don't like that one for sure. (OpenAL output was added with the sole purpose of supporting macOS the easy way. It isn't available everywhere by default.) As for the rest, needs review and testing.

@sezero
Copy link
Contributor

sezero commented Nov 23, 2020

Well, I locally did wget https://github.com/Mindwerks/wildmidi/pull/223.patch && git am 223.patch
and then did mkdir b && cd b && cmake -DCMAKE_BUILD_TYPE=Release -DWANT_STATIC=1 ..:

-- The C compiler identification is GNU 4.4.7
-- Check for working C compiler: /usr/bin/cc
-- Check for working C compiler: /usr/bin/cc -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Detecting C compile features
-- Detecting C compile features - done
-- Build Type: Release
-- Performing Test HAVE_VISIBILITY_DEFAULT
-- Performing Test HAVE_VISIBILITY_DEFAULT - Success
-- Performing Test HAVE_VISIBILITY_HIDDEN
-- Performing Test HAVE_VISIBILITY_HIDDEN - Success
-- Performing Test HAVE___BUILTIN_EXPECT
-- Performing Test HAVE___BUILTIN_EXPECT - Success
-- Performing Test HAVE_C_INLINE
-- Performing Test HAVE_C_INLINE - Success
-- Performing Test HAVE_C___INLINE__
-- Performing Test HAVE_C___INLINE__ - Success
-- Performing Test HAVE_C___INLINE
-- Performing Test HAVE_C___INLINE - Success
-- Performing Test HAVE_NO_UNDEFINED
-- Performing Test HAVE_NO_UNDEFINED - Success
-- Looking for stdint.h
-- Looking for stdint.h - found
-- Looking for inttypes.h
-- Looking for inttypes.h - found
-- Check if the system is big endian
-- Searching 16 bit integer
-- Looking for sys/types.h
-- Looking for sys/types.h - found
-- Looking for stddef.h
-- Looking for stddef.h - found
-- Check size of unsigned short
-- Check size of unsigned short - done
-- Using unsigned short
-- Check if the system is big endian - little endian
-- M_LIBRARY: /usr/lib/libm.so
-- Configuring done
-- Generating done

The resulting config.h has this:

/* Define our audio drivers */
/* #undef HAVE_LINUX_SOUNDCARD_H */
/* #undef HAVE_SYS_SOUNDCARD_H */
/* #undef HAVE_MACHINE_SOUNDCARD_H */
/* #undef HAVE_SOUNDCARD_H */

#define AUDIODRV_NONE 1
#define AUDIODRV_WAVE 1
#define AUDIODRV_ALSA 0
#define AUDIODRV_OSS 0
#define AUDIODRV_OPENAL 0
#define AUDIODRV_AHI 0
#define AUDIODRV_WIN32_MM 0
#define AUDIODRV_OS2DART 0
#define AUDIODRV_DOSSB 0

Obviously not acceptable as is.

Haven't done any further testing, nor have I compared the moved code to the
original ones.

@winterheart
Copy link
Contributor Author

@sezero main idea is give to user choice what he like to enable. If user desired to enable OpenAL, ALSA and OSS, new functionality gives him that opportunity. If he decided to disable all - he'll get what he want to: only very basic functionality with one 'true' output which will work on all platforms - wave.

Build system changed for that, so you can enable or disable any variation of outputs that theoretically support target platform.
So, for Linux system build options will be cmake -DCMAKE_BUILD_TYPE=Release -DWANT_STATIC=1 -DWANT_ALSA=ON -DWANT_OSS=ON -DWANT_OPENAL=ON .. (if you want all of them - and this is sane configuration, you can choose any supported playback with -P <playback_output> option).

OpenAL chosen as default output with one reason - it runs practically everywhere. If you have OpenAL support on target, you'll don't have to think about platform-specific output and build environment. So this ease to port player/library into new platform. Like I sad, if you have POSIX system with available OpenAL on it, you'll be OK, nothing to do for porting wildmidi player. But still old behavior still available - disable OpenAL and enable one of native output, and you'll receive same functionality as from current master. Player autodetect next available output and will use it for playback.

@winterheart
Copy link
Contributor Author

ping?

@sezero sezero added this to the 0.4.5 milestone Mar 21, 2021
@sezero sezero modified the milestones: 0.4.5, 0.4.6 Jan 8, 2023
@psi29a
Copy link
Member

psi29a commented Mar 27, 2024

@winterheart if you're still around, would be nice to rebase this.

I think the main point of contention was the 'default'. We can still have that discussion if you want.

Keep in mind that openal is deprecated on macos and will eventually be removed. So we'll defo need a replacement, though that is likely not for this PR.

winterheart and others added 19 commits March 27, 2024 20:28
Player should supports all available outputs besides noout and waveout.
Extended wildmidi_info struct with core functions (open, write, close, pause, resume), which should be implemented for each supported driver. If there no need on certain function there may be used one from noout.
There also may need to fix other architectures.
Now all playbacks are equals.
Simplified project, reworked player build options. Now multiple playback outputs can be coexists same time.
By default preferred output is OpenAL (if available).
Implementing artifacts upload for each pipeline
Multiconfiguration generators (such as VS MSBuild) relies on CMAKE_CONFIGURATION_TYPES instead of CMAKE_BUILD_TYPE. To build one of supported build types you need to issue `cmake --build <directory> --config <build_type>`.
@sezero sezero force-pushed the player-refactor branch 2 times, most recently from 2528e4a to 78d602a Compare March 30, 2024 13:40
cmake >= 3.4 is now needed because of string(APPEND ...)
@sezero
Copy link
Contributor

sezero commented Mar 31, 2024

I think I'm mostly done with this. @psi29a, @winterheart: Take a look (and test if possible) one last time.

I still want to add a native backend for netbsd, but that can be done after this gets in.

@psi29a
Copy link
Member

psi29a commented Mar 31, 2024

Yeah, native backends needed for:

  • macos
  • netbsd
  • more...

We can put that on our issue tracker

@sezero
Copy link
Contributor

sezero commented Mar 31, 2024

Yeah, native backends needed for:

macos

That's already done: coreaudio is added in this P/R

netbsd

Working on it, can possibly do it today

more...

For e.g. ?

@psi29a
Copy link
Member

psi29a commented Mar 31, 2024

Oh wow, reads notes

@psi29a
Copy link
Member

psi29a commented Mar 31, 2024

That means we can make use of github actions to make macos builds as well... perhaps in another PR?

We can have a rule to make release builds when tagged.

@sezero
Copy link
Contributor

sezero commented Mar 31, 2024

That means we can make use of github actions to make macos builds as well... perhaps in another PR?

Current github actions generates artifacts, thanks to @winterheart
Don't know specifically about macos, though

We can have a rule to make release builds when tagged.

For another PR.

Merging this now.

@sezero sezero merged commit 5d39997 into Mindwerks:master Mar 31, 2024
8 checks passed
@sezero
Copy link
Contributor

sezero commented Mar 31, 2024

@psi29a, @winterheart: With alsa or oss configured, --device=default or --device=/dev/dsp works, but -d default or -d /dev/dsp does not: Fails with ./wildmidi: invalid option -- 'd'

It used to work before (e.g. in 0.4.5) -- what am I doing wrong? (It's probably something obvious, but I can't see it for now..)

@sezero
Copy link
Contributor

sezero commented Mar 31, 2024

@psi29a, @winterheart: With alsa or oss configured, --device=default or --device=/dev/dsp works, but -d default or -d /dev/dsp does not: Fails with ./wildmidi: invalid option -- 'd'

It used to work before (e.g. in 0.4.5) -- what am I doing wrong? (It's probably something obvious, but I can't see it for now..)

Created this as a new ticket #250 so that it doesn't get lost. Please respond / discuss at there.

@psi29a
Copy link
Member

psi29a commented Apr 1, 2024

Ah, was going to validate on my mac, but alright. If I come up with anything, will create an issue for it. Cheers everyone!

@sezero
Copy link
Contributor

sezero commented Apr 1, 2024

Ah, was going to validate on my mac, but alright. If I come up with anything, will create an issue for it.

Yes please

@sezero
Copy link
Contributor

sezero commented Apr 1, 2024

Just added a native netbsd backend to master

@sezero
Copy link
Contributor

sezero commented Apr 1, 2024

Ah, was going to validate on my mac, but alright. If I come up with anything, will create an issue for it.

Yes please

And also check the newly added sndio (on openbsd) and netbsd backends if you are able.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants