Wire up transport sequence number / send time callbacks to webrtc via libjingle.

talk/session/media/channel.cc

 /*
  * libjingle
  * Copyright 2004 Google Inc.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are met:
  *
  *  1. Redistributions of source code must retain the above copyright notice,
  *     this list of conditions and the following disclaimer.
  *  2. Redistributions in binary form must reproduce the above copyright notice,
@@ skipping 12 matching lines @@
  * WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
  * OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
  * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 #include "talk/session/media/channel.h"
 
 #include "talk/media/base/constants.h"
 #include "talk/media/base/rtputils.h"
 #include "webrtc/p2p/base/transportchannel.h"
+#include "talk/app/webrtc/mediacontroller.h"
 #include "talk/session/media/channelmanager.h"
 #include "webrtc/base/bind.h"
 #include "webrtc/base/buffer.h"
 #include "webrtc/base/byteorder.h"
 #include "webrtc/base/common.h"
 #include "webrtc/base/dscp.h"
 #include "webrtc/base/logging.h"
+#include "webrtc/call.h"
+#include "webrtc/common_types.h"
 
 namespace cricket {
 
 using rtc::Bind;
 
 enum {
   MSG_EARLYMEDIATIMEOUT = 1,
   MSG_SCREENCASTWINDOWEVENT,
   MSG_RTPPACKET,
   MSG_RTCPPACKET,
@@ skipping 10 matching lines @@
 static const int kAgcMinus10db = -10;
 
 static void SafeSetError(const std::string& message, std::string* error_desc) {
   if (error_desc) {
     *error_desc = message;
   }
 }
 
 struct PacketMessageData : public rtc::MessageData {
   rtc::Buffer packet;
-  rtc::DiffServCodePoint dscp;
+  rtc::PacketOptions options;
 };
 
 struct ScreencastEventMessageData : public rtc::MessageData {
   ScreencastEventMessageData(uint32 s, rtc::WindowEvent we)
       : ssrc(s),
         event(we) {
   }
   uint32 ssrc;
   rtc::WindowEvent event;
 };
@@ skipping 83 matching lines @@
 template <class Codec, class Options>
 void RtpSendParametersFromMediaDescription(
     const MediaContentDescriptionImpl<Codec>* desc,
     RtpSendParameters<Codec, Options>* send_params) {
   RtpParametersFromMediaDescription(desc, send_params);
   send_params->max_bandwidth_bps = desc->bandwidth();
 }
 
 BaseChannel::BaseChannel(rtc::Thread* thread,
                          MediaChannel* media_channel,
+                         webrtc::MediaControllerInterface* media_controller,
                          TransportController* transport_controller,
                          const std::string& content_name,
                          bool rtcp)
     : worker_thread_(thread),
       transport_controller_(transport_controller),
       media_channel_(media_channel),
+      media_controller_(media_controller),
       content_name_(content_name),
       rtcp_transport_enabled_(rtcp),
       transport_channel_(nullptr),
       rtcp_transport_channel_(nullptr),
       enabled_(false),
       writable_(false),
       rtp_ready_to_send_(false),
       rtcp_ready_to_send_(false),
       was_ever_writable_(false),
       local_content_direction_(MD_INACTIVE),
@@ skipping 147 matching lines @@
   UpdateWritableState_w();
   SetReadyToSend(true, new_tc && new_tc->writable());
 }
 
 void BaseChannel::ConnectToTransportChannel(TransportChannel* tc) {
   ASSERT(worker_thread_ == rtc::Thread::Current());
 
   tc->SignalWritableState.connect(this, &BaseChannel::OnWritableState);
   tc->SignalReadPacket.connect(this, &BaseChannel::OnChannelRead);
   tc->SignalReadyToSend.connect(this, &BaseChannel::OnReadyToSend);
+  media_controller_->ConnectTransportChannel(tc);

[pthatcher1, 2015/10/07 16:44:38]

This isn't quite the right place to put this, since it will be called every time
bundle configuration changes.  I think it would be better to put it in
WebRtcSession::CreateChannels after the BaseChannels are created.  At that
point, get the transport channel from the audio and video channels.

[stefan-webrtc, 2015/10/08 12:53:45]

Ok, that won't cause problems if the transport channel changes somehow? Seems
like since we have BaseChannel::SetTransport we may create new transport
channels after WebRtcSession::CreateChannels has returned?

I guess if I connect the two after the call to EnableBundle we should be fine?

[pthatcher1, 2015/10/08 20:27:55]

You can connect before or after EnableBundle.  The only way the transport
channel can change is if it changes to a transport channel from another
BaseChannel.   

That does mean we should worry about the case where they are bundled onto the
BaseChannel for data (the DataChannel).  In that case, you'd need to either
listen to the TransportChannel of the DataChannel or do it post-BUNDLE.  But
BUNDLE can happen at any time, so that's not really safe to rely on.

[stefan-webrtc, 2015/10/09 14:18:44]

Acknowledged.

 }
 
 void BaseChannel::DisconnectFromTransportChannel(TransportChannel* tc) {
   ASSERT(worker_thread_ == rtc::Thread::Current());
 
   tc->SignalWritableState.disconnect(this);
   tc->SignalReadPacket.disconnect(this);
   tc->SignalReadyToSend.disconnect(this);
+  media_controller_->DisconnectTransportChannel(tc);

[pthatcher1, 2015/10/07 16:44:38]

Woh, this won't work.  If two BaseChannels share a TransportChannel and one of
them is deleted, it will disconnect, and then the media controller will also
disconnect.

I think the easiest thing is to just not have a DisconnectTransportChannel
method.  The worst thing that will happen is that MediaController will have
dangling pointers in transport_channels_.  But as long as we document that, I
think it's fine.  

[stefan-webrtc, 2015/10/07 16:55:25]

Very good point. I will look at these comments later today or tomorrow.

[stefan-webrtc, 2015/10/08 12:53:45]

I assume media controller is guaranteed to outlive transport channels then?

[pthatcher1, 2015/10/08 20:27:55]

Yes, a MediaController has to outlive all the BaseChannels and once all the
BaseChannels are destroyed, then the TransportChannels are as well.

However, that is a bit subtle and probably easy to screw up in the future.  We
could fix it, but it would be a bit complex.  Now that I read your code in
WebRtcSession, I realize that it probably makes more sense to use WebRtcSession
as our temporary bind point and not MediaController.  I apologize for jumping
around on the design so much on this, but I think this will be a lot better.
Something like this:

1.  WebRtcSession connects to TransportChannel::OnSentPacket in three places:
  A.  At the end of CreateVoiceChannel.
  B.  At the end of CreateVideoChannel.
  C.  At the end of CreateDataChannel.
2.  WebRtcSession never disconnects from TransportChannel::OnSentPacket because
the session is guaranteed to outlive the TransportChannel since it owns the
TransportController.
3.  WebRtcSession::OnSentPacket_w can then call
media_controller_->OnSentPacket() or
media_controller_->call_w()->OnSentPacket(), whichever is easier.


This way, it's safer and the MediaController doesn't have to tack the
TransportChannel lifetimes.  


Sorry again for the jumping around to different designs.  But it looks like we
can end up in a good state.

[stefan-webrtc, 2015/10/09 14:18:44]

No problem, I prefer to do the right thing here and simplify for the future
improvements you have planned. And of course it would be better to avoid random
crashes... :) This is difficult to get right, and I wouldn't have been able to
do it in twice the time without your help.

I managed to get this working, and with a bit of code reading I managed to write
a test.

 }
 
 bool BaseChannel::Enable(bool enable) {
   worker_thread_->Invoke<void>(Bind(
       enable ? &BaseChannel::EnableMedia_w : &BaseChannel::DisableMedia_w,
       this));
   return true;
 }
 
 bool BaseChannel::AddRecvStream(const StreamParams& sp) {
@@ skipping 59 matching lines @@
 bool BaseChannel::IsReadyToSend() const {
   // Send outgoing data if we are enabled, have local and remote content,
   // and we have had some form of connectivity.
   return enabled() &&
          IsReceiveContentDirection(remote_content_direction_) &&
          IsSendContentDirection(local_content_direction_) &&
          was_ever_writable();
 }
 
 bool BaseChannel::SendPacket(rtc::Buffer* packet,
-                             rtc::DiffServCodePoint dscp) {
-  return SendPacket(false, packet, dscp);
+                             const rtc::PacketOptions& options) {
+  return SendPacket(false, packet, options);
 }
 
 bool BaseChannel::SendRtcp(rtc::Buffer* packet,
-                           rtc::DiffServCodePoint dscp) {
-  return SendPacket(true, packet, dscp);
+                           const rtc::PacketOptions& options) {
+  return SendPacket(true, packet, options);
 }
 
 int BaseChannel::SetOption(SocketType type, rtc::Socket::Option opt,
                            int value) {
   TransportChannel* channel = NULL;
   switch (type) {
     case ST_RTP:
       channel = transport_channel_;
       socket_options_.push_back(
           std::pair<rtc::Socket::Option, int>(opt, value));
@@ skipping 48 matching lines @@
     media_channel_->OnReadyToSend(false);
   }
 }
 
 bool BaseChannel::PacketIsRtcp(const TransportChannel* channel,
                                const char* data, size_t len) {
   return (channel == rtcp_transport_channel_ ||
           rtcp_mux_filter_.DemuxRtcp(data, static_cast<int>(len)));
 }
 
-bool BaseChannel::SendPacket(bool rtcp, rtc::Buffer* packet,
-                             rtc::DiffServCodePoint dscp) {
+bool BaseChannel::SendPacket(bool rtcp,
+                             rtc::Buffer* packet,
+                             const rtc::PacketOptions& options) {
   // SendPacket gets called from MediaEngine, typically on an encoder thread.
   // If the thread is not our worker thread, we will post to our worker
   // so that the real work happens on our worker. This avoids us having to
   // synchronize access to all the pieces of the send path, including
   // SRTP and the inner workings of the transport channels.
   // The only downside is that we can't return a proper failure code if
   // needed. Since UDP is unreliable anyway, this should be a non-issue.
   if (rtc::Thread::Current() != worker_thread_) {
     // Avoid a copy by transferring the ownership of the packet data.
     int message_id = (!rtcp) ? MSG_RTPPACKET : MSG_RTCPPACKET;
     PacketMessageData* data = new PacketMessageData;
     data->packet = packet->Pass();
-    data->dscp = dscp;
+    data->options = options;
     worker_thread_->Post(this, message_id, data);
     return true;
   }
 
   // Now that we are on the correct thread, ensure we have a place to send this
   // packet before doing anything. (We might get RTCP packets that we don't
   // intend to send.) If we've negotiated RTCP mux, send RTCP over the RTP
   // transport.
   TransportChannel* channel = (!rtcp || rtcp_mux_filter_.IsActive()) ?
       transport_channel_ : rtcp_transport_channel_;
   if (!channel || !channel->writable()) {
     return false;
   }
 
   // Protect ourselves against crazy data.
   if (!ValidPacket(rtcp, packet)) {
     LOG(LS_ERROR) << "Dropping outgoing " << content_name_ << " "
                   << PacketType(rtcp)
                   << " packet: wrong size=" << packet->size();
     return false;
   }
 
-  rtc::PacketOptions options(dscp);
+  rtc::PacketOptions updated_options;
+  updated_options = options;
   // Protect if needed.
   if (srtp_filter_.IsActive()) {
     bool res;
     uint8_t* data = packet->data();
     int len = static_cast<int>(packet->size());
     if (!rtcp) {
     // If ENABLE_EXTERNAL_AUTH flag is on then packet authentication is not done
     // inside libsrtp for a RTP packet. A external HMAC module will be writing
     // a fake HMAC value. This is ONLY done for a RTP packet.
     // Socket layer will update rtp sendtime extension header if present in
     // packet with current time before updating the HMAC.
 #if !defined(ENABLE_EXTERNAL_AUTH)
       res = srtp_filter_.ProtectRtp(
           data, len, static_cast<int>(packet->capacity()), &len);
 #else
-      options.packet_time_params.rtp_sendtime_extension_id =
+      updated_options.packet_time_params.rtp_sendtime_extension_id =
           rtp_abs_sendtime_extn_id_;
       res = srtp_filter_.ProtectRtp(
           data, len, static_cast<int>(packet->capacity()), &len,
-          &options.packet_time_params.srtp_packet_index);
+          &updated_options.packet_time_params.srtp_packet_index);
       // If protection succeeds, let's get auth params from srtp.
       if (res) {
         uint8* auth_key = NULL;
         int key_len;
         res = srtp_filter_.GetRtpAuthParams(
-            &auth_key, &key_len, &options.packet_time_params.srtp_auth_tag_len);
+            &auth_key, &key_len,
+            &updated_options.packet_time_params.srtp_auth_tag_len);
         if (res) {
-          options.packet_time_params.srtp_auth_key.resize(key_len);
-          options.packet_time_params.srtp_auth_key.assign(auth_key,
-                                                          auth_key + key_len);
+          updated_options.packet_time_params.srtp_auth_key.resize(key_len);
+          updated_options.packet_time_params.srtp_auth_key.assign(
+              auth_key, auth_key + key_len);
         }
       }
 #endif
       if (!res) {
         int seq_num = -1;
         uint32 ssrc = 0;
         GetRtpSeqNum(data, len, &seq_num);
         GetRtpSsrc(data, len, &ssrc);
         LOG(LS_ERROR) << "Failed to protect " << content_name_
                       << " RTP packet: size=" << len
@@ skipping 658 matching lines @@
       FindHeaderExtension(extensions, kRtpAbsoluteSenderTimeHeaderExtension);
   rtp_abs_sendtime_extn_id_ =
       send_time_extension ? send_time_extension->id : -1;
 }
 
 void BaseChannel::OnMessage(rtc::Message *pmsg) {
   switch (pmsg->message_id) {
     case MSG_RTPPACKET:
     case MSG_RTCPPACKET: {
       PacketMessageData* data = static_cast<PacketMessageData*>(pmsg->pdata);
-      SendPacket(pmsg->message_id == MSG_RTCPPACKET, &data->packet, data->dscp);
+      SendPacket(pmsg->message_id == MSG_RTCPPACKET, &data->packet,
+                 data->options);
       delete data;  // because it is Posted
       break;
     }
     case MSG_FIRSTPACKETRECEIVED: {
       SignalFirstPacketReceived(this);
       break;
     }
   }
 }
 
 void BaseChannel::FlushRtcpMessages() {
   // Flush all remaining RTCP messages. This should only be called in
   // destructor.
   ASSERT(rtc::Thread::Current() == worker_thread_);
   rtc::MessageList rtcp_messages;
   worker_thread_->Clear(this, MSG_RTCPPACKET, &rtcp_messages);
   for (rtc::MessageList::iterator it = rtcp_messages.begin();
        it != rtcp_messages.end(); ++it) {
     worker_thread_->Send(this, MSG_RTCPPACKET, it->pdata);
   }
 }
 
 VoiceChannel::VoiceChannel(rtc::Thread* thread,
                            MediaEngineInterface* media_engine,
                            VoiceMediaChannel* media_channel,
+                           webrtc::MediaControllerInterface* media_controller,
                            TransportController* transport_controller,
                            const std::string& content_name,
                            bool rtcp)
     : BaseChannel(thread,
                   media_channel,
+                  media_controller,
                   transport_controller,
                   content_name,
                   rtcp),
       media_engine_(media_engine),
       received_media_(false) {}
 
 VoiceChannel::~VoiceChannel() {
   StopAudioMonitor();
   StopMediaMonitor();
   // this can't be done in the base class, since it calls a virtual
@@ skipping 293 matching lines @@
   SignalAudioMonitor(this, info);
 }
 
 void VoiceChannel::GetSrtpCryptoSuiteNames(
     std::vector<std::string>* ciphers) const {
   GetSupportedAudioCryptoSuites(ciphers);
 }
 
 VideoChannel::VideoChannel(rtc::Thread* thread,
                            VideoMediaChannel* media_channel,
+                           webrtc::MediaControllerInterface* media_controller,
                            TransportController* transport_controller,
                            const std::string& content_name,
                            bool rtcp)
     : BaseChannel(thread,
                   media_channel,
+                  media_controller,
                   transport_controller,
                   content_name,
                   rtcp),
       renderer_(NULL),
       previous_we_(rtc::WE_CLOSE) {}
 
 bool VideoChannel::Init() {
   if (!BaseChannel::Init()) {
     return false;
   }
@@ skipping 370 matching lines @@
   GetSupportedVideoCryptoSuites(ciphers);
 }
 
 DataChannel::DataChannel(rtc::Thread* thread,
                          DataMediaChannel* media_channel,
                          TransportController* transport_controller,
                          const std::string& content_name,
                          bool rtcp)
     : BaseChannel(thread,
                   media_channel,
+                  nullptr,
                   transport_controller,
                   content_name,
                   rtcp),
       data_channel_type_(cricket::DCT_NONE),
       ready_to_send_data_(false) {}
 
 DataChannel::~DataChannel() {
   StopMediaMonitor();
   // this can't be done in the base class, since it calls a virtual
   DisableMedia_w();
@@ skipping 292 matching lines @@
   return (data_channel_type_ == DCT_RTP);
 }
 
 void DataChannel::OnStreamClosedRemotely(uint32 sid) {
   rtc::TypedMessageData<uint32>* message =
       new rtc::TypedMessageData<uint32>(sid);
   signaling_thread()->Post(this, MSG_STREAMCLOSEDREMOTELY, message);
 }
 
 }  // namespace cricket