Merge pull request #7072 from mrfelek/fix/ngs2-system-lock-try-render

fix(libSceNgs2): let render skip a grain while another thread holds the system lock

core/libs/prx/libSceNgs2.native/src/Ngs2Internal.hpp

@@ -209,6 +209,7 @@ void Ngs2ClearReverb(Ngs2Voice& voice);
 bool Ngs2ProcessReverb(Ngs2Voice& voice, std::uint32_t grain, std::uint32_t sampleRate);
 void Ngs2ProcessLegacyUserFx(Ngs2Voice& voice, std::uint32_t grain, std::uint32_t sampleRate);
 void Ngs2RenderSystem(Ngs2System& system, const Ngs2RenderBufferInfo* bufferInfo, std::uint32_t numBufferInfo);
+void Ngs2RenderSilence(const Ngs2System& system, const Ngs2RenderBufferInfo* bufferInfo, std::uint32_t numBufferInfo);
 float Ngs2DefaultLevel(std::uint32_t sourceChannels, std::uint32_t source, std::uint32_t destChannels, std::uint32_t dest);
 int Ngs2PanInit(Ngs2PanWork* work, const float* speaker_angles, float unit_angle, uint32_t num_speakers);
 int Ngs2PanGetVolumeMatrix(Ngs2PanWork* work, const Ngs2PanParam* params, uint32_t num_params, uint32_t matrix_format, float* out_volume_matrix);

core/libs/prx/libSceNgs2.native/src/Render.cpp

@@ -298,7 +298,7 @@ static void WriteOutput(const Ngs2RenderBufferInfo& output, const std::vector<fl
     }
 }
 
-void Ngs2RenderSystem(Ngs2System& system, const Ngs2RenderBufferInfo* bufferInfo, std::uint32_t numBufferInfo) {
+static std::vector<std::size_t> OutputBytes(const Ngs2System& system, const Ngs2RenderBufferInfo* bufferInfo, std::uint32_t numBufferInfo) {
     const auto grain = system.option.num_grain_samples;
     std::vector<std::size_t> outputBytes(numBufferInfo);
     for (std::uint32_t i = 0; i < numBufferInfo; i++) {
@@ -310,6 +310,19 @@ void Ngs2RenderSystem(Ngs2System& system, const Ngs2RenderBufferInfo* bufferInfo
         outputBytes[i] = static_cast<std::size_t>(grain) * output.num_channels * SampleBytes(output.waveform_type);
         if (output.buffer_size < outputBytes[i]) throw std::invalid_argument("NGS2: render buffer " + std::to_string(i) + " is too small");
     }
+    return outputBytes;
+}
+
+void Ngs2RenderSilence(const Ngs2System& system, const Ngs2RenderBufferInfo* bufferInfo, std::uint32_t numBufferInfo) {
+    const auto outputBytes = OutputBytes(system, bufferInfo, numBufferInfo);
+    for (std::uint32_t i = 0; i < numBufferInfo; i++) {
+        if (outputBytes[i] != 0) std::memset(bufferInfo[i].buffer, 0, outputBytes[i]);
+    }
+}
+
+void Ngs2RenderSystem(Ngs2System& system, const Ngs2RenderBufferInfo* bufferInfo, std::uint32_t numBufferInfo) {
+    const auto grain = system.option.num_grain_samples;
+    const auto outputBytes = OutputBytes(system, bufferInfo, numBufferInfo);
     std::vector<Ngs2Voice*> voices;
     for (auto* rack : system.racks) {
         for (auto& voice : rack->voices) {

core/libs/prx/libSceNgs2.native/src/System.cpp

@@ -1,13 +1,16 @@
 #include <algorithm>
+#include <condition_variable>
 #include <cstddef>
 #include <cstdint>
 #include <cstdio>
 #include <cstring>
+#include <map>
 #include <memory>
 #include <mutex>
 #include <new>
 #include <stdexcept>
 #include <string>
+#include <thread>
 #include <vector>
 
 #include "prx/libc/include/General.hpp"
@@ -30,6 +33,34 @@ std::recursive_mutex& Ngs2Mutex() {
     return *mutex;
 }
 
+struct SystemLockState {
+    std::thread::id owner;
+    std::uint32_t depth = 0;
+};
+
+static std::mutex& SystemLockMutex() {
+    static auto* const mutex = new std::mutex();
+    return *mutex;
+}
+
+static std::condition_variable& SystemLockReleased() {
+    static auto* const released = new std::condition_variable();
+    return *released;
+}
+
+static std::map<Ngs2Handle, SystemLockState>& SystemLocks() {
+    static auto* const locks = new std::map<Ngs2Handle, SystemLockState>();
+    return *locks;
+}
+
+static void ReleaseSystemLock(SystemLockState& state) {
+    if (state.depth == 0) return;
+    if (--state.depth == 0) {
+        state.owner = {};
+        SystemLockReleased().notify_all();
+    }
+}
+
 Ngs2System* Ngs2FindSystem(Ngs2Handle handle) {
     auto& systems = Systems();
     const auto found = std::find(systems.begin(), systems.end(), reinterpret_cast<Ngs2System*>(handle));
@@ -110,6 +141,8 @@ static int CreateSystem(const Ngs2SystemOption& option, const Ngs2ContextBufferI
     system->uid = nextUid++;
     Systems().push_back(system);
     *handle = reinterpret_cast<Ngs2Handle>(system);
+    std::lock_guard lock(SystemLockMutex());
+    SystemLocks()[*handle] = {};
     return SCE_NGS2_OK;
 }
 
@@ -151,6 +184,11 @@ int APS5_VABI sceNgs2SystemDestroy(uintptr_t system_handle, Ngs2ContextBufferInf
         if (result != SCE_NGS2_OK) return result;
     }
     std::erase(Systems(), system);
+    {
+        std::lock_guard locks(SystemLockMutex());
+        SystemLocks().erase(system_handle);
+        SystemLockReleased().notify_all();
+    }
     const auto bufferInfo = system->bufferInfo;
     const auto allocator = system->allocator;
     std::destroy_at(system);
@@ -223,16 +261,25 @@ int APS5_VABI sceNgs2SystemGetUserData(uintptr_t system_handle, uintptr_t* user_
 }
 
 int APS5_VABI sceNgs2SystemLock(uintptr_t system_handle) {
-    Ngs2Mutex().lock();
-    if (Ngs2FindSystem(system_handle) != nullptr) return SCE_NGS2_OK;
-    Ngs2Mutex().unlock();
-    return SCE_NGS2_ERROR_INVALID_SYSTEM_HANDLE;
+    const auto self = std::this_thread::get_id();
+    std::unique_lock lock(SystemLockMutex());
+    auto& locks = SystemLocks();
+    SystemLockReleased().wait(lock, [&] {
+        const auto found = locks.find(system_handle);
+        return found == locks.end() || found->second.depth == 0 || found->second.owner == self;
+    });
+    const auto found = locks.find(system_handle);
+    if (found == locks.end()) return SCE_NGS2_ERROR_INVALID_SYSTEM_HANDLE;
+    found->second.owner = self;
+    found->second.depth++;
+    return SCE_NGS2_OK;
 }
 
 int APS5_VABI sceNgs2SystemUnlock(uintptr_t system_handle) {
-    std::lock_guard lock(Ngs2Mutex());
-    if (Ngs2FindSystem(system_handle) == nullptr) return SCE_NGS2_ERROR_INVALID_SYSTEM_HANDLE;
-    Ngs2Mutex().unlock();
+    std::lock_guard lock(SystemLockMutex());
+    const auto found = SystemLocks().find(system_handle);
+    if (found == SystemLocks().end()) return SCE_NGS2_ERROR_INVALID_SYSTEM_HANDLE;
+    ReleaseSystemLock(found->second);
     return SCE_NGS2_OK;
 }
 
@@ -241,6 +288,26 @@ int APS5_VABI sceNgs2SystemRender(uintptr_t system_handle, const Ngs2RenderBuffe
     auto* system = Ngs2FindSystem(system_handle);
     if (system == nullptr) return SCE_NGS2_ERROR_INVALID_SYSTEM_HANDLE;
     if (buffer_info == nullptr || num_buffer_info == 0) APS5_INVALID_ARG_EX;
+    SystemLockState* state = nullptr;
+    {
+        const auto self = std::this_thread::get_id();
+        std::lock_guard locks(SystemLockMutex());
+        auto& found = SystemLocks().at(system_handle);
+        if (found.depth != 0 && found.owner != self) {
+            Ngs2RenderSilence(*system, buffer_info, num_buffer_info);
+            return SCE_NGS2_OK;
+        }
+        found.owner = self;
+        found.depth++;
+        state = &found;
+    }
+    struct Release {
+        SystemLockState* state;
+        ~Release() {
+            std::lock_guard locks(SystemLockMutex());
+            ReleaseSystemLock(*state);
+        }
+    } release{state};
     Ngs2RenderSystem(*system, buffer_info, num_buffer_info);
     return SCE_NGS2_OK;
 }

core/libs/prx/libSceNgs2.native/tests/Ngs2Render.cpp

@@ -881,23 +881,57 @@ static void TestStereoIntoSurround() {
 
 static void TestLock() {
     const auto system = CreateSystem();
+    const auto voice = Voice(CreateRack(system, SCE_NGS2_RACK_ID_SAMPLER));
     Require(sceNgs2SystemLock(0x1234) == SCE_NGS2_ERROR_INVALID_SYSTEM_HANDLE);
     Require(sceNgs2SystemUnlock(0x1234) == SCE_NGS2_ERROR_INVALID_SYSTEM_HANDLE);
-    Ngs2SystemInfo info{};
-    std::thread free([&] { Require(sceNgs2SystemGetInfo(system, &info, sizeof(info)) == SCE_NGS2_OK); });
-    free.join();
+    Require(sceNgs2SystemUnlock(system) == SCE_NGS2_OK);
 
+    Ngs2SystemInfo info{};
     Require(sceNgs2SystemLock(system) == SCE_NGS2_OK);
+    Require(sceNgs2SystemLock(system) == SCE_NGS2_OK);
+    std::atomic<bool> otherDone = false;
+    std::thread other([&] {
+        Ngs2SystemInfo otherInfo{};
+        std::uint32_t flags = 0;
+        Require(sceNgs2SystemGetInfo(system, &otherInfo, sizeof(otherInfo)) == SCE_NGS2_OK);
+        Require(sceNgs2VoiceGetStateFlags(voice, &flags) == SCE_NGS2_OK);
+        std::vector<std::int16_t> out(Grain * 2, -1);
+        const Ngs2RenderBufferInfo buffer{out.data(), out.size() * sizeof(std::int16_t), SCE_NGS2_WAVEFORM_TYPE_PCM_I16L, 1};
+        Require(sceNgs2SystemRender(system, &buffer, 1) == SCE_NGS2_OK);
+        Require(std::all_of(out.begin(), out.begin() + Grain, [](std::int16_t sample) { return sample == 0; }));
+        Require(std::all_of(out.begin() + Grain, out.end(), [](std::int16_t sample) { return sample == -1; }));
+        Require(sceNgs2SystemGetInfo(system, &otherInfo, sizeof(otherInfo)) == SCE_NGS2_OK && otherInfo.render_count == 0);
+        otherDone = true;
+    });
+    for (int i = 0; i < 200 && !otherDone; i++) std::this_thread::sleep_for(std::chrono::milliseconds(10));
+    Require(otherDone);
+    other.join();
+    RenderI16(system);
+    Require(sceNgs2SystemGetInfo(system, &info, sizeof(info)) == SCE_NGS2_OK && info.render_count == 1);
+
     std::atomic<bool> done = false;
     std::thread blocked([&] {
-        Require(sceNgs2SystemGetInfo(system, &info, sizeof(info)) == SCE_NGS2_OK);
+        Require(sceNgs2SystemLock(system) == SCE_NGS2_OK);
         done = true;
+        RenderI16(system);
+        Require(sceNgs2SystemUnlock(system) == SCE_NGS2_OK);
     });
     std::this_thread::sleep_for(std::chrono::milliseconds(100));
     Require(!done);
     Require(sceNgs2SystemUnlock(system) == SCE_NGS2_OK);
+    std::this_thread::sleep_for(std::chrono::milliseconds(100));
+    Require(!done);
+    Require(sceNgs2SystemUnlock(system) == SCE_NGS2_OK);
     blocked.join();
     Require(done);
+    Require(sceNgs2SystemGetInfo(system, &info, sizeof(info)) == SCE_NGS2_OK && info.render_count == 2);
+
+    Require(sceNgs2SystemLock(system) == SCE_NGS2_OK);
+    std::thread unlocker([&] { Require(sceNgs2SystemUnlock(system) == SCE_NGS2_OK); });
+    unlocker.join();
+    std::thread renderer([&] { RenderI16(system); });
+    renderer.join();
+    Require(sceNgs2SystemGetInfo(system, &info, sizeof(info)) == SCE_NGS2_OK && info.render_count == 3);
     Require(sceNgs2SystemDestroy(system, nullptr) == SCE_NGS2_OK);
 }
 

docs/dev/debt/libSceNgs2.native.md

@@ -20,7 +20,7 @@
 - [NGS2 custom module ids](../../../core/libs/prx/libSceNgs2.native/src/Custom.cpp) (libSceNgs2.native) - a custom rack with a module id other than 0x13 (sampler rack), 0x1d and 0x1f (submixer rack) throws. Trails in the Sky 2nd Chapter (PPSA35169) creates a rack with module id 0x20 (`NGS2: custom module id 0x20 is not implemented`, #4246); which module it is, and its structure, are unknown.
 - [NGS2 reverb rack](../../../core/libs/prx/libSceNgs2.native/src/Reverb.cpp) (libSceNgs2.native, rack 0x2001) - follows the PS4 firmware 11.00 `libSceNgs2.sprx`, read and checked against its output at 11.025 to 48 kHz, including parameter changes during playback (differences at float rounding level); not yet compared with the PS5 library. Structure layouts come from [shadPS4](https://github.com/shadps4-emu/shadPS4)'s `ngs2_reverb.h` and the param ids 0x20010000 (setup) and 0x20010001 (I3DL2) from PPSA22520. Only the default `reverb_size` of 1 is handled, with its 14400-sample reflection buffer; other sizes and the library's half-rate mode throw. System sample rates below about 7.5 kHz, where an allpass delay would have no samples, throw. Setup only accepts equal input and output channel counts
 - [sceNgs2ReportRegisterHandler, sceNgs2ReportUnregisterHandler](../../../core/libs/prx/libSceNgs2.native/src/Unimplemented.cpp) (libSceNgs2.native, NIDs `uBIN24Tv2MI`, `nPzb7Ly-VjE`) are throwing stubs. The signatures (report type, handler, user data, out handle) and (report handle) follow [SharpProspero](https://github.com/SvenGDK/SharpProspero) and [shadPS4](https://github.com/shadps4-emu/shadPS4)'s `ngs2_report.h`. No report is produced yet, so registering a handler cannot be honoured. PPSA04540 imports both and registers a CPU load handler (type 5)
-- [sceNgs2SystemSetSampleRate, sceNgs2SystemSetUserData, sceNgs2SystemGetUserData, sceNgs2SystemLock, sceNgs2SystemUnlock](../../../core/libs/prx/libSceNgs2.native/src/System.cpp) (libSceNgs2.native) - shadPS4 only checks the system handle (`INVALID_SYSTEM_HANDLE`) and SharpProspero names what each call does; neither gives the other limits. A sample rate of 0 and a null user data pointer throw, any other rate is accepted, and changing the rate while a sampler filter is enabled throws because the filter coefficients were computed for the old rate. Lock and unlock take the same lock as `sceNgs2RackLock`
+- [sceNgs2SystemSetSampleRate, sceNgs2SystemSetUserData, sceNgs2SystemGetUserData, sceNgs2SystemLock, sceNgs2SystemUnlock](../../../core/libs/prx/libSceNgs2.native/src/System.cpp) (libSceNgs2.native) - shadPS4 only checks the system handle (`INVALID_SYSTEM_HANDLE`) and SharpProspero names what each call does; neither gives the other limits. A sample rate of 0 and a null user data pointer throw, any other rate is accepted, and changing the rate while a sampler filter is enabled throws because the filter coefficients were computed for the old rate. `sceNgs2SystemLock` and `sceNgs2SystemUnlock` follow [prosper's notes](https://github.com/mattias800/prosper/blob/d61eb2c0ad2de879ef74df9c4aa050c512804a18/prosper/src/hle/audio/hle_audio.cpp#L4367-L4384) on the PS5 module: Lock takes a per-system lock and counts nested holds, Unlock releases one hold from any thread and returns 0 without a hold, and `sceNgs2SystemRender` only tries the lock: while another thread holds it, Render clears one grain of each output, leaves the render count and voices unchanged and returns 0. The other calls do not wait for the system lock. Peace Walker (PPSA18216) holds it while it spins on a flag that its audio thread clears only after `sceNgs2SystemRender` and eight `sceNgs2VoiceGetStateFlags` calls, so on the console neither can wait for it. Whether the console's voice and rack calls take the lock at all is not known. `sceNgs2RackLock` still holds the library's lock until `sceNgs2RackUnlock`, as before
 - [sceNgs2RackGetInfo](../../../core/libs/prx/libSceNgs2.native/src/Rack.cpp) (libSceNgs2.native) - field order from [shadPS4](https://github.com/shadps4-emu/shadPS4)'s `OrbisNgs2RackInfo` with the 64-byte name of the other info structures (216 bytes); `type` is assumed to be the rack id divided by 0x1000, `min_grain_samples` 64 (the system minimum) and `state_flags` 1; `uid` comes from a per-process rack counter, `render_count` is the owning system's render count and `active_voice_count` counts voices that are not empty (paused and stopped voices included). The last process ratio and tick and `active_channel_work_count` are left zero
 - [sceNgs2VoiceQueryInfo](../../../core/libs/prx/libSceNgs2.native/src/Voice.cpp) (libSceNgs2.native) - only info id 0x4001 is known: PPSA22520 passes an 8-byte buffer and branches on the first word being 1 or 2, so it is assumed to be the voice channel count (0 before the voice is set up); the second word is zeroed and other ids throw
 - [sceNgs2VoiceControl patch](../../../core/libs/prx/libSceNgs2.native/src/Voice.cpp) (libSceNgs2.native) - a destination rack handle with input id 0 resolves to that rack's first voice; this matches the Dead Cells call in #4334, but selection for racks with multiple voices has not been checked against a console. Nonzero destination input ids still throw. `sceNgs2VoiceGetPortInfo` reports the resolved voice handle and zero delay.