Add AudioSendStream to Call.

webrtc/call/call.cc

 /*
  *  Copyright (c) 2013 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
 #include <string.h>
 
 #include <map>
 #include <vector>
 
 #include "webrtc/audio/audio_receive_stream.h"
+#include "webrtc/audio/audio_send_stream.h"
 #include "webrtc/base/checks.h"
 #include "webrtc/base/scoped_ptr.h"
 #include "webrtc/base/thread_annotations.h"
 #include "webrtc/call.h"
 #include "webrtc/call/rtc_event_log.h"
 #include "webrtc/common.h"
 #include "webrtc/config.h"
 #include "webrtc/modules/rtp_rtcp/interface/rtp_header_parser.h"
 #include "webrtc/modules/rtp_rtcp/source/byte_io.h"
 #include "webrtc/modules/utility/interface/process_thread.h"
@@ skipping 77 matching lines @@
   std::map<uint32_t, AudioReceiveStream*> audio_receive_ssrcs_
       GUARDED_BY(receive_crit_);
   std::map<uint32_t, VideoReceiveStream*> video_receive_ssrcs_
       GUARDED_BY(receive_crit_);
   std::set<VideoReceiveStream*> video_receive_streams_
       GUARDED_BY(receive_crit_);
   std::map<std::string, AudioReceiveStream*> sync_stream_mapping_
       GUARDED_BY(receive_crit_);
 
   rtc::scoped_ptr<RWLockWrapper> send_crit_;
+  std::map<uint32_t, AudioSendStream*> audio_send_ssrcs_ GUARDED_BY(send_crit_);

[tommi, 2015/10/14 13:21:18]

document where ownership of the send stream lies?

[the sun, 2015/10/14 14:25:24]

Done.

   std::map<uint32_t, VideoSendStream*> video_send_ssrcs_ GUARDED_BY(send_crit_);
   std::set<VideoSendStream*> video_send_streams_ GUARDED_BY(send_crit_);
 
   VideoSendStream::RtpStateMap suspended_video_send_ssrcs_;
 
   RtcEventLog* event_log_;
 
   RTC_DISALLOW_COPY_AND_ASSIGN(Call);
 };
 }  // namespace internal
@@ skipping 31 matching lines @@
 
   Trace::CreateTrace();
   module_process_thread_->Start();
 
   channel_group_->SetBweBitrates(config_.bitrate_config.min_bitrate_bps,
                                  config_.bitrate_config.start_bitrate_bps,
                                  config_.bitrate_config.max_bitrate_bps);
 }
 
 Call::~Call() {
+  RTC_CHECK_EQ(0u, audio_send_ssrcs_.size());

[tommi, 2015/10/14 13:21:18]

nit: RTC_CHECK(audio_send_ssrcs_.empty());  I see you're just repeating the
pattern, but if you don't mind fixing all of these checks, that would be
appreciated.

[the sun, 2015/10/14 14:25:24]

Done.

   RTC_CHECK_EQ(0u, video_send_ssrcs_.size());
   RTC_CHECK_EQ(0u, video_send_streams_.size());
   RTC_CHECK_EQ(0u, audio_receive_ssrcs_.size());
   RTC_CHECK_EQ(0u, video_receive_ssrcs_.size());
   RTC_CHECK_EQ(0u, video_receive_streams_.size());
 
   module_process_thread_->Stop();
   Trace::ReturnTrace();
 }
 
 PacketReceiver* Call::Receiver() { return this; }
 
 webrtc::AudioSendStream* Call::CreateAudioSendStream(
     const webrtc::AudioSendStream::Config& config) {
-  return nullptr;
+  TRACE_EVENT0("webrtc", "Call::CreateAudioSendStream");
+  LOG(LS_INFO) << "CreateAudioSendStream: " << config.ToString();
+  AudioSendStream* send_stream = new AudioSendStream(config);
+  {
+    rtc::CritScope lock(&network_enabled_crit_);
+    WriteLockScoped write_lock(*send_crit_);
+    RTC_DCHECK(audio_send_ssrcs_.find(config.rtp.ssrc) ==
+               audio_send_ssrcs_.end());
+    audio_send_ssrcs_[config.rtp.ssrc] = send_stream;
+
+    if (!network_enabled_)
+      send_stream->SignalNetworkState(kNetworkDown);
+  }
+  return send_stream;

[tommi, 2015/10/14 13:21:18]

is it guaranteed that CreateAudioSendStream() and DestroyAudioSendStream() are
always called on the same thread?
If so, please add a thread checker.  If not, then it looks to me that we have a
race since we're returning a pointer here to an object that may have been
deleted.

[the sun, 2015/10/14 14:25:24]

Well, from what I can make out, in libjingle only Call::SignalNetworkState() is
the only call not happening on the worker thread, so another option would be to
marshall that call as well. Then we could get rid of all these locks and only
have a thread checker.

pbos, mflodman?

 }
 
 void Call::DestroyAudioSendStream(webrtc::AudioSendStream* send_stream) {
+  TRACE_EVENT0("webrtc", "Call::DestroyAudioSendStream");
+  RTC_DCHECK(send_stream != nullptr);
+
+  send_stream->Stop();
+
+  AudioSendStream* audio_send_stream =
+      static_cast<AudioSendStream*>(send_stream);

[tommi, 2015/10/14 13:21:18]

I'm not groking this cast :)

[the sun, 2015/10/14 14:25:24]

We're casting a webrtc::AudioSendStream to a webrtc::internal::AudioSendStream.
I can make it explicit if you want, but then I'll change the corresponding casts
in all functions below, and send pbos your way when he complains. :)

[tommi, 2015/10/14 14:49:28]

Ah :)  that wasn't obvious to me especially since we're in webrtc:: already.  I
guess I'd prefer to have the internal class use a different name (e.g.
AudioSendStreamInternal), but you could also for now just comment what the cast
is for.

[the sun, 2015/10/15 12:56:06]

I've added explicit casts for the audio functions; I think readable code trumps
comments.

+  {
+    WriteLockScoped write_lock(*send_crit_);
+    size_t num_deleted = audio_send_ssrcs_.erase(
+        audio_send_stream->config().rtp.ssrc);
+    RTC_DCHECK(num_deleted == 1);
+  }
+  delete audio_send_stream;
 }
 
 webrtc::AudioReceiveStream* Call::CreateAudioReceiveStream(
     const webrtc::AudioReceiveStream::Config& config) {
   TRACE_EVENT0("webrtc", "Call::CreateAudioReceiveStream");
   LOG(LS_INFO) << "CreateAudioReceiveStream: " << config.ToString();
   AudioReceiveStream* receive_stream = new AudioReceiveStream(
       channel_group_->GetRemoteBitrateEstimator(), config);
   {
     WriteLockScoped write_lock(*receive_crit_);
@@ skipping 160 matching lines @@
   channel_group_->GetBitrateController()->AvailableBandwidth(&send_bandwidth);
   std::vector<unsigned int> ssrcs;
   uint32_t recv_bandwidth = 0;
   channel_group_->GetRemoteBitrateEstimator()->LatestEstimate(&ssrcs,
                                                               &recv_bandwidth);
   stats.send_bandwidth_bps = send_bandwidth;
   stats.recv_bandwidth_bps = recv_bandwidth;
   stats.pacer_delay_ms = channel_group_->GetPacerQueuingDelayMs();
   {
     ReadLockScoped read_lock(*send_crit_);
+    // TODO(solenberg): Add audio send streams.
     for (const auto& kv : video_send_ssrcs_) {
       int rtt_ms = kv.second->GetRtt();
       if (rtt_ms > 0)
         stats.rtt_ms = rtt_ms;
     }
   }
   return stats;
 }
 
 void Call::SetBitrateConfig(
@@ skipping 19 matching lines @@
 }
 
 void Call::SignalNetworkState(NetworkState state) {
   // Take crit for entire function, it needs to be held while updating streams
   // to guarantee a consistent state across streams.
   rtc::CritScope lock(&network_enabled_crit_);
   network_enabled_ = state == kNetworkUp;
   channel_group_->SignalNetworkState(state);
   {
     ReadLockScoped write_lock(*send_crit_);
+    for (auto& kv : audio_send_ssrcs_) {

[tommi, 2015/10/14 13:21:18]

no chance ov avoiding holding these locks (network_enabled_crit_ and send_crit_)
while calling out?

[the sun, 2015/10/14 14:25:24]

See above comment...

+      kv.second->SignalNetworkState(state);
+    }
     for (auto& kv : video_send_ssrcs_) {
       kv.second->SignalNetworkState(state);
     }
   }
   {
     ReadLockScoped write_lock(*receive_crit_);
     for (auto& kv : video_receive_ssrcs_) {
       kv.second->SignalNetworkState(state);
     }
   }
@@ skipping 120 matching lines @@
     size_t length,
     const PacketTime& packet_time) {
   if (RtpHeaderParser::IsRtcp(packet, length))
     return DeliverRtcp(media_type, packet, length);
 
   return DeliverRtp(media_type, packet, length, packet_time);
 }
 
 }  // namespace internal
 }  // namespace webrtc