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

Refactored audio stack with PortAudio #497

Open
wants to merge 27 commits into
base: master
Choose a base branch
from

Conversation

donmor
Copy link

@donmor donmor commented Mar 4, 2023

Built with OpenAL, Ludo has audible audio latency (hundreds of milliseconds!), which is unbearable for retro gaming.
I managed to refactor it with PortAudio, which is as much portable as OpenAL, and has much lower latency.

@kivutar
Copy link
Member

kivutar commented Mar 7, 2023

Awesome, will try it asap...

@kivutar
Copy link
Member

kivutar commented Mar 7, 2023

Can you replace openal-soft by portaudio in the osx sections of ci.yml and cd.yml?

kivutar and others added 5 commits March 7, 2023 15:34
Windows part needs more tests
Needs more tests on windows
Correction
Correction
@kivutar
Copy link
Member

kivutar commented Mar 7, 2023

I think I know what's wrong with the test on OSX. Not sure why it happens on this PR and not on others though.

Doing this in SetPixelFormat seems to workaround the bug. There is a division by 0 or of 0.

	// PixelStorei also needs to be updated whenever bpp changes
	defer func() {
		if video.pitch != 0 && video.bpp != 0 {
			gl.PixelStorei(gl.UNPACK_ROW_LENGTH, video.pitch/video.bpp)
		}
	}()

Edit: forget what I said, the test did fail on something portaudio related, in portaudio.(*Stream).Close(0x0) in audio.Reconfigure(0xac44) in audio.go:112

@kivutar
Copy link
Member

kivutar commented Mar 7, 2023

You can just call audio.Init() after Init(&video.Video{}) in Test_coreLoadGame

audio/audio.go Outdated
paStream.Close()
h, _ := portaudio.DefaultOutputDevice()
paStream, _ = portaudio.OpenStream(NewParameters(h), paCallback)
paStream.Start()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you can do some basic error management like

	if paStream != nil {
		if err := paStream.Close(); err != nil {
			log.Fatalln(err)
		}
	}
	h, err := portaudio.DefaultOutputDevice()
	if err != nil {
		log.Fatalln(err)
	}
	paStream, err = portaudio.OpenStream(NewParameters(h), paCallback)
	if err != nil {
		log.Fatalln(err)
	}
	if err = paStream.Start(); err != nil {
		log.Fatalln(err)
	}

@donmor
Copy link
Author

donmor commented Mar 7, 2023

BTW is there any package manager other than choco in windows env of github workflow?

@kivutar
Copy link
Member

kivutar commented Mar 7, 2023

There may be another one, I see portaudio on a package manager called vcpkg here PortAudio/portaudio#578

I tried your branch on OSX, I noticed 2 things:

  • the audio is crackling a little bit, but noticeably
  • the implementation doesn't use blocking audio, but it's usually good to block on audio in emulation, and portaudio documentation does mention a way of doing it

@donmor
Copy link
Author

donmor commented Mar 8, 2023

I think the crackling occurrs when the core sends data slower than sample rate, causing 0 filled in the interval.
Also, since the condition of cores is not expectable, using blocking may cause serious gliches like repeating or unsyncing, I guess.

@donmor
Copy link
Author

donmor commented Mar 8, 2023

Here' how mine work:
write() convert and put samples into paBuf and push paPtr ahead, and blocks the core if it goes too fast, preventing the not-played data in paBuf from being overwrited like in the classic snake game :)
paCallback() is called by portaudio, which get samples from paBuf and push paPlayPtr ahead, which always chases paPtr. if paPlayPtr is not smaller than paPtr, the playback stops by receiving zeros.

@donmor
Copy link
Author

donmor commented Mar 8, 2023

Hmm.. Hey, maybe we can just turn off blocking in write() when it is fast-forwarding, instead of returning size and letting it muted. And volume may be reduced when fast-forwarding.

donmor added 12 commits March 8, 2023 11:22
Changing build tools of portaudio to cmake
Change build tools of portaudio to cmake
Improvement
Add support to F-Fwd
stereo test
@donmor
Copy link
Author

donmor commented Mar 9, 2023

Weird enough... Only right channel is played even though PA stream opened with 2 channels. I'll go to stackoverflow <:-/

Finally I found the right way to output stereo
Stereo stuff
@donmor
Copy link
Author

donmor commented Mar 10, 2023

Okay finally it is stereo now :-)

minor fix
@donmor
Copy link
Author

donmor commented Mar 10, 2023

BTW I want to know if the patch will pass ci/cd now :-)

@donmor
Copy link
Author

donmor commented Mar 13, 2023

It seems that github workflow env has no audio device, causing the failure 'cause portaudio works with audio hardware. We have to do something to the tests <:-/

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.

None yet

2 participants