Bug 1456115 - Remove locking in AcmReceiver::GetAudio. r?dminor draft
authorPaul Adenot <paul@paul.cx>
Thu, 12 Apr 2018 14:36:02 +0200
changeset 787246 910254a18841b98287e6f8986601279f4e2f945d
parent 787245 6059cc975097bdbd46679f013ab7ef08caf28114
child 787247 960ca31f6b6fb2738d54f6683431484fb976e12d
push id107695
push userpaul@paul.cx
push dateTue, 24 Apr 2018 16:04:50 +0000
reviewersdminor
bugs1456115
milestone61.0a1
Bug 1456115 - Remove locking in AcmReceiver::GetAudio. r?dminor This also causes a lot of dropouts. We don't need to lock here. NetEQ is thread safe, and its created in the ctor. The rest of the members are made atomic or is simply never accessed in multiple threads. MozReview-Commit-ID: 2fRw5ZgxdpQ
media/webrtc/trunk/webrtc/audio/audio_receive_stream.cc
media/webrtc/trunk/webrtc/common_types.h
media/webrtc/trunk/webrtc/modules/audio_coding/acm2/acm_receiver.cc
media/webrtc/trunk/webrtc/modules/audio_coding/acm2/acm_receiver.h
--- a/media/webrtc/trunk/webrtc/audio/audio_receive_stream.cc
+++ b/media/webrtc/trunk/webrtc/audio/audio_receive_stream.cc
@@ -198,17 +198,17 @@ webrtc::AudioReceiveStream::Stats AudioR
   stats.jitter_buffer_ms = ns.currentBufferSize;
   stats.jitter_buffer_preferred_ms = ns.preferredBufferSize;
   stats.expand_rate = Q14ToFloat(ns.currentExpandRate);
   stats.speech_expand_rate = Q14ToFloat(ns.currentSpeechExpandRate);
   stats.secondary_decoded_rate = Q14ToFloat(ns.currentSecondaryDecodedRate);
   stats.accelerate_rate = Q14ToFloat(ns.currentAccelerateRate);
   stats.preemptive_expand_rate = Q14ToFloat(ns.currentPreemptiveRate);
 
-  auto ds = channel_proxy_->GetDecodingCallStatistics();
+  auto ds(channel_proxy_->GetDecodingCallStatistics());
   stats.decoding_calls_to_silence_generator = ds.calls_to_silence_generator;
   stats.decoding_calls_to_neteq = ds.calls_to_neteq;
   stats.decoding_normal = ds.decoded_normal;
   stats.decoding_plc = ds.decoded_plc;
   stats.decoding_cng = ds.decoded_cng;
   stats.decoding_plc_cng = ds.decoded_plc_cng;
   stats.decoding_muted_output = ds.decoded_muted_output;
 
--- a/media/webrtc/trunk/webrtc/common_types.h
+++ b/media/webrtc/trunk/webrtc/common_types.h
@@ -9,16 +9,17 @@
  */
 
 #ifndef WEBRTC_COMMON_TYPES_H_
 #define WEBRTC_COMMON_TYPES_H_
 
 #include <stddef.h>
 #include <string.h>
 
+#include <atomic>
 #include <ostream>
 #include <string>
 #include <vector>
 
 #include "webrtc/api/video/video_rotation.h"
 #include "webrtc/base/checks.h"
 #include "webrtc/base/optional.h"
 #include "webrtc/typedefs.h"
