libretro / RetroArch

Cross-platform, sophisticated frontend for the libretro API. Licensed GPLv3.
http://www.libretro.com
GNU General Public License v3.0
10.4k stars 1.84k forks source link

Audio timing.sample_rate =audio_out_rate glitch #71

Closed p1pkin closed 12 years ago

p1pkin commented 12 years ago

Initially I've tryed to use higher (48000) samplerate in GenPlusGX libretro port, and got very buggy sound - depending on audio_driver and audio_rate_control settings it can be crackles/pops, or no sound at all, or 1/2 framerate drop. After some experiments I've found glich is happens then timing.sample_rate in libretro port and audio_out_rate in retroarch.cfg is equal, so even with "clean git" GenPlusGX compile I've got this bug with audio_out_rate = "44100" I've also checked FCEUX port and got the same bug with audio_out_rate = "32050" So this is definitely RetroArch core bug, and not ports-related.

also this https://github.com/ekeeke/Genesis-Plus-GX/issues/2 is probably a result of same issue. changing timing.sample_rate to 44100 (instead of retroarch-default 48000) kind of fixed it.

Steps to reproduce:

  1. set audio_out_rate to libretro-port samplerate (44100 for GenPlus, 32050 for FCEUX, etc)
  2. run it
p1pkin commented 12 years ago

ps: all that was tested on win32

Themaister commented 12 years ago

A related bug was fixed in commit: 34713f40d1a611222f682bf278f5cd9d587638a6. Make sure you're running latest versions.

p1pkin commented 12 years ago

I am sure, tested on current git mingw compile and on beta-2

p1pkin commented 12 years ago

Themaister: https://github.com/Themaister/RetroArch/commit/34713f40d1a611222f682bf278f5cd9d587638a6 fixes bug in Sinc-resampler, but win32 x86 build use Hermite (only x86_64 have SINC=1 arg)

I've tested x86 build with SINC=1 and all works fine, so probably bug in that hermite-resampler.

p1pkin commented 12 years ago

btw, why not use SINC in 32bit build ? imo it gives better result

Themaister commented 12 years ago

Sinc is a bit slower than Hermite due to quality, and when a user uses 32-bit it is assumed that it is because the computer is fairly old (like XP 32-bit). I cannot make sure that a x86 computer has SSE (which sinc uses extensively to gain speed) and I don't want two builds. I don't want either to switch out implementations in runtime, which is not possible when using intrinsics (I have to enable SSE in compiler to have it work with intrinsics).

p1pkin commented 12 years ago

OK, it's your decision, but I'm not sure that there is at least one person using a PC with Pentium2 on Athlon(not XP) for emulation...

Themaister commented 12 years ago

If ditching support for older processors in the default builds is considered fine, I can build in SSE support and sinc ofc.

Themaister commented 12 years ago

Hermite should be fixed now as of latest commit.

p1pkin commented 12 years ago

No, its still broken, but now I hear sound with kind of distortion or high freq noise effect.

Themaister commented 12 years ago

Still broken in what way? I verified that crackling occured, and those were gone after the commit.

If downsampling is occuring with hermite it's not going to sound good anyways (aliasing), and hermite is crap anyways at high frequencies. Input rate is usually adjusted dynamically.

p1pkin commented 12 years ago

I mean it is still broken, but now in another way - I hear no crackling, but kind of high freq distortion or reverberation, it is best notable on games with high tone FM music or sfx

p1pkin commented 12 years ago

ps: with audio_rate_control = "true" this audio artifacts much less notable, but still present

Themaister commented 12 years ago

Yes, with hermite there will be a high frequency distortion when it's being resampled very near the Nyquist frequency, so it's not really avoidable unless expensive low-pass filtering takes place beforehand (which sidesteps the point of Hermite to begin with, speed), so I don't consider it a bug. Sinc will deal a lot better with this issue.

I've made Sinc default on x86 builds now. They will require SSE1.

p1pkin commented 12 years ago

thanks for explanation. this issue can be closed.