From 111852a9831a57b9ce19299ebf28f1e1e6b61914 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Fri, 25 Sep 2020 00:27:08 -0400 Subject: [PATCH 1/2] effect_context: Make use of explicit where applicable While we're at it we can make the destructor of the base class virtual to ensure that any polymorphism issues never occur. --- src/audio_core/effect_context.h | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/src/audio_core/effect_context.h b/src/audio_core/effect_context.h index 2f2da72dd..c2d2aa3ba 100644 --- a/src/audio_core/effect_context.h +++ b/src/audio_core/effect_context.h @@ -166,13 +166,13 @@ public: std::array raw; }; }; - static_assert(sizeof(EffectInfo::InParams) == 0xc0, "InParams is an invalid size"); + static_assert(sizeof(InParams) == 0xc0, "InParams is an invalid size"); struct OutParams { UsageStatus status{}; INSERT_PADDING_BYTES(15); }; - static_assert(sizeof(EffectInfo::OutParams) == 0x10, "OutParams is an invalid size"); + static_assert(sizeof(OutParams) == 0x10, "OutParams is an invalid size"); }; struct AuxAddress { @@ -184,8 +184,8 @@ struct AuxAddress { class EffectBase { public: - EffectBase(EffectType effect_type); - ~EffectBase(); + explicit EffectBase(EffectType effect_type); + virtual ~EffectBase(); virtual void Update(EffectInfo::InParams& in_params) = 0; virtual void UpdateForCommandGeneration() = 0; @@ -206,8 +206,7 @@ protected: template class EffectGeneric : public EffectBase { public: - EffectGeneric(EffectType effect_type) : EffectBase::EffectBase(effect_type) {} - ~EffectGeneric() = default; + explicit EffectGeneric(EffectType effect_type) : EffectBase(effect_type) {} T& GetParams() { return internal_params; @@ -224,7 +223,7 @@ private: class EffectStubbed : public EffectBase { public: explicit EffectStubbed(); - ~EffectStubbed(); + ~EffectStubbed() override; void Update(EffectInfo::InParams& in_params) override; void UpdateForCommandGeneration() override; @@ -233,7 +232,7 @@ public: class EffectI3dl2Reverb : public EffectGeneric { public: explicit EffectI3dl2Reverb(); - ~EffectI3dl2Reverb(); + ~EffectI3dl2Reverb() override; void Update(EffectInfo::InParams& in_params) override; void UpdateForCommandGeneration() override; @@ -245,7 +244,7 @@ private: class EffectBiquadFilter : public EffectGeneric { public: explicit EffectBiquadFilter(); - ~EffectBiquadFilter(); + ~EffectBiquadFilter() override; void Update(EffectInfo::InParams& in_params) override; void UpdateForCommandGeneration() override; @@ -254,7 +253,7 @@ public: class EffectAuxInfo : public EffectGeneric { public: explicit EffectAuxInfo(); - ~EffectAuxInfo(); + ~EffectAuxInfo() override; void Update(EffectInfo::InParams& in_params) override; void UpdateForCommandGeneration() override; @@ -275,7 +274,7 @@ private: class EffectDelay : public EffectGeneric { public: explicit EffectDelay(); - ~EffectDelay(); + ~EffectDelay() override; void Update(EffectInfo::InParams& in_params) override; void UpdateForCommandGeneration() override; @@ -287,7 +286,7 @@ private: class EffectBufferMixer : public EffectGeneric { public: explicit EffectBufferMixer(); - ~EffectBufferMixer(); + ~EffectBufferMixer() override; void Update(EffectInfo::InParams& in_params) override; void UpdateForCommandGeneration() override; @@ -296,7 +295,7 @@ public: class EffectReverb : public EffectGeneric { public: explicit EffectReverb(); - ~EffectReverb(); + ~EffectReverb() override; void Update(EffectInfo::InParams& in_params) override; void UpdateForCommandGeneration() override; From 8b4ecf22d485958f69ecbd2fa4ca55d9ce393826 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Fri, 25 Sep 2020 00:28:35 -0400 Subject: [PATCH 2/2] audio_core: Resolve sign conversion warnings While were at it, we can also enable sign conversion warnings and other common warnings as errors to prevent these from creeping back into the codebase. --- src/audio_core/CMakeLists.txt | 10 ++++++++++ src/audio_core/command_generator.cpp | 12 ++++++------ src/audio_core/effect_context.cpp | 8 ++++---- src/audio_core/effect_context.h | 8 ++++---- src/audio_core/info_updater.cpp | 7 +++---- src/audio_core/mix_context.cpp | 4 ++-- src/audio_core/splitter_context.cpp | 6 +++--- src/audio_core/voice_context.cpp | 4 ++-- 8 files changed, 34 insertions(+), 25 deletions(-) diff --git a/src/audio_core/CMakeLists.txt b/src/audio_core/CMakeLists.txt index cb00ef60e..6a7075f73 100644 --- a/src/audio_core/CMakeLists.txt +++ b/src/audio_core/CMakeLists.txt @@ -44,6 +44,16 @@ add_library(audio_core STATIC create_target_directory_groups(audio_core) +if (NOT MSVC) + target_compile_options(audio_core PRIVATE + -Werror=ignored-qualifiers + -Werror=implicit-fallthrough + -Werror=reorder + -Werror=sign-compare + -Werror=unused-variable + ) +endif() + target_link_libraries(audio_core PUBLIC common core) target_link_libraries(audio_core PRIVATE SoundTouch) diff --git a/src/audio_core/command_generator.cpp b/src/audio_core/command_generator.cpp index 8f7da49e6..7f2597257 100644 --- a/src/audio_core/command_generator.cpp +++ b/src/audio_core/command_generator.cpp @@ -152,7 +152,7 @@ void CommandGenerator::GenerateVoiceCommand(ServerVoiceInfo& voice_info) { if (!destination_data->IsConfigured()) { continue; } - if (destination_data->GetMixId() >= mix_context.GetCount()) { + if (destination_data->GetMixId() >= static_cast(mix_context.GetCount())) { continue; } @@ -435,7 +435,7 @@ void CommandGenerator::GenerateAuxCommand(s32 mix_buffer_offset, EffectBase* inf GetMixBuffer(output_index), worker_params.sample_count, offset, write_count); memory.WriteBlock(aux->GetRecvInfo(), &recv_info, sizeof(AuxInfoDSP)); - if (samples_read != worker_params.sample_count && + if (samples_read != static_cast(worker_params.sample_count) && samples_read <= params.sample_count) { std::memset(GetMixBuffer(output_index), 0, params.sample_count - samples_read); } @@ -611,7 +611,8 @@ void CommandGenerator::GenerateMixCommands(ServerMixInfo& mix_info) { const auto& dest_mix = mix_context.GetInfo(destination_data->GetMixId()); const auto& dest_in_params = dest_mix.GetInParams(); const auto mix_index = (base - 1) % in_params.buffer_count + in_params.buffer_offset; - for (std::size_t i = 0; i < dest_in_params.buffer_count; i++) { + for (std::size_t i = 0; i < static_cast(dest_in_params.buffer_count); + i++) { const auto mixed_volume = in_params.volume * destination_data->GetMixVolume(i); if (mixed_volume != 0.0f) { GenerateMixCommand(dest_in_params.buffer_offset + i, mix_index, mixed_volume, @@ -704,7 +705,7 @@ s32 CommandGenerator::DecodePcm16(ServerVoiceInfo& voice_info, VoiceState& dsp_s std::vector buffer(samples_processed * channel_count); memory.ReadBlock(buffer_pos, buffer.data(), buffer.size() * sizeof(s16)); - for (std::size_t i = 0; i < samples_processed; i++) { + for (std::size_t i = 0; i < static_cast(samples_processed); i++) { sample_buffer[mix_offset + i] = buffer[i * channel_count + channel]; } } @@ -789,7 +790,7 @@ s32 CommandGenerator::DecodeAdpcm(ServerVoiceInfo& voice_info, VoiceState& dsp_s position_in_frame += 2; // Decode entire frame - if (remaining_samples >= SAMPLES_PER_FRAME) { + if (remaining_samples >= static_cast(SAMPLES_PER_FRAME)) { for (std::size_t i = 0; i < SAMPLES_PER_FRAME / 2; i++) { // Sample 1 @@ -866,7 +867,6 @@ void CommandGenerator::DecodeFromWaveBuffers(ServerVoiceInfo& voice_info, s32* o const auto resample_rate = static_cast( static_cast(in_params.sample_rate) / static_cast(target_sample_rate) * static_cast(static_cast(in_params.pitch * 32768.0f))); - auto* output_base = output; if (dsp_state.fraction + sample_count * resample_rate > static_cast(SCALED_MIX_BUFFER_SIZE - 4ULL)) { return; diff --git a/src/audio_core/effect_context.cpp b/src/audio_core/effect_context.cpp index adfec3df5..4d9cdf524 100644 --- a/src/audio_core/effect_context.cpp +++ b/src/audio_core/effect_context.cpp @@ -184,19 +184,19 @@ void EffectAuxInfo::UpdateForCommandGeneration() { } } -const VAddr EffectAuxInfo::GetSendInfo() const { +VAddr EffectAuxInfo::GetSendInfo() const { return send_info; } -const VAddr EffectAuxInfo::GetSendBuffer() const { +VAddr EffectAuxInfo::GetSendBuffer() const { return send_buffer; } -const VAddr EffectAuxInfo::GetRecvInfo() const { +VAddr EffectAuxInfo::GetRecvInfo() const { return recv_info; } -const VAddr EffectAuxInfo::GetRecvBuffer() const { +VAddr EffectAuxInfo::GetRecvBuffer() const { return recv_buffer; } diff --git a/src/audio_core/effect_context.h b/src/audio_core/effect_context.h index c2d2aa3ba..2c4ce53ef 100644 --- a/src/audio_core/effect_context.h +++ b/src/audio_core/effect_context.h @@ -257,10 +257,10 @@ public: void Update(EffectInfo::InParams& in_params) override; void UpdateForCommandGeneration() override; - const VAddr GetSendInfo() const; - const VAddr GetSendBuffer() const; - const VAddr GetRecvInfo() const; - const VAddr GetRecvBuffer() const; + VAddr GetSendInfo() const; + VAddr GetSendBuffer() const; + VAddr GetRecvInfo() const; + VAddr GetRecvBuffer() const; private: VAddr send_info{}; diff --git a/src/audio_core/info_updater.cpp b/src/audio_core/info_updater.cpp index f53ce21a5..2940e53a9 100644 --- a/src/audio_core/info_updater.cpp +++ b/src/audio_core/info_updater.cpp @@ -64,7 +64,6 @@ bool InfoUpdater::UpdateBehaviorInfo(BehaviorInfo& in_behavior_info) { } bool InfoUpdater::UpdateMemoryPools(std::vector& memory_pool_info) { - const auto force_mapping = behavior_info.IsMemoryPoolForceMappingEnabled(); const auto memory_pool_count = memory_pool_info.size(); const auto total_memory_pool_in = sizeof(ServerMemoryPoolInfo::InParams) * memory_pool_count; const auto total_memory_pool_out = sizeof(ServerMemoryPoolInfo::OutParams) * memory_pool_count; @@ -174,7 +173,7 @@ bool InfoUpdater::UpdateVoices(VoiceContext& voice_context, } // Voice states for each channel std::array voice_states{}; - ASSERT(in_params.id < voice_count); + ASSERT(static_cast(in_params.id) < voice_count); // Grab our current voice info auto& voice_info = voice_context.GetInfo(static_cast(in_params.id)); @@ -352,8 +351,8 @@ ResultCode InfoUpdater::UpdateMixes(MixContext& mix_context, std::size_t mix_buf for (std::size_t i = 0; i < mix_count; i++) { const auto& in = mix_in_params[i]; total_buffer_count += in.buffer_count; - if (in.dest_mix_id > mix_count && in.dest_mix_id != AudioCommon::NO_MIX && - in.mix_id != AudioCommon::FINAL_MIX) { + if (static_cast(in.dest_mix_id) > mix_count && + in.dest_mix_id != AudioCommon::NO_MIX && in.mix_id != AudioCommon::FINAL_MIX) { LOG_ERROR( Audio, "Invalid mix destination, mix_id={:X}, dest_mix_id={:X}, mix_buffer_count={:X}", diff --git a/src/audio_core/mix_context.cpp b/src/audio_core/mix_context.cpp index 042891490..4bca72eb0 100644 --- a/src/audio_core/mix_context.cpp +++ b/src/audio_core/mix_context.cpp @@ -53,7 +53,7 @@ void MixContext::UpdateDistancesFromFinalMix() { auto mix_id = in_params.mix_id; // Needs to be referenced out of scope s32 distance_to_final_mix{AudioCommon::FINAL_MIX}; - for (; distance_to_final_mix < info_count; distance_to_final_mix++) { + for (; distance_to_final_mix < static_cast(info_count); distance_to_final_mix++) { if (mix_id == AudioCommon::FINAL_MIX) { // If we're at the final mix, we're done break; @@ -77,7 +77,7 @@ void MixContext::UpdateDistancesFromFinalMix() { } // If we're out of range for our distance, mark it as no final mix - if (distance_to_final_mix >= info_count) { + if (distance_to_final_mix >= static_cast(info_count)) { distance_to_final_mix = AudioCommon::NO_FINAL_MIX; } diff --git a/src/audio_core/splitter_context.cpp b/src/audio_core/splitter_context.cpp index 79bb2f516..f21b53147 100644 --- a/src/audio_core/splitter_context.cpp +++ b/src/audio_core/splitter_context.cpp @@ -306,7 +306,7 @@ bool SplitterContext::UpdateInfo(const std::vector& input, std::size_t& inpu break; } - if (header.send_id < 0 || header.send_id > info_count) { + if (header.send_id < 0 || static_cast(header.send_id) > info_count) { LOG_ERROR(Audio, "Bad splitter data id"); break; } @@ -348,7 +348,7 @@ bool SplitterContext::UpdateData(const std::vector& input, std::size_t& inpu break; } - if (header.splitter_id < 0 || header.splitter_id > data_count) { + if (header.splitter_id < 0 || static_cast(header.splitter_id) > data_count) { LOG_ERROR(Audio, "Bad splitter data id"); break; } @@ -434,7 +434,7 @@ const std::vector& NodeStates::GetIndexList() const { } void NodeStates::PushTsortResult(s32 index) { - ASSERT(index < node_count); + ASSERT(index < static_cast(node_count)); index_list[index_pos++] = index; } diff --git a/src/audio_core/voice_context.cpp b/src/audio_core/voice_context.cpp index 1d8f69844..863ac9267 100644 --- a/src/audio_core/voice_context.cpp +++ b/src/audio_core/voice_context.cpp @@ -488,11 +488,11 @@ s32 VoiceContext::DecodePcm16(s32* output_buffer, ServerWaveBuffer* wave_buffer, // Fast path if (channel_count == 1) { - for (std::size_t i = 0; i < samples_processed; i++) { + for (std::ptrdiff_t i = 0; i < samples_processed; i++) { output_buffer[i] = buffer_data[i]; } } else { - for (std::size_t i = 0; i < samples_processed; i++) { + for (std::ptrdiff_t i = 0; i < samples_processed; i++) { output_buffer[i] = buffer_data[i * channel_count + channel]; } }