@@ -403,24 +404,47 @@ struct AudioDecodingCallStats {
       : calls_to_silence_generator(0),
         calls_to_neteq(0),
         decoded_normal(0),
         decoded_plc(0),
         decoded_cng(0),
         decoded_plc_cng(0),
         decoded_muted_output(0) {}
 
-  int calls_to_silence_generator;  // Number of calls where silence generated,
+  AudioDecodingCallStats(const AudioDecodingCallStats& other)
+  {
+    calls_to_silence_generator = other.calls_to_silence_generator.load();
+    calls_to_neteq = other.calls_to_neteq.load();
+    decoded_normal = other.decoded_normal.load();
+    decoded_plc = other.decoded_plc.load();
+    decoded_cng = other.decoded_cng.load();
+    decoded_plc_cng = other.decoded_plc_cng.load();
+    decoded_muted_output = other.decoded_muted_output.load();
+  }
+
+  AudioDecodingCallStats& operator=(const AudioDecodingCallStats& other)
+  {
+    calls_to_silence_generator = other.calls_to_silence_generator.load();
+    calls_to_neteq = other.calls_to_neteq.load();
+    decoded_normal = other.decoded_normal.load();
+    decoded_plc = other.decoded_plc.load();
+    decoded_cng = other.decoded_cng.load();
+    decoded_plc_cng = other.decoded_plc_cng.load();
+    decoded_muted_output = other.decoded_muted_output.load();
+    return *this;
+  }
+
+  std::atomic<int> calls_to_silence_generator;  // Number of calls where silence generated,
                                    // and NetEq was disengaged from decoding.
-  int calls_to_neteq;              // Number of calls to NetEq.
-  int decoded_normal;  // Number of calls where audio RTP packet decoded.
-  int decoded_plc;     // Number of calls resulted in PLC.
-  int decoded_cng;  // Number of calls where comfort noise generated due to DTX.
-  int decoded_plc_cng;  // Number of calls resulted where PLC faded to CNG.
-  int decoded_muted_output;  // Number of calls returning a muted state output.
+  std::atomic<int> calls_to_neteq;              // Number of calls to NetEq.
+  std::atomic<int> decoded_normal;  // Number of calls where audio RTP packet decoded.
+  std::atomic<int> decoded_plc;     // Number of calls resulted in PLC.
+  std::atomic<int> decoded_cng;  // Number of calls where comfort noise generated due to DTX.
+  std::atomic<int> decoded_plc_cng;  // Number of calls resulted where PLC faded to CNG.
+  std::atomic<int> decoded_muted_output;  // Number of calls returning a muted state output.
 };
 
 // Type of Noise Suppression.
 enum NsModes {
   kNsUnchanged = 0,   // previously set mode
   kNsDefault,         // platform default
   kNsConference,      // conferencing default
   kNsLowSuppression,  // lowest suppression
--- a/media/webrtc/trunk/webrtc/modules/audio_coding/acm2/acm_receiver.cc
+++ b/media/webrtc/trunk/webrtc/modules/audio_coding/acm2/acm_receiver.cc
@@ -114,19 +114,16 @@ int AcmReceiver::InsertPacket(const WebR
   }
   return 0;
 }
 
 int AcmReceiver::GetAudio(int desired_freq_hz,
                           AudioFrame* audio_frame,
                           bool* muted) {
   RTC_DCHECK(muted);
-  // Accessing members, take the lock.
-  rtc::CritScope lock(&crit_sect_);
-
   if (neteq_->GetAudio(audio_frame, muted) != NetEq::kOK) {
     LOG(LERROR) << "AcmReceiver::GetAudio - NetEq Failed.";
     return -1;
   }
 
   const int current_sample_rate_hz = neteq_->last_output_sample_rate_hz();
 
   // Update if resampling is required.
--- a/media/webrtc/trunk/webrtc/modules/audio_coding/acm2/acm_receiver.h
+++ b/media/webrtc/trunk/webrtc/modules/audio_coding/acm2/acm_receiver.h
@@ -273,21 +273,23 @@ class AcmReceiver {
       uint8_t first_payload_byte) const EXCLUSIVE_LOCKS_REQUIRED(crit_sect_);
 
   uint32_t NowInTimestamp(int decoder_sampling_rate) const;
 
   rtc::CriticalSection crit_sect_;
   rtc::Optional<CodecInst> last_audio_decoder_ GUARDED_BY(crit_sect_);
   rtc::Optional<SdpAudioFormat> last_audio_format_ GUARDED_BY(crit_sect_);
   ACMResampler resampler_ GUARDED_BY(crit_sect_);
-  std::unique_ptr<int16_t[]> last_audio_buffer_ GUARDED_BY(crit_sect_);
+  // After construction, this is only ever touched on the thread that calls
+  // AcmReceiver::GetAudio, and only modified in this method.
+  std::unique_ptr<int16_t[]> last_audio_buffer_;
   CallStatistics call_stats_ GUARDED_BY(crit_sect_);
-  NetEq* neteq_;
+  NetEq* const neteq_;
   Clock* clock_;  // TODO(henrik.lundin) Make const if possible.
-  bool resampled_last_output_frame_ GUARDED_BY(crit_sect_);
+  std::atomic<bool> resampled_last_output_frame_;
   rtc::Optional<int> last_packet_sample_rate_hz_ GUARDED_BY(crit_sect_);
   std::atomic<int> last_audio_format_clockrate_hz_;
 };
 
 }  // namespace acm2
 
 }  // namespace webrtc