Bug 1456115 - Remove locking in AcmReceiver::GetAudio. r=dminor
☠☠ backed out by d1e4fdc34670 ☠ ☠
authorPaul Adenot <paul@paul.cx>
Thu, 12 Apr 2018 14:36:02 +0200
changeset 468950 e48268ffa4d8c92fa5f740929625bde722c78b65
parent 468949 bffecfd92808acbf5f3ab2d16482ac8f020c5892
child 468951 9c77f5b53b564d73386fb6c74f4f4d8a321d3492
push id9165
push userasasaki@mozilla.com
push dateThu, 26 Apr 2018 21:04:54 +0000
treeherdermozilla-beta@064c3804de2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdminor
bugs1456115
milestone61.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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