From 329387745615a0d6cdb06c9c540667693fc625bf Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 7 Mar 2019 05:32:40 -0500 Subject: [PATCH 1/4] service/audio/hwopus: Enclose internals in an anonymous namespace Makes it impossible to violate the ODR, as well as providing a place for future changes. --- src/core/hle/service/audio/hwopus.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/core/hle/service/audio/hwopus.cpp b/src/core/hle/service/audio/hwopus.cpp index 11eba4a129..00a4b9d535 100644 --- a/src/core/hle/service/audio/hwopus.cpp +++ b/src/core/hle/service/audio/hwopus.cpp @@ -16,7 +16,7 @@ #include "core/hle/service/audio/hwopus.h" namespace Service::Audio { - +namespace { struct OpusDeleter { void operator()(void* ptr) const { operator delete(ptr); @@ -178,10 +178,11 @@ private: u32 channel_count; }; -static std::size_t WorkerBufferSize(u32 channel_count) { +std::size_t WorkerBufferSize(u32 channel_count) { ASSERT_MSG(channel_count == 1 || channel_count == 2, "Invalid channel count"); return opus_decoder_get_size(static_cast(channel_count)); } +} // Anonymous namespace void HwOpus::GetWorkBufferSize(Kernel::HLERequestContext& ctx) { IPC::RequestParser rp{ctx}; From d41d85766fac0ef1e8b6ac356e0aaffc67fd2a0e Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 7 Mar 2019 05:35:49 -0500 Subject: [PATCH 2/4] service/audio/hwopus: Move Opus packet header out of the IHardwareOpusDecoderManager This will be utilized by more than just that class in the future. This also renames it from OpusHeader to OpusPacketHeader to be more specific about what kind of header it is. --- src/core/hle/service/audio/hwopus.cpp | 34 +++++++++++++-------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/core/hle/service/audio/hwopus.cpp b/src/core/hle/service/audio/hwopus.cpp index 00a4b9d535..61af2e6d6b 100644 --- a/src/core/hle/service/audio/hwopus.cpp +++ b/src/core/hle/service/audio/hwopus.cpp @@ -23,6 +23,12 @@ struct OpusDeleter { } }; +struct OpusPacketHeader { + u32_be size; + INSERT_PADDING_WORDS(1); +}; +static_assert(sizeof(OpusPacketHeader) == 0x8, "OpusHeader is an invalid size"); + class IHardwareOpusDecoderManager final : public ServiceFramework { public: IHardwareOpusDecoderManager(std::unique_ptr decoder, u32 sample_rate, @@ -113,23 +119,23 @@ private: std::vector& output, u64* out_performance_time) { const auto start_time = std::chrono::high_resolution_clock::now(); const std::size_t raw_output_sz = output.size() * sizeof(opus_int16); - if (sizeof(OpusHeader) > input.size()) { + if (sizeof(OpusPacketHeader) > input.size()) { LOG_ERROR(Audio, "Input is smaller than the header size, header_sz={}, input_sz={}", - sizeof(OpusHeader), input.size()); + sizeof(OpusPacketHeader), input.size()); return false; } - OpusHeader hdr{}; - std::memcpy(&hdr, input.data(), sizeof(OpusHeader)); - if (sizeof(OpusHeader) + static_cast(hdr.sz) > input.size()) { + OpusPacketHeader hdr{}; + std::memcpy(&hdr, input.data(), sizeof(OpusPacketHeader)); + if (sizeof(OpusPacketHeader) + static_cast(hdr.size) > input.size()) { LOG_ERROR(Audio, "Input does not fit in the opus header size. data_sz={}, input_sz={}", - sizeof(OpusHeader) + static_cast(hdr.sz), input.size()); + sizeof(OpusPacketHeader) + static_cast(hdr.size), input.size()); return false; } - const auto frame = input.data() + sizeof(OpusHeader); + const auto frame = input.data() + sizeof(OpusPacketHeader); const auto decoded_sample_count = opus_packet_get_nb_samples( - frame, static_cast(input.size() - sizeof(OpusHeader)), + frame, static_cast(input.size() - sizeof(OpusPacketHeader)), static_cast(sample_rate)); if (decoded_sample_count * channel_count * sizeof(u16) > raw_output_sz) { LOG_ERROR( @@ -141,18 +147,18 @@ private: const int frame_size = (static_cast(raw_output_sz / sizeof(s16) / channel_count)); const auto out_sample_count = - opus_decode(decoder.get(), frame, hdr.sz, output.data(), frame_size, 0); + opus_decode(decoder.get(), frame, hdr.size, output.data(), frame_size, 0); if (out_sample_count < 0) { LOG_ERROR(Audio, "Incorrect sample count received from opus_decode, " "output_sample_count={}, frame_size={}, data_sz_from_hdr={}", - out_sample_count, frame_size, static_cast(hdr.sz)); + out_sample_count, frame_size, static_cast(hdr.size)); return false; } const auto end_time = std::chrono::high_resolution_clock::now() - start_time; sample_count = out_sample_count; - consumed = static_cast(sizeof(OpusHeader) + hdr.sz); + consumed = static_cast(sizeof(OpusPacketHeader) + hdr.size); if (out_performance_time != nullptr) { *out_performance_time = std::chrono::duration_cast(end_time).count(); @@ -167,12 +173,6 @@ private: opus_decoder_ctl(decoder.get(), OPUS_RESET_STATE); } - struct OpusHeader { - u32_be sz; // Needs to be BE for some odd reason - INSERT_PADDING_WORDS(1); - }; - static_assert(sizeof(OpusHeader) == 0x8, "OpusHeader is an invalid size"); - std::unique_ptr decoder; u32 sample_rate; u32 channel_count; From 960057cba0b9b66cadbd64b83516a442235ddb32 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 7 Mar 2019 05:43:06 -0500 Subject: [PATCH 3/4] service/audio/hwopus: Provide a name for the second word of OpusPacketHeader This indicates the entropy coder's final range. --- src/core/hle/service/audio/hwopus.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/core/hle/service/audio/hwopus.cpp b/src/core/hle/service/audio/hwopus.cpp index 61af2e6d6b..aeaa2623b1 100644 --- a/src/core/hle/service/audio/hwopus.cpp +++ b/src/core/hle/service/audio/hwopus.cpp @@ -9,7 +9,7 @@ #include -#include "common/common_funcs.h" +#include "common/assert.h" #include "common/logging/log.h" #include "core/hle/ipc_helpers.h" #include "core/hle/kernel/hle_ipc.h" @@ -24,8 +24,10 @@ struct OpusDeleter { }; struct OpusPacketHeader { + // Packet size in bytes. u32_be size; - INSERT_PADDING_WORDS(1); + // Indicates the final range of the codec's entropy coder. + u32_be final_range; }; static_assert(sizeof(OpusPacketHeader) == 0x8, "OpusHeader is an invalid size"); From d03ae881fd108dd41039af1ebecc197fc4fe27c7 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 7 Mar 2019 05:52:25 -0500 Subject: [PATCH 4/4] service/audio/hwopus: Move decoder state to its own class Moves the non-multistream specific state to its own class. This will be necessary to support the multistream variants of opus decoding. --- src/core/hle/service/audio/hwopus.cpp | 139 ++++++++++++++++---------- 1 file changed, 87 insertions(+), 52 deletions(-) diff --git a/src/core/hle/service/audio/hwopus.cpp b/src/core/hle/service/audio/hwopus.cpp index aeaa2623b1..377e12cfa3 100644 --- a/src/core/hle/service/audio/hwopus.cpp +++ b/src/core/hle/service/audio/hwopus.cpp @@ -23,6 +23,8 @@ struct OpusDeleter { } }; +using OpusDecoderPtr = std::unique_ptr; + struct OpusPacketHeader { // Packet size in bytes. u32_be size; @@ -31,29 +33,8 @@ struct OpusPacketHeader { }; static_assert(sizeof(OpusPacketHeader) == 0x8, "OpusHeader is an invalid size"); -class IHardwareOpusDecoderManager final : public ServiceFramework { +class OpusDecoderStateBase { public: - IHardwareOpusDecoderManager(std::unique_ptr decoder, u32 sample_rate, - u32 channel_count) - : ServiceFramework("IHardwareOpusDecoderManager"), decoder(std::move(decoder)), - sample_rate(sample_rate), channel_count(channel_count) { - // clang-format off - static const FunctionInfo functions[] = { - {0, &IHardwareOpusDecoderManager::DecodeInterleavedOld, "DecodeInterleavedOld"}, - {1, nullptr, "SetContext"}, - {2, nullptr, "DecodeInterleavedForMultiStreamOld"}, - {3, nullptr, "SetContextForMultiStream"}, - {4, &IHardwareOpusDecoderManager::DecodeInterleavedWithPerfOld, "DecodeInterleavedWithPerfOld"}, - {5, nullptr, "DecodeInterleavedForMultiStreamWithPerfOld"}, - {6, &IHardwareOpusDecoderManager::DecodeInterleaved, "DecodeInterleaved"}, - {7, nullptr, "DecodeInterleavedForMultiStream"}, - }; - // clang-format on - - RegisterHandlers(functions); - } - -private: /// Describes extra behavior that may be asked of the decoding context. enum class ExtraBehavior { /// No extra behavior. @@ -63,30 +44,36 @@ private: ResetContext, }; - void DecodeInterleavedOld(Kernel::HLERequestContext& ctx) { - LOG_DEBUG(Audio, "called"); + enum class PerfTime { + Disabled, + Enabled, + }; - DecodeInterleavedHelper(ctx, nullptr, ExtraBehavior::None); - } - - void DecodeInterleavedWithPerfOld(Kernel::HLERequestContext& ctx) { - LOG_DEBUG(Audio, "called"); - - u64 performance = 0; - DecodeInterleavedHelper(ctx, &performance, ExtraBehavior::None); - } - - void DecodeInterleaved(Kernel::HLERequestContext& ctx) { - LOG_DEBUG(Audio, "called"); - - IPC::RequestParser rp{ctx}; - const auto extra_behavior = - rp.Pop() ? ExtraBehavior::ResetContext : ExtraBehavior::None; - - u64 performance = 0; - DecodeInterleavedHelper(ctx, &performance, extra_behavior); + virtual ~OpusDecoderStateBase() = default; + + // Decodes interleaved Opus packets. Optionally allows reporting time taken to + // perform the decoding, as well as any relevant extra behavior. + virtual void DecodeInterleaved(Kernel::HLERequestContext& ctx, PerfTime perf_time, + ExtraBehavior extra_behavior) = 0; +}; + +// Represents the decoder state for a non-multistream decoder. +class OpusDecoderState final : public OpusDecoderStateBase { +public: + explicit OpusDecoderState(OpusDecoderPtr decoder, u32 sample_rate, u32 channel_count) + : decoder{std::move(decoder)}, sample_rate{sample_rate}, channel_count{channel_count} {} + + void DecodeInterleaved(Kernel::HLERequestContext& ctx, PerfTime perf_time, + ExtraBehavior extra_behavior) override { + if (perf_time == PerfTime::Disabled) { + DecodeInterleavedHelper(ctx, nullptr, extra_behavior); + } else { + u64 performance = 0; + DecodeInterleavedHelper(ctx, &performance, extra_behavior); + } } +private: void DecodeInterleavedHelper(Kernel::HLERequestContext& ctx, u64* performance, ExtraBehavior extra_behavior) { u32 consumed = 0; @@ -97,8 +84,7 @@ private: ResetDecoderContext(); } - if (!Decoder_DecodeInterleaved(consumed, sample_count, ctx.ReadBuffer(), samples, - performance)) { + if (!DecodeOpusData(consumed, sample_count, ctx.ReadBuffer(), samples, performance)) { LOG_ERROR(Audio, "Failed to decode opus data"); IPC::ResponseBuilder rb{ctx, 2}; // TODO(ogniK): Use correct error code @@ -117,8 +103,8 @@ private: ctx.WriteBuffer(samples.data(), samples.size() * sizeof(s16)); } - bool Decoder_DecodeInterleaved(u32& consumed, u32& sample_count, const std::vector& input, - std::vector& output, u64* out_performance_time) { + bool DecodeOpusData(u32& consumed, u32& sample_count, const std::vector& input, + std::vector& output, u64* out_performance_time) const { const auto start_time = std::chrono::high_resolution_clock::now(); const std::size_t raw_output_sz = output.size() * sizeof(opus_int16); if (sizeof(OpusPacketHeader) > input.size()) { @@ -175,11 +161,61 @@ private: opus_decoder_ctl(decoder.get(), OPUS_RESET_STATE); } - std::unique_ptr decoder; + OpusDecoderPtr decoder; u32 sample_rate; u32 channel_count; }; +class IHardwareOpusDecoderManager final : public ServiceFramework { +public: + explicit IHardwareOpusDecoderManager(std::unique_ptr decoder_state) + : ServiceFramework("IHardwareOpusDecoderManager"), decoder_state{std::move(decoder_state)} { + // clang-format off + static const FunctionInfo functions[] = { + {0, &IHardwareOpusDecoderManager::DecodeInterleavedOld, "DecodeInterleavedOld"}, + {1, nullptr, "SetContext"}, + {2, nullptr, "DecodeInterleavedForMultiStreamOld"}, + {3, nullptr, "SetContextForMultiStream"}, + {4, &IHardwareOpusDecoderManager::DecodeInterleavedWithPerfOld, "DecodeInterleavedWithPerfOld"}, + {5, nullptr, "DecodeInterleavedForMultiStreamWithPerfOld"}, + {6, &IHardwareOpusDecoderManager::DecodeInterleaved, "DecodeInterleaved"}, + {7, nullptr, "DecodeInterleavedForMultiStream"}, + }; + // clang-format on + + RegisterHandlers(functions); + } + +private: + void DecodeInterleavedOld(Kernel::HLERequestContext& ctx) { + LOG_DEBUG(Audio, "called"); + + decoder_state->DecodeInterleaved(ctx, OpusDecoderStateBase::PerfTime::Disabled, + OpusDecoderStateBase::ExtraBehavior::None); + } + + void DecodeInterleavedWithPerfOld(Kernel::HLERequestContext& ctx) { + LOG_DEBUG(Audio, "called"); + + decoder_state->DecodeInterleaved(ctx, OpusDecoderStateBase::PerfTime::Enabled, + OpusDecoderStateBase::ExtraBehavior::None); + } + + void DecodeInterleaved(Kernel::HLERequestContext& ctx) { + LOG_DEBUG(Audio, "called"); + + IPC::RequestParser rp{ctx}; + const auto extra_behavior = rp.Pop() + ? OpusDecoderStateBase::ExtraBehavior::ResetContext + : OpusDecoderStateBase::ExtraBehavior::None; + + decoder_state->DecodeInterleaved(ctx, OpusDecoderStateBase::PerfTime::Enabled, + extra_behavior); + } + + std::unique_ptr decoder_state; +}; + std::size_t WorkerBufferSize(u32 channel_count) { ASSERT_MSG(channel_count == 1 || channel_count == 2, "Invalid channel count"); return opus_decoder_get_size(static_cast(channel_count)); @@ -223,8 +259,7 @@ void HwOpus::OpenOpusDecoder(Kernel::HLERequestContext& ctx) { const std::size_t worker_sz = WorkerBufferSize(channel_count); ASSERT_MSG(buffer_sz >= worker_sz, "Worker buffer too large"); - std::unique_ptr decoder{ - static_cast(operator new(worker_sz))}; + OpusDecoderPtr decoder{static_cast(operator new(worker_sz))}; if (const int err = opus_decoder_init(decoder.get(), sample_rate, channel_count)) { LOG_ERROR(Audio, "Failed to init opus decoder with error={}", err); IPC::ResponseBuilder rb{ctx, 2}; @@ -235,8 +270,8 @@ void HwOpus::OpenOpusDecoder(Kernel::HLERequestContext& ctx) { IPC::ResponseBuilder rb{ctx, 2, 0, 1}; rb.Push(RESULT_SUCCESS); - rb.PushIpcInterface(std::move(decoder), sample_rate, - channel_count); + rb.PushIpcInterface( + std::make_unique(std::move(decoder), sample_rate, channel_count)); } HwOpus::HwOpus() : ServiceFramework("hwopus") {