-
Notifications
You must be signed in to change notification settings - Fork 632
Set Sample Rate to 32000 hz #4780
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
Conversation
soh/soh/OTRGlobals.cpp
Outdated
| #if 0 | ||
| // Values for 44100 hz | ||
| #define SAMPLES_HIGH 752 | ||
| #define SAMPLES_LOW 720 | ||
| #else | ||
| // Values for 32000 hz | ||
| #define SAMPLES_HIGH 560 | ||
| #define SAMPLES_LOW 528 | ||
|
|
||
| #endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of this can we just change
//AudioMgr_ThreadEntry(&gAudioMgr);
// 528 and 544 relate to 60 fps at 32 kHz 32000/60 = 533.333..
// in an ideal world, one third of the calls should use num_samples=544 and two thirds num_samples=528
//#define SAMPLES_HIGH 560
//#define SAMPLES_LOW 528
// PAL values
//#define SAMPLES_HIGH 656
//#define SAMPLES_LOW 624
// 44KHZ values
#if 0
// Values for 44100 hz
#define SAMPLES_HIGH 752
#define SAMPLES_LOW 720
#else
// Values for 32000 hz
#define SAMPLES_HIGH 560
#define SAMPLES_LOW 528
#endifto
//AudioMgr_ThreadEntry(&gAudioMgr);
// 528 and 544 relate to 60 fps at 32 kHz 32000/60 = 533.333..
// in an ideal world, one third of the calls should use num_samples=544 and two thirds num_samples=528
#define SAMPLES_HIGH 560
#define SAMPLES_LOW 528There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you willing to lose values for 44100? You don't rather have this for quick change and testing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After the experience we've had so far with 44k, I doubt we'll go back, but if we do, we can look for this PR to see the differences.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i agree with @Malkierian. if someone wants to try to get 44.1k working again they can check the git blame and find them
soh/soh/OTRGlobals.cpp
Outdated
| overlay->SetCurrentFont(CVarGetString(CVAR_GAME_OVERLAY_FONT, "Press Start 2P")); | ||
|
|
||
| context->InitAudio({ .SampleRate = 44100, .SampleLength = 1024, .DesiredBuffered = 2480 }); | ||
| context->InitAudio({ .SampleRate = 32000, .SampleLength = 1024, .DesiredBuffered = 2480 }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like DesiredBuffered was changed for 44.1k here
int SDLAudioPlayer::GetDesiredBuffered(void) {
- return 1680;
+ return 2480;
}interestingly, it seems SampleLength was already 1024 before the switch to 44.1k
i also am not sure where these values are coming from:
32000/1680 = 19.047619048
44100/2480 = 17.782258065
@Kenix3 @NEstelami any ideas?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't find that. Where is it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shipwright/libultraship/libultraship/SDLAudioPlayer.cpp
Lines 31 to 33 in d24c845
| int SDLAudioPlayer::GetDesiredBuffered(void) { | |
| return 1680; | |
| } |
vs
Shipwright/libultraship/libultraship/SDLAudioPlayer.cpp
Lines 31 to 33 in 1f1b81a
| int SDLAudioPlayer::GetDesiredBuffered(void) { | |
| return 2480; | |
| } |
and
Shipwright/libultraship/libultraship/WasapiAudioPlayer.cpp
Lines 83 to 85 in 09432ee
| int WasapiAudioPlayer::GetDesiredBuffered(void) { | |
| return 1680; | |
| } |
vs
Shipwright/libultraship/libultraship/WasapiAudioPlayer.cpp
Lines 83 to 85 in 1f1b81a
| int WasapiAudioPlayer::GetDesiredBuffered(void) { | |
| return 2480; | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, did some digging and figured out stuff about the value for samples
it looks like in SDL3 samples has been removed from AudioSpec
https://wiki.libsdl.org/SDL3/README/migration#sdl_audioh
https://wiki.libsdl.org/SDL2/SDL_AudioSpec
https://wiki.libsdl.org/SDL3/SDL_AudioSpec
from the migration guide:
SDL_AudioSpec has been reduced; now it only holds format, channel, and sample rate. SDL_GetSilenceValueForFormat() can provide the information from the SDL_AudioSpec's removed silence field. SDL3 now manages the removed samples field; apps that want more control over device latency and throughput can force a newly-opened device's sample count with the SDL_HINT_AUDIO_DEVICE_SAMPLE_FRAMES hint, but most apps should not risk messing with the defaults. The other SDL2 SDL_AudioSpec fields aren't relevant anymore.
sdl3 logic for getting the value is here https://github.com/libsdl-org/SDL/blob/7c9f6c631304829cb5d6fc68d873dace47c17152/src/audio/SDL_audio.c#L147-L166
if (freq <= 22050) {
return 512;
} else if (freq <= 48000) {
return 1024;
} else if (freq <= 96000) {
return 2048;
} else {
return 4096;
}so based on what SDL is doing it seems 1024 is a reasonable value for both 32k and 44.1k
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| context->InitAudio({ .SampleRate = 32000, .SampleLength = 1024, .DesiredBuffered = 2480 }); | |
| context->InitAudio({ .SampleRate = 32000, .SampleLength = 1024, .DesiredBuffered = 1680 }); |
see
https://github.com/HarbourMasters/Shipwright/pull/4780/files?diff=unified&w=0#r1900436401
https://github.com/HarbourMasters/Shipwright/pull/4780/files?diff=unified&w=0#r1900475059
soh/soh/OTRGlobals.cpp
Outdated
| overlay->SetCurrentFont(CVarGetString(CVAR_GAME_OVERLAY_FONT, "Press Start 2P")); | ||
|
|
||
| context->InitAudio({ .SampleRate = 44100, .SampleLength = 1024, .DesiredBuffered = 2480 }); | ||
| context->InitAudio({ .SampleRate = 32000, .SampleLength = 1024, .DesiredBuffered = 2480 }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| context->InitAudio({ .SampleRate = 32000, .SampleLength = 1024, .DesiredBuffered = 2480 }); | |
| context->InitAudio({ .SampleRate = 32000, .SampleLength = 1024, .DesiredBuffered = 1680 }); |
see
https://github.com/HarbourMasters/Shipwright/pull/4780/files?diff=unified&w=0#r1900436401
https://github.com/HarbourMasters/Shipwright/pull/4780/files?diff=unified&w=0#r1900475059
This reverts commit 9455579.
…asters#5234) This reverts commit feea299. Also applies rupee screech fix LL provided to 2ship.
…asters#5234) (HarbourMasters#5508) This reverts commit feea299. Also applies rupee screech fix LL provided to 2ship.
This change fixes Vibrato and Portamento issues with the port.
Build Artifacts