Bug 1376873 - Disable or replace thread check assertions; r=pehrsons

The changes to Call are required because we create the Call object on the main
thread, but deliver packets and query stats from the socket thread.

The changes to ChannelProxy are required because we query stats from the
socket thread rather than the main thread.

For RtpVideoStreamReceiver, this removes the worker_task_checker_ assertions
and replaces them with a critical section. This is how the code worked prior
to this update. We create the Call object (and thus eventually the
RtpVideoStreamReceiver) on the main thread, but we want to deliver packets on
the socket thread. To retain these assertions we'd either have to dispatch calls
to deliver packets from the socket thread to the main, which seems pretty bad
from a performance point of view, or we'd have to refactor the code to create
the Call object on the socket thread, which seems like a major refactoring
best done outside of a branch update. Going back to the previous behaviour
seemed like the least bad alternative.

Differential Revision: https://phabricator.services.mozilla.com/D7440

--HG--
extra : rebase_source : f427225442dba10683f3928add8059a3630aafe5

media/webrtc/signaling/src/media-conduit/RtpPacketQueue.h

@@ -1,13 +1,14 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-#ifndef RTP_PACKET_QUEUE_H_
-#define RTP_PACKET_QUEUE_H_
-
-#include "nsTArray.h"
+#ifndef RtpPacketQueue_h
+#define RtpPacketQueue_h
 
 #include "MediaConduitInterface.h"
+#include "nsTArray.h"
 
 namespace mozilla {
 
@@ -34,9 +35,7 @@ public:
 
   void Enqueue(const void* data, int len)
   {
-    UniquePtr<QueuedPacket> packet((QueuedPacket*) malloc(sizeof(QueuedPacket) + len-1));
-    packet->mLen = len;
-    memcpy(packet->mData, data, len);
+    UniquePtr<QueuedPacket> packet(new QueuedPacket(data, len));
     mQueuedPackets.AppendElement(std::move(packet));
     mQueueActive = true;
   }
@@ -49,12 +48,24 @@ public:
 private:
   bool mQueueActive = false;
   struct QueuedPacket {
-    int mLen;
-    uint8_t mData[1];
+    const int mLen;
+    uint8_t *mData;
+
+    QueuedPacket(const void *aData, size_t aLen)
+      : mLen(aLen)
+    {
+      mData = new uint8_t[mLen];
+      memcpy(mData, aData, mLen);
+    }
+
+    ~QueuedPacket()
+    {
+      delete(mData);
+    }
   };
   nsTArray<UniquePtr<QueuedPacket>> mQueuedPackets;
 };
 
-}
+} // namespace mozilla
 
-#endif
+#endif //RtpPacketQueue_h

media/webrtc/trunk/webrtc/audio/audio_receive_stream.cc

@@ -161,7 +161,8 @@ void AudioReceiveStream::Stop() {
 }
 
 webrtc::AudioReceiveStream::Stats AudioReceiveStream::GetStats() const {
-  RTC_DCHECK_RUN_ON(&worker_thread_checker_);
+  // TODO: Mozilla - currently we run stats on the STS thread
+  //RTC_DCHECK_RUN_ON(&worker_thread_checker_);
   webrtc::AudioReceiveStream::Stats stats;
   stats.remote_ssrc = config_.rtp.remote_ssrc;
 

media/webrtc/trunk/webrtc/call/call.cc

@@ -596,7 +596,8 @@ void Call::UpdateReceiveHistograms() {
 }
 
 PacketReceiver* Call::Receiver() {
-  RTC_DCHECK_CALLED_SEQUENTIALLY(&configuration_sequence_checker_);
+  //Mozilla: Called from STS thread while delivering packets
+  //RTC_DCHECK_CALLED_SEQUENTIALLY(&configuration_sequence_checker_);
   return this;
 }
 
@@ -1407,7 +1408,8 @@ PacketReceiver::DeliveryStatus Call::DeliverPacket(
     const uint8_t* packet,
     size_t length,
     const PacketTime& packet_time) {
-  RTC_DCHECK_CALLED_SEQUENTIALLY(&configuration_sequence_checker_);
+  //Mozilla: Called from STS thread while delivering packets
+  //RTC_DCHECK_CALLED_SEQUENTIALLY(&configuration_sequence_checker_);
   if (RtpHeaderParser::IsRtcp(packet, length))
     return DeliverRtcp(media_type, packet, length);
 

media/webrtc/trunk/webrtc/video/rtp_video_stream_receiver.cc

@@ -304,10 +304,11 @@ int32_t RtpVideoStreamReceiver::OnInitializeDecoder(
 // This method handles both regular RTP packets and packets recovered
 // via FlexFEC.
 void RtpVideoStreamReceiver::OnRtpPacket(const RtpPacketReceived& packet) {
-  RTC_DCHECK_CALLED_SEQUENTIALLY(&worker_task_checker_);
-
-  if (!receiving_) {
-    return;
+  {
+    rtc::CritScope lock(&receive_cs_);
+    if (!receiving_) {
+      return;
+    }
   }
 
   if (!packet.recovered()) {
@@ -450,7 +451,7 @@ rtc::Optional<int64_t> RtpVideoStreamReceiver::LastReceivedKeyframePacketMs()
 }
 
 void RtpVideoStreamReceiver::AddSecondarySink(RtpPacketSinkInterface* sink) {
-  RTC_DCHECK_CALLED_SEQUENTIALLY(&worker_task_checker_);
+  rtc::CritScope lock(&receive_cs_);
   RTC_DCHECK(std::find(secondary_sinks_.cbegin(), secondary_sinks_.cend(),
                        sink) == secondary_sinks_.cend());
   secondary_sinks_.push_back(sink);
@@ -458,7 +459,7 @@ void RtpVideoStreamReceiver::AddSecondarySink(RtpPacketSinkInterface* sink) {
 
 void RtpVideoStreamReceiver::RemoveSecondarySink(
     const RtpPacketSinkInterface* sink) {
-  RTC_DCHECK_CALLED_SEQUENTIALLY(&worker_task_checker_);
+  rtc::CritScope lock(&receive_cs_);
   auto it = std::find(secondary_sinks_.begin(), secondary_sinks_.end(), sink);
   if (it == secondary_sinks_.end()) {
     // We might be rolling-back a call whose setup failed mid-way. In such a
@@ -490,7 +491,6 @@ void RtpVideoStreamReceiver::ReceivePacket(const uint8_t* packet,
 
 void RtpVideoStreamReceiver::ParseAndHandleEncapsulatingHeader(
     const uint8_t* packet, size_t packet_length, const RTPHeader& header) {
-  RTC_DCHECK_CALLED_SEQUENTIALLY(&worker_task_checker_);
   if (rtp_payload_registry_.IsRed(header)) {
     int8_t ulpfec_pt = rtp_payload_registry_.ulpfec_payload_type();
     if (packet[header.headerLength] == ulpfec_pt) {
@@ -546,10 +546,11 @@ void RtpVideoStreamReceiver::NotifyReceiverOfFecPacket(
 
 bool RtpVideoStreamReceiver::DeliverRtcp(const uint8_t* rtcp_packet,
                                          size_t rtcp_packet_length) {
-  RTC_DCHECK_CALLED_SEQUENTIALLY(&worker_task_checker_);
-
-  if (!receiving_) {
-    return false;
+  {
+    rtc::CritScope lock(&receive_cs_);
+    if (!receiving_) {
+      return false;
+    }
   }
 
   rtp_rtcp_->IncomingRtcpPacket(rtcp_packet, rtcp_packet_length);
@@ -619,12 +620,12 @@ void RtpVideoStreamReceiver::SignalNetworkState(NetworkState state) {
 }
 
 void RtpVideoStreamReceiver::StartReceive() {
-  RTC_DCHECK_CALLED_SEQUENTIALLY(&worker_task_checker_);
+  rtc::CritScope lock(&receive_cs_);
   receiving_ = true;
 }
 
 void RtpVideoStreamReceiver::StopReceive() {
-  RTC_DCHECK_CALLED_SEQUENTIALLY(&worker_task_checker_);
+  rtc::CritScope lock(&receive_cs_);
   receiving_ = false;
 }
 

media/webrtc/trunk/webrtc/video/rtp_video_stream_receiver.h

@@ -182,9 +182,9 @@ class RtpVideoStreamReceiver : public RtpData,
   ReceiveStatistics* const rtp_receive_statistics_;
   std::unique_ptr<UlpfecReceiver> ulpfec_receiver_;
 
-  rtc::SequencedTaskChecker worker_task_checker_;
-  bool receiving_ RTC_GUARDED_BY(worker_task_checker_);
-  int64_t last_packet_log_ms_ RTC_GUARDED_BY(worker_task_checker_);
+  rtc::CriticalSection receive_cs_;
+  bool receiving_ RTC_GUARDED_BY(receive_cs_);
+  int64_t last_packet_log_ms_ RTC_GUARDED_BY(receive_cs_);
 
   const std::unique_ptr<RtpRtcp> rtp_rtcp_;
 
@@ -208,7 +208,7 @@ class RtpVideoStreamReceiver : public RtpData,
   bool has_received_frame_;
 
   std::vector<RtpPacketSinkInterface*> secondary_sinks_
-      RTC_GUARDED_BY(worker_task_checker_);
+      RTC_GUARDED_BY(receive_cs_);
 };
 
 }  // namespace webrtc