diff --git a/src/xenia/apu/xma_context.cc b/src/xenia/apu/xma_context.cc index 05463630d..e0cf74bb9 100644 --- a/src/xenia/apu/xma_context.cc +++ b/src/xenia/apu/xma_context.cc @@ -35,7 +35,9 @@ extern "C" { namespace xe { namespace apu { -XmaContext::XmaContext() = default; +XmaContext::XmaContext() + : work_completion_event_( + xe::threading::Event::CreateAutoResetEvent(false)) {} XmaContext::~XmaContext() {} diff --git a/src/xenia/apu/xma_context.h b/src/xenia/apu/xma_context.h index d74ae08ac..f4e9645a6 100644 --- a/src/xenia/apu/xma_context.h +++ b/src/xenia/apu/xma_context.h @@ -15,6 +15,7 @@ #include #include +#include "xenia/base/threading.h" #include "xenia/memory.h" #include "xenia/xbox.h" @@ -212,11 +213,28 @@ class XmaContext { uint32_t id() { return id_; } uint32_t guest_ptr() { return guest_ptr_; } - bool is_allocated() { return is_allocated_; } - bool is_enabled() { return is_enabled_; } + bool is_allocated() { return is_allocated_.load(std::memory_order_acquire); } + bool is_enabled() { return is_enabled_.load(std::memory_order_acquire); } - void set_is_allocated(bool is_allocated) { is_allocated_ = is_allocated; } - void set_is_enabled(bool is_enabled) { is_enabled_ = is_enabled; } + void set_is_allocated(bool is_allocated) { + is_allocated_.store(is_allocated, std::memory_order_release); + } + void set_is_enabled(bool is_enabled) { + is_enabled_.store(is_enabled, std::memory_order_release); + } + + // Signals that the worker has finished processing this context after a kick. + void SignalWorkDone() { + if (work_completion_event_) { + work_completion_event_->Set(); + } + } + // Blocks until the worker has finished processing this context. + void WaitForWorkDone() { + if (work_completion_event_) { + xe::threading::Wait(work_completion_event_.get(), false); + } + } protected: static void DumpRaw(AVFrame* frame, int id); @@ -229,8 +247,9 @@ class XmaContext { uint32_t id_ = 0; uint32_t guest_ptr_ = 0; xe_mutex lock_; - volatile bool is_allocated_ = false; - volatile bool is_enabled_ = false; + std::atomic is_allocated_ = false; + std::atomic is_enabled_ = false; + std::unique_ptr work_completion_event_; // ffmpeg structures AVPacket* av_packet_ = nullptr; diff --git a/src/xenia/apu/xma_context_new.cc b/src/xenia/apu/xma_context_new.cc index 5e4b5dd0c..46f4d8919 100644 --- a/src/xenia/apu/xma_context_new.cc +++ b/src/xenia/apu/xma_context_new.cc @@ -121,6 +121,7 @@ bool XmaContextNew::Work() { auto context_ptr = memory()->TranslateVirtual(guest_ptr()); XMA_CONTEXT_DATA data(context_ptr); + const XMA_CONTEXT_DATA initial_data = data; if (!data.output_buffer_valid) { return true; @@ -142,7 +143,7 @@ bool XmaContextNew::Work() { data.output_buffer_read_offset == data.output_buffer_write_offset) { ClearLocked(&data); } - data.Store(context_ptr); + StoreContextMerged(data, initial_data, context_ptr); return true; } @@ -156,7 +157,7 @@ bool XmaContextNew::Work() { XELOGD("XmaContext {}: No space for subframe decoding {}/{}!", id(), minimum_subframe_decode_count, remaining_subframe_blocks_in_output_buffer_); - data.Store(context_ptr); + StoreContextMerged(data, initial_data, context_ptr); return true; } @@ -192,26 +193,11 @@ bool XmaContextNew::Work() { data.output_buffer_valid = 0; } - // TODO: Rewrite! - // There is a case when game can modify certain parts of context mid-play - // and decoder should be aware of it - data.Store(context_ptr); + StoreContextMerged(data, initial_data, context_ptr); return true; } -void XmaContextNew::Enable() { - std::lock_guard lock(lock_); - - auto context_ptr = memory()->TranslateVirtual(guest_ptr()); - XMA_CONTEXT_DATA data(context_ptr); - - XELOGAPU("XmaContext: kicking context {} (buffer {} {}/{} bits)", id(), - data.current_buffer, data.input_buffer_read_offset, - data.GetCurrentInputBufferPacketCount() * kBitsPerPacket); - - data.Store(context_ptr); - set_is_enabled(true); -} +void XmaContextNew::Enable() { set_is_enabled(true); } bool XmaContextNew::Block(bool poll) { if (!lock_.try_lock()) { @@ -247,16 +233,12 @@ void XmaContextNew::ClearLocked(XMA_CONTEXT_DATA* data) { current_frame_remaining_subframes_ = 0; } -void XmaContextNew::Disable() { - std::lock_guard lock(lock_); - XELOGAPU("XmaContext: disabling context {}", id()); - set_is_enabled(false); -} +void XmaContextNew::Disable() { set_is_enabled(false); } void XmaContextNew::Release() { // Lock it in case the decoder thread is working on it now. std::lock_guard lock(lock_); - assert_true(is_allocated_ == true); + assert_true(is_allocated()); set_is_allocated(false); auto context_ptr = memory()->TranslateVirtual(guest_ptr()); @@ -757,5 +739,36 @@ bool XmaContextNew::DecodePacket(AVCodecContext* av_context, return true; } +void XmaContextNew::StoreContextMerged(const XMA_CONTEXT_DATA& data, + const XMA_CONTEXT_DATA& initial_data, + uint8_t* context_ptr) { + XMA_CONTEXT_DATA fresh(context_ptr); + + // DWORD 0: decoder owns loop_count, output_buffer_write_offset. + // Only clear valid flags the decoder actually consumed (was 1, now 0). + fresh.loop_count = data.loop_count; + fresh.output_buffer_write_offset = data.output_buffer_write_offset; + if (initial_data.input_buffer_0_valid && !data.input_buffer_0_valid) { + fresh.input_buffer_0_valid = 0; + } + if (initial_data.input_buffer_1_valid && !data.input_buffer_1_valid) { + fresh.input_buffer_1_valid = 0; + } + + // DWORD 1: decoder conditionally clears output_buffer_valid + if (initial_data.output_buffer_valid && !data.output_buffer_valid) { + fresh.output_buffer_valid = 0; + } + + // DWORD 2: decoder owns input_buffer_read_offset, error_status + fresh.input_buffer_read_offset = data.input_buffer_read_offset; + fresh.error_status = data.error_status; + + // DWORD 4: decoder owns current_buffer + fresh.current_buffer = data.current_buffer; + + fresh.Store(context_ptr); +} + } // namespace apu } // namespace xe diff --git a/src/xenia/apu/xma_context_new.h b/src/xenia/apu/xma_context_new.h index 111a4e0d4..9a0a956f9 100644 --- a/src/xenia/apu/xma_context_new.h +++ b/src/xenia/apu/xma_context_new.h @@ -99,6 +99,12 @@ class XmaContextNew : public XmaContext { bool DecodePacket(AVCodecContext* av_context, const AVPacket* av_packet, AVFrame* av_frame); + // Re-reads context from guest memory and merges only decoder-owned fields, + // preserving any game modifications made during decoding. + void StoreContextMerged(const XMA_CONTEXT_DATA& data, + const XMA_CONTEXT_DATA& initial_data, + uint8_t* context_ptr); + std::array input_buffer_; // first byte contains bit offset information std::array xma_frame_; diff --git a/src/xenia/apu/xma_decoder.cc b/src/xenia/apu/xma_decoder.cc index 36aa24be4..ca06f8c6c 100644 --- a/src/xenia/apu/xma_decoder.cc +++ b/src/xenia/apu/xma_decoder.cc @@ -193,17 +193,15 @@ X_STATUS XmaDecoder::Setup(kernel::KernelState* kernel_state) { } void XmaDecoder::WorkerThreadMain() { - uint32_t idle_loop_count = 0; while (worker_running_) { // Okay, let's loop through XMA contexts to find ones we need to decode! bool did_work = false; for (uint32_t n = 0; n < kContextCount; n++) { - did_work = contexts_[n]->Work() || did_work; - - // TODO: Need thread safety to do this. - // Probably not too important though. - // registers_.current_context = n; - // registers_.next_context = (n + 1) % kContextCount; + bool worked = contexts_[n]->Work(); + if (worked) { + contexts_[n]->SignalWorkDone(); + } + did_work = did_work || worked; } if (paused_) { @@ -211,10 +209,8 @@ void XmaDecoder::WorkerThreadMain() { resume_fence_.Wait(); } - if (!did_work) { - idle_loop_count++; - } else { - idle_loop_count = 0; + if (did_work) { + continue; } xe::threading::Wait(work_event_.get(), false); } @@ -340,6 +336,7 @@ void XmaDecoder::WriteRegister(uint32_t addr, uint32_t value) { // The context ID is a bit in the range of the entire context array. const uint32_t base_context_id = (r - XmaRegister::Context0Kick) * 32; + const uint32_t kicked_value = value; while (value) { const uint32_t context_id = base_context_id + std::countr_zero(value); auto& context = *contexts_[context_id]; @@ -351,6 +348,16 @@ void XmaDecoder::WriteRegister(uint32_t addr, uint32_t value) { } // Signal the decoder thread to start processing. work_event_->SetBoostPriority(); + if (cvars::use_dedicated_xma_thread) { + // Block until the worker finishes, so the game sees updated context data. + uint32_t remaining = kicked_value; + while (remaining) { + const uint32_t context_id = + base_context_id + std::countr_zero(remaining); + contexts_[context_id]->WaitForWorkDone(); + remaining &= remaining - 1; + } + } } else if (r >= XmaRegister::Context0Lock && r <= XmaRegister::Context9Lock) { // Context lock command. // This requests a lock by flagging the context. @@ -360,11 +367,10 @@ void XmaDecoder::WriteRegister(uint32_t addr, uint32_t value) { const uint32_t context_id = base_context_id + std::countr_zero(value); auto& context = *contexts_[context_id]; context.Disable(); + // Ensure the worker isn't mid-processing this context. + context.Block(false); value &= value - 1; } - - // Signal the decoder thread to start processing. - // work_event_->Set(); } else if (r >= XmaRegister::Context0Clear && r <= XmaRegister::Context9Clear) { // Context clear command. diff --git a/src/xenia/apu/xma_decoder.h b/src/xenia/apu/xma_decoder.h index 753ebe703..9423b0fb6 100644 --- a/src/xenia/apu/xma_decoder.h +++ b/src/xenia/apu/xma_decoder.h @@ -47,7 +47,7 @@ class XmaDecoder { uint32_t ReadRegister(uint32_t addr); void WriteRegister(uint32_t addr, uint32_t value); - bool is_paused() const { return paused_; } + bool is_paused() const { return paused_.load(std::memory_order_acquire); } void Pause(); void Resume(); @@ -74,7 +74,7 @@ class XmaDecoder { kernel::object_ref worker_thread_; std::unique_ptr work_event_ = nullptr; - bool paused_ = false; + std::atomic paused_ = false; xe::threading::Fence pause_fence_; // Signaled when worker paused. xe::threading::Fence resume_fence_; // Signaled when resume requested.