To derive the bit depth, we can determine the number of bytes per frame (kChannels * 2), divide by the number of channels to get bytes per sample (2), and finally convert to bits by multiplying by 8 (16 bits).
Adopt Google C++ style guide naming:
m_hWaveOut → wave_out_handle
m_hSoundEvent → sound_event_handle
m_aBuffers → buffers
m_iSampleRate → sample_rate
m_bShutdown → shutdown
m_iLastCursorPos → last_cursor_pos
b_InitSuccess → init_success
NUM_CHUNKS → num_blocks
BUFFERSIZE_FRAMES → num_frames_per_block
CHUNKSIZE → block_size
I've also fixed spacing in the constructor and reorganized the list of member variables in the header for easier reading, and added context to the comment in the header file, changed a static const int to constexpr int, etc
s
With WaveOut, typically one defines the following values before starting a WaveOut thread: sample rate, number of blocks, and size of each block in frames. Based on these values, a particular latency will be calculated. StepMania historically follows this approach.
However, it is possible to target a specific latency instead, and extrapolate the needed number of blocks if the sample rate and block size values are known.
This PR refactors the driver to calculate the ideal number of blocks based on a pre-determined optimal target latency and block size. This allows us to guarantee a nearly-imperceptible difference in WaveOut latency (0.29 ms) between 44.1 kHz and 48 kHz.
Nothing related to driver functionality is changed. We just change the way the buffer parameters are determined. This PR only changes code which runs before the WaveOut thread is launched.
When we consider how the WaveOut API works, we need to provide three of four values and the fourth is calculated. So, at 44100 Hz, with 15 blocks of 256 frames each, 15 * 256 gives us 3840 frames of latency (3840 / 44100 = ~87 ms). If we use this same configuration with 48000 Hz, we get a latency of 80ms which is a significant difference.
However, if we increase to 16 blocks at 48000 Hz, we get 4096 frames of latency (~85 ms), which is much closer to the 87 ms when using 15 blocks at 44100 Hz.
A target latency of 118 ms allows for the smallest possible delta when using a block size of 512 frames, and testing with sample rates of 44100 and 48000. At 44100 Hz / 512 frames / 118 ms target latency, we have a calculated latency of 127.7098 ms. At 48000 Hz / 512 frames/ 118 ms target latency, we have a calculated latency of 128 ms - a 0.29 ms difference.
I determined this by simulating the CalculteNumBufferChunks calculation for a range of 100 to 300.
This was actually already considered in the surrounding code. I just had to uncomment it. This recommendation has remained consistent in the Microsoft documentation since this code was written. It's safe to uncomment it to ensure our main buffer stays active, since shutdown of the mixer engine is unwanted.
/MP2 limits the build processes to a maximum of 2. Many CPU's nowadays have more than two cores, so the integer specifier is removed so that the build is not artificially constrained to two cores.
/FS allows multiple compiler processes to write to the .pdb file; it should be used in conjunction with /MP to speed up the build.
/permissive- disables certain MSVC-specific extensions and follows the C standard more closely. Using this flag should help to ensure behavior matches gcc & clang a little more closely.
Note, /utf-8 does NOT build the program with Unicode support. It merely indicates to the compiler that the source files are using UTF-8 encoding.
The existing code uses repeated error messages at different points in the code, making debugging very difficult. This makes every step more unique for easier state tracking. Also changes trace to warn, to make it a little easier to spot failed screenshots in the log.
The existing code looks like it's attempting to call another constructor, and seems to be creating a temporary object, which I don't think is the intention of the code.
Currently the input mapping screen asks for an updated list of button names every single frame. As this is a bit excessive, it's changed to run every 120ish frames.
Thanks to evbo from the ITC discord for pointing out this improvement. It does take bring the thread polling interval down from every 10ms to every 1ms.
All logic is moved into one section now instead of being split between two places.
This is a minor change, but I'd be more comfortable with this because it more explicitly defines the different possibilities for group/machine offset combinations than the existing code.
Revert "Reorganize machine sync bias handing in Group constructor"
This reverts commit b53d26df494aea9d7491d2410c214b146ab09034.
Reorganize machine sync bias handing in Group constructor
Currently, video banners are treated the same as still-image banners, where we assume we need to let the low-res banner fade in before swapping it out with the high-res banner. This isn't the correct way to handle video banners, since we don't make low-res placeholders for them, so we now check the banner's extension to see if it's a video or not, and directly load the video if so.
Remove this method, which only executes on Windows, which overwrites a few of your preferences based on whether or not you have at least 192MB of free RAM. The entire body of the function is wrapped in _WIN32 ifdefs. It is the only function making use of the LastSeenMemory value stored in Preferences.ini.
The LOG->Warn line is being executed unconditionally due to the the lack of braces. The indentation is misleading as a result. This adds braces to ensure the intended behavior is performed.
With our new libjpeg_turbo library, a quality of 150 doesn't seem to be valid, so I'm changing this to 95. I chose 95 over 100 because the libjpeg_turbo project uses a quality of 95 for all testing and benchmarking shown in their documentation, and notes that a quality of 95 "has been determined to be perceptually lossless under most viewing conditions".
Removing the case where `StepMania::SaveScreenshot` may request to save a low quality jpeg. The file size difference between a quality 70 and quality 95 jpeg is very small by modern storage standards.
In the song wheel, if you scroll back and forth over an MP3 song very fast, you might get some audible corruption at the beginning of the song preview, and it might even crash the game.
This fixes both of those problems.
Resolves#610.
`RageSoundReader_MP3` has three different methods of setting the position. The default choice of `SetPosition_toc` is unable to provide an accurate result. The reason Edit Mode has such a severe desync when working with MP3's is due to the preference of using `SetPosition_toc`.
We can prevent this by using `SetPosition_hard` which works very well. Judging by comments in the file, it seems like `SetPosition_hard` was too resource intensive to use as the default method, 20+ years ago.
The only downside is, if you start from an arbitrary position (which is **only** possible in Edit Mode), you have a few milliseconds of corrupted audio, but then the sync is perfect. You can hear this in the demonstration video below.
When we start from the beginning of a file, such as playing a song, or using `Play whole song` in edit mode, `MADLIB_rewind` will be used.
`SetPosition_hard` provides an accurate result, and is consistent with the offset provided by `MADLIB_rewind`. The result is a consistent sync experience whether the MP3 is started from the beginning, or from an arbitrary position midway thru the file.
The `m_bAccurateSync` boolean dependency has been removed from SetPosition since it does not seem to provide any benefit on modern hardware. It seems fine to remove it so that we can call `MADLIB_rewind` or `SetPosition_hard` as fast as possible.
In my testing I never got `SetPosition_estimate` to be used, but I've left it in as a backup option.
This is the fixed Merge from the 2025-1-26 test build. It will more strictly check the drift with each iteration of the m_aSounds loop, whereas the original implementation can allow small amounts of desync to accumulate over time if none of them exceed the threshold.
I've fixed the issues with the DirectSound driver which a number of commits from 2016 attempted to address. Additionally, build testers strongly prefer the fixed DirectSound to WaveOut (the current default driver for Windows) in terms of both game stability and sync stability, so I'm making it the default driver as well.
StepMania commit 75a9532 from 2006 had a subtle error, the declaration of len and locked_buf will hide the previous local declaration of these variables. The issues causing the DirectSound driver to be unreliable in terms of sync were due to this. This PR renames the variables from the 2006 commit to prevent this, and also ensures all variables are properly initialized.
Renamed `locked_buf` and `len` in `MixerThread` to `locked_buf_init` and `len_init` to avoid shadowing.
Prevent use of uninitialized member variables by initializing `m_pPCM`, `m_iSampleRate`, and `m_bShutdownMixerThread` in the constructor.
Preferred in Windows, especially when we can't guarantee that we are able to perform a proper clean-up before terminating. Halt may be called with the Kill flag which indicates everything must shut down. This is the preferred approach in the Windows API.
Another preferred option is to use WaitForSingleObject with a timeout, but Halt with the Kill flag is being called when the game is about to crash, so that's not a realistic option and may cause the program to hang indefinitely.
This file handles OS side interaction as well as the creation of threads within the game - std::unique_ptr and std::mutex are introduced here for their safety and memory management benefits.
A custom wrapper PortableSignalObjectAndWait was removed in favor of directly calling SignalObjectAndWait.
In SetThreadName, we change from EXCEPTION_CONTINUE_EXECUTION to EXCEPTION_EXECUTE_HANDLER as suggested by the compiler to avoid a potential infinite loop.
ThreadImpl_Win32::Halt has been refactored to unequivocably call SuspendThread instead of TerminateThread, as TerminateThread doesn't allow for proper thread cleanup.
Don't bother with the try-catch in SetThreadName unless this is a debug build (since exceptions are disabled in ITGm, so it won't work as expected).
DebugInfoHunt was also updated since GetFileInformation directly uses it.
- GetMemoryDebugInfo is updated as GlobalMemoryStatus is deprecated.
- GetWindowsVersionDebugInfo is updated to use RtlGetVersion.
- GetFileInformation updated to use vector and string, as well as const correctness and modern Windows methods. Mostly removing Win9x-XP compatibility code.
- Also gets rid of some workarounds aimed at XP era versions of Visual Studio.
Fix a mistake in Windows version detection logic
Bringing in algorithm for std::copy, and vector for automatic memory management.
Initializing the buffer with zeroes in the same operation as resizing the buffer. Going back to using an unsigned int for realsize, to prevent the buffer from becoming an unreasonably large size.
The constructor and destructor can be empty since the vector<float>'s own destructor will take care of freeing allocated memory.
The static_cast to an int in read(int16_t *pBuf) is changed to a static_cast to an int16_t, as audio data is expressed in 16 bit integers, and the fractional part is inherently rounded towards zero by truncation.
Speed up the loading window. SetPixelV is slow. No perceptible difference in loading window behavior, but is faster. The goal here is to alleviate the case where the loading window is the bottleneck when loading songs on a fast computer.
Replacing the SM5 power-of-two code with the implementation from Bit Twiddling Hacks, since the current implementation isn't correctly handling the edge case where the input is 0, and is a little overcomplicated.
Bring in std::optional<DEVMODE> to gracefully handle the case where the function may not return anything valid. This helps to avoid undefined behavior caused by unexpected/invalid return values, as an extra protection to supplement the check if we can identify the refresh rate.
This eluded the update to use `_WIN32` preprocessor macro across the codebase.
Also adding an error message so we don't fail silently if we can't get the command line arguments.
The DirectInput / DirectSound version defs need to be exposed to other parts of the engine, so they are moved to the header instead of the cpp file.
The includes can be consolidated after that.
I'm also replacing the usage of a raw array of char *'s with an RString and using c_str() to get its value - a small change, but raw arrays are unreliable.
Directly use CommandLineToArgvW, replacing legacy code. Support Unicode natively by natively using std::string and wchar. shellapi.h is brought in for CommandLineToArgvW
Mitigate some security concerns, update API calls (RegOpenKeyExW & ShellExecuteEx), support unicode natively, const correctness, etc. - not doing anything new, just updating what's here to be more modern.
Floating point time is being stored as a float here, we can prevent precision loss by changing some of these to double during the lifetime of the function to get a more accurate result.
Floating point time is being stored as a float here, we can prevent precision loss by changing some of these to double during the lifetime of the function to get a more accurate result.
Since the EPSILON variable in the header file was declared but not assigned any value, it could lead to undefined behavior or calculation errors when the variable is used without initialization. This ensures a consistent value is always used and is also defined at compile time for further safety.
This prevents a crash that occurs when trying to exit the game while a connection attempt is being made (such as closing the game while it's in the middle of trying to connect to GrooveStats).
Following the discovery of the underlying bugs in BitmapText causing issues with performance of Step Statistics, I wanted to revisit some changes I made to ActorMultiVertex in ae6d7cecb5. Some things were reverted to prefer original behavior.
Ideally this would also have an option to allow AutoSync, AutoPlay, and Assist Tick with the debug overlay disabled, but that would require migrating that functionality elsewhere.
Minor cleanup on `GetMainAndSubTitlesFromFullTitle` to use a range-based for loop, and also is slightly improved so that separators of any length will work, whereas the original code only works when the separator is one character long.
sukibaby