From 683f3165f95abc345603ef1c2385677a0a70218d Mon Sep 17 00:00:00 2001 From: Tommi Date: Sun, 23 Apr 2023 09:36:09 +0200 Subject: [PATCH] Add slightly more constness to SourceFrame and the embedded AudioFrame This makes it a bit more clear that values of member variables of SourceFrame are never directly changed and that doing so is not an intentional part of the design. Also made use of `SourceFrame` vs `const SourceFrame` more consistent since the audio frame of a `const SourceFrame` was being modified in some places. Accessing the embedded AudioFrame can be done via the const audio_frame() accessor or via the mutable_audio_frame() accessor when modifying the frame is needed. This helps with clarifying later on when downstream code paths such as ones that access the `packet_infos_` data, can know that it won't be modified for the rest of the frame's lifetime (thus avoiding having to make copies). Bug: none Change-Id: I175cec8fcdb85063239a5f9c299b7878c639f00e Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/302383 Reviewed-by: Sam Zackrisson Commit-Queue: Tomas Gunnarsson Cr-Commit-Position: refs/heads/main@{#39926} --- modules/audio_mixer/audio_mixer_impl.cc | 86 ++++++++++++++----------- 1 file changed, 48 insertions(+), 38 deletions(-) diff --git a/modules/audio_mixer/audio_mixer_impl.cc b/modules/audio_mixer/audio_mixer_impl.cc index 666497484b..a1247d7338 100644 --- a/modules/audio_mixer/audio_mixer_impl.cc +++ b/modules/audio_mixer/audio_mixer_impl.cc @@ -39,62 +39,71 @@ struct AudioMixerImpl::SourceStatus { namespace { -struct SourceFrame { +class SourceFrame { + public: + // Default constructor required by call to `vector::resize()` below. SourceFrame() = default; SourceFrame(AudioMixerImpl::SourceStatus* source_status, AudioFrame* audio_frame, bool muted) - : source_status(source_status), audio_frame(audio_frame), muted(muted) { - RTC_DCHECK(source_status); - RTC_DCHECK(audio_frame); - if (!muted) { - energy = AudioMixerCalculateEnergy(*audio_frame); - } - } + : SourceFrame(source_status, + audio_frame, + muted, + muted ? 0u : AudioMixerCalculateEnergy(*audio_frame)) {} SourceFrame(AudioMixerImpl::SourceStatus* source_status, AudioFrame* audio_frame, bool muted, uint32_t energy) - : source_status(source_status), - audio_frame(audio_frame), - muted(muted), - energy(energy) { + : source_status_(source_status), + audio_frame_(audio_frame), + muted_(muted), + energy_(energy) { RTC_DCHECK(source_status); - RTC_DCHECK(audio_frame); + RTC_DCHECK(audio_frame_); } - AudioMixerImpl::SourceStatus* source_status = nullptr; - AudioFrame* audio_frame = nullptr; - bool muted = true; - uint32_t energy = 0; + AudioMixerImpl::SourceStatus* source_status() { return source_status_; } + const AudioFrame* audio_frame() const { return audio_frame_; } + AudioFrame* mutable_audio_frame() { return audio_frame_; } + bool muted() const { return muted_; } + uint32_t energy() const { return energy_; } + + private: + // The below values are never changed directly, hence only accessors are + // offered. The values can change though via implicit assignment when sorting + // vectors. Pointer values will be nullptr when default constructed as a + // result of calling `vector::resize()`. + AudioMixerImpl::SourceStatus* source_status_ = nullptr; + AudioFrame* audio_frame_ = nullptr; + bool muted_ = true; + uint32_t energy_ = 0u; }; // ShouldMixBefore(a, b) is used to select mixer sources. // Returns true if `a` is preferred over `b` as a source to be mixed. bool ShouldMixBefore(const SourceFrame& a, const SourceFrame& b) { - if (a.muted != b.muted) { - return b.muted; + if (a.muted() != b.muted()) { + return b.muted(); } - const auto a_activity = a.audio_frame->vad_activity_; - const auto b_activity = b.audio_frame->vad_activity_; + const auto a_activity = a.audio_frame()->vad_activity_; + const auto b_activity = b.audio_frame()->vad_activity_; if (a_activity != b_activity) { return a_activity == AudioFrame::kVadActive; } - return a.energy > b.energy; + return a.energy() > b.energy(); } -void RampAndUpdateGain( - rtc::ArrayView mixed_sources_and_frames) { - for (const auto& source_frame : mixed_sources_and_frames) { - float target_gain = source_frame.source_status->is_mixed ? 1.0f : 0.0f; - Ramp(source_frame.source_status->gain, target_gain, - source_frame.audio_frame); - source_frame.source_status->gain = target_gain; +void RampAndUpdateGain(rtc::ArrayView mixed_sources_and_frames) { + for (auto& source_frame : mixed_sources_and_frames) { + float target_gain = source_frame.source_status()->is_mixed ? 1.0f : 0.0f; + Ramp(source_frame.source_status()->gain, target_gain, + source_frame.mutable_audio_frame()); + source_frame.source_status()->gain = target_gain; } } @@ -226,13 +235,13 @@ rtc::ArrayView AudioMixerImpl::GetAudioFromSources( audio_source_mixing_data_view.end(), ShouldMixBefore); int max_audio_frame_counter = max_sources_to_mix_; - int ramp_list_lengh = 0; + int ramp_list_length = 0; int audio_to_mix_count = 0; // Go through list in order and put unmuted frames in result list. - for (const auto& p : audio_source_mixing_data_view) { + for (auto& p : audio_source_mixing_data_view) { // Filter muted. - if (p.muted) { - p.source_status->is_mixed = false; + if (p.muted()) { + p.source_status()->is_mixed = false; continue; } @@ -240,15 +249,16 @@ rtc::ArrayView AudioMixerImpl::GetAudioFromSources( bool is_mixed = false; if (max_audio_frame_counter > 0) { --max_audio_frame_counter; - helper_containers_->audio_to_mix[audio_to_mix_count++] = p.audio_frame; - helper_containers_->ramp_list[ramp_list_lengh++] = - SourceFrame(p.source_status, p.audio_frame, false, -1); + helper_containers_->audio_to_mix[audio_to_mix_count++] = + p.mutable_audio_frame(); + helper_containers_->ramp_list[ramp_list_length++] = + SourceFrame(p.source_status(), p.mutable_audio_frame(), false, -1); is_mixed = true; } - p.source_status->is_mixed = is_mixed; + p.source_status()->is_mixed = is_mixed; } RampAndUpdateGain(rtc::ArrayView( - helper_containers_->ramp_list.data(), ramp_list_lengh)); + helper_containers_->ramp_list.data(), ramp_list_length)); return rtc::ArrayView( helper_containers_->audio_to_mix.data(), audio_to_mix_count); }