[CryoTheRenegade]

Follow-up to #3658, from xezon's "Are there more of these?" on #3580. MilesAudioManager and MPSCIntrusiveQueue pass their storage to the Interlocked* functions through reinterpret_cast<volatile long*> and reinterpret_cast<void* volatile*>. A PlayingStatus is not a long and a T* is not a void*, so those accesses are undefined behavior. Since #3325, Utility/atomic_adapter.h provides std::atomic for every compiler, VC6 included, so the casts are no longer needed.

• PlayingAudio::m_status is now a std::atomic<PlayingStatus>. The three places that advance the status call a new file-static advancePlayingStatus, which wraps compare_exchange_strong and returns the previous status like InterlockedCompareExchange did:

  -	InterlockedCompareExchange(reinterpret_cast<volatile long*>(&playing->m_status), PS_Stopping, PS_Playing);
  +	advancePlayingStatus(playing, PS_Playing, PS_Stopping);

  The static_assert that m_status has the size of a long is removed, since nothing casts it to one any more.

• MPSCIntrusiveQueue::m_head is now a std::atomic<T*>. Push uses compare_exchange_weak and Flush uses exchange, in place of InterlockedCompareExchangePointer and InterlockedExchangePointer.

The memory ordering stays sequentially consistent, as with the Interlocked* functions.

Evidence

• Disassembly: I compared MilesAudioManager.obj and W3DScreenshot.obj (the only user of MPSCIntrusiveQueue) for both games against a build of main.

  • VS 2026 (ClangCL) Release: the code is the same apart from layout. MilesAudioManager::pauseAudio merges two copies of its loop into one, and calls getAudioEventInfo in the same places. screenshotThreadFunc changes only in register use. The dynamic initializer of the screenshot queue is gone, because std::atomic<T*> is zero-initialized at compile time.

  • VC6 Release: the adapter's member functions are not inlined.

    • Each read of m_status, such as m_status == PS_Playing, now calls atomic::load_stored, which calls the adapter's ReadAcquire, a plain mov. On main it was one inline load.
    • Where stopPlayingAudio is inlined into its callers, such as processPlayingList and stopAudio, a status change now calls atomic::compare_exchange, which calls InterlockedCompareExchange. On main it called InterlockedCompareExchange directly. In stopPlayingAudio, notifyOfAudioCompletion and startNextLoop, the call is the same as on main.
    • The queue's Push and Flush call the adapter's InterlockedCompareExchange and InterlockedExchange in place of the pointer versions.

    The extra calls are in the loops over the playing sounds, and add no locked instructions.

AI Disclosure

Claude Opus 5.5 was used to aid in implementation and to compare the disassembly.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The audio status field and MPSC queue head now use std::atomic. Audio status transitions and queue push and flush operations use atomic operations instead of Windows interlocked calls.

Changes

Audio Status Transitions

Layer / File(s)	Summary
Atomic audio status transitions Core/GameEngineDevice/Include/MilesAudioDevice/MilesAudioManager.h, Core/GameEngineDevice/Source/MilesAudioDevice/MilesAudioManager.cpp	PlayingAudio::m_status now uses std::atomic<PlayingStatus>. The stop, completion, and loop-delay paths use advancePlayingStatus for status transitions. The size assertion for m_status was removed.

MPSC Queue Atomics

Layer / File(s)	Summary
Atomic queue head operations Core/Libraries/Source/WWVegas/WWLib/mpsc_intrusive_queue.h	The queue head now uses std::atomic<T*>. Push retries a weak compare-exchange, and Flush atomically detaches the head.

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Refactor

Suggested labels: Audio, Refactor, Gen, ZH

Suggested reviewers: xezon, bobtista

Merge Risk: Minimal · up to 4dd1c

No actionable merge-blocking risk is established. The atomic replacements appear mergeable subject to normal supported-compiler builds and tests.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title names the main change: replacing reinterpret_cast-based Interlocked calls in MilesAudioManager with std::atomic to avoid undefined behavior. It matches the changeset, which converts PlayingA…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Description check		The description is directly related to the changeset. It explains that MilesAudioManager and MPSCIntrusiveQueue replace reinterpret_cast-based Interlocked* calls with std::atomic, names the affected m…

Full details: Title check

Explanation

The title names the main change: replacing reinterpret_cast-based Interlocked calls in MilesAudioManager with std::atomic to avoid undefined behavior. It matches the changeset, which converts PlayingAudio::m_status to std::atomic and adds advancePlayingStatus. The prefix 'fix(milesaudiomanager)' is a conventional-commit style scope and is acceptable. It is slightly long at 107 characters, but it is specific and conveys the primary change.

Full details: Description check

Explanation

The description is directly related to the changeset. It explains that MilesAudioManager and MPSCIntrusiveQueue replace reinterpret_cast-based Interlocked* calls with std::atomic, names the affected members (PlayingAudio::m_status and MPSCIntrusiveQueue::m_head), describes the new advancePlayingStatus helper and the compare_exchange and exchange calls, and reports disassembly comparisons for VS 2026 and VC6 builds.

Tip

🎯 Customize your risk assessments

Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.

✅ Automatic approvals, with your rules

Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 4422e7b4-7532-47d1-b867-9a075c37a067

Commits

Reviewing files that changed from the base of the PR and between b42b7d3 and 4dd1c01.

Files selected for processing: 3

• Core/GameEngineDevice/Include/MilesAudioDevice/MilesAudioManager.h
• Core/GameEngineDevice/Source/MilesAudioDevice/MilesAudioManager.cpp
• Core/Libraries/Source/WWVegas/WWLib/mpsc_intrusive_queue.h

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.