Initial implementation of RtpTransportControllerReceive and related interfaces.

webrtc/call/rtp_transport_controller_receive.h

+/*
+ *  Copyright (c) 2017 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.
+ */
+#ifndef WEBRTC_CALL_RTP_TRANSPORT_CONTROLLER_RECEIVE_H_
+#define WEBRTC_CALL_RTP_TRANSPORT_CONTROLLER_RECEIVE_H_
+
+#include <memory>
+
+#include "webrtc/base/array_view.h"
+
+// For DeliveryStatus.
+// TODO(nisse): This file ought to not depend on call.
+#include "webrtc/call/call.h"
+
+namespace webrtc {
+
+class ReceiveSideCongestionController;
+class RtpPacketReceived;
+
+// This class represents a receiver of already parsed RTP packets.
+class RtpPacketSinkInterface {
+ public:
+  virtual ~RtpPacketSinkInterface() {}
+  virtual void OnRtpPacket(const RtpPacketReceived& packet) = 0;

[pthatcher1, 2017/04/29 00:58:46]

Why the name "RtpPacketReceived"?  Why not just "RtpPacket" or perhaps, since
it's a "parsed RTP packet", must "ParsedRtpPacket"?  "RtpPacketReceived" is a
confusing name.  

+};
+
+// TODO(nisse): This additional interface may be a bit confusing, it's
+// needed because RTP extensions are currently associated with each
+// stream. We should be able to identify extensions earlier, since the
+// mapping is the same for all streams on the same transport. Then we
+// can delete this class, replacing it with RtpPacketSinkInterface.
+// There's also the subtlety that the per-transport negotiation may
+// differ from the extensions expected for a particular stream, and we
+// may have code that breaks if we identify extension which is not
+// consistent with the configuration of the stream.
+
+// This class represents a receiver of RTP packets, which has the
+// additional responsibility of identifying extension headers. I.e.,
+// the caller of OnRtpPacketReceive is expected to have called
+// packet->Parse, but not packet->IdentifyExtensions.
+class RtpPacketReceiverInterface {
+ public:
+  virtual ~RtpPacketReceiverInterface() {}
+  virtual bool OnRtpPacketReceive(RtpPacketReceived* packet) = 0;
+};

[pthatcher1, 2017/04/29 00:58:46]

Could we, instead, have an RtpHeaderExtensionIdentifier that has
IdentifyHeaderExtensions?  Then instead of calling OnRtpPacketReceive with an
RtpPacket that doesn't know it's header extensions, it first calls
IdentifyHeaderExtensions and then calls RtpPacketSinceInterface::OnRtpPacket?   
That would be a lot more clear.

But even then, I think it would be better to just get rid the need for this in
the first place before doing this work.  

[nisse-webrtc, 2017/05/02 09:34:47]

I think we could. It adds one more method call on the receive path, but that's
probably no big deal. Or we could keep parsing and identification in Call for
the time being, like it's currently done with Call::receive_rtp_config.
Maybe. It's tricky to find a sane order of the steps...

+
+// This class represents the RTP receive parsing and demuxing, for a

[pthatcher1, 2017/04/29 00:58:46]

"RTP receive parsing and demuxing".  What does "receive" mean here?  And why
does the demuxer need to parse first?  Why not have an RtpParserInterface?  And
an RtpDemuxerInterface?  Even if the demuxer also parses, it should at least be
called RtpDemuxer.  That would be a *much* more clear name.  

[nisse-webrtc, 2017/05/02 09:34:47]

The intent is that this interface is passed to ReceiveStream constructors. Then
what it needs to do will change over time. Initially, it will be a demuxer.
Further on, it should act like a factory for RtpTransportReceiver objects. 

+// single transport. It isn't thread aware, leaving responsibility of
+// multithreading issues to the user of this class. TODO(nisse): Add
+// RTCP processing, we should aim to terminate RTCP and not leave any
+// RTCP processing to individual receive streams. TODO(nisse): Extract
+// parsing and demuxing logic into separate classes. A demuxer should
+// handle only a single transport.
+class RtpTransportControllerReceiveInterface {
+ public:
+  // Configuration needed for media-independent processing of received
+  // packets.
+  struct Config {
+    // Send side bandwidth estimation is defined in
+    // draft-holmer-rmcat-transport-wide-cc-extensions. When invoking
+    // the ReceiveSideCongestionController, we need to know if
+    // send-side bwe is configured or not.
+    bool use_send_side_bwe = false;
+  };
+
+  // Creates the default implementation.
+  static std::unique_ptr<RtpTransportControllerReceiveInterface> Create(
+      ReceiveSideCongestionController* receive_side_cc,
+      bool enable_receive_side_bwe);
+
+  virtual ~RtpTransportControllerReceiveInterface() {}
+
+  // Registers the receiver responsible for an ssrc.
+  virtual void AddReceiver(uint32_t ssrc,
+                           const Config& config,

[pthatcher1, 2017/04/29 00:58:46]

Is this going to do full demux using MID, SSRCs, and PTs?  If so, the config
should contain the ssrc.  Later, it needs to expand to an MID, a list of SSRCs,
and a list of PTs.  So it's probably best to just stick it in the config now so
you don't have to keep changing the interface.

I can see this doing just SSRC demux and a higher-level construct will do full
demux, and that's probably fine.  But it should be made clear here.  

+                           RtpPacketReceiverInterface* receiver) = 0;
+  // TODO(nisse): It's unclear what info is conveniently available at
+  // remove time. For now, we take only the |receiver| pointer and
+  // iterate over the mapping.
+  virtual void RemoveReceiver(const RtpPacketReceiverInterface* receiver) = 0;
+
+  // Used to represent auxillary sinks, currently used for FlexFec.
+  // The responsible receiver must be registered first.
+  virtual void AddSink(uint32_t ssrc, RtpPacketSinkInterface* sink) = 0;

[pthatcher1, 2017/04/29 00:58:46]

Why do we need two separate interfaces for getting an RTP packet?  Why doesn't
this just use AddReceiver?

+  // TODO(nisse): It's unclear what info is conveniently available at
+  // remove time. For now, we take only the |receiver| pointer and
+  // iterate over the mapping.
+  virtual void RemoveSink(const RtpPacketSinkInterface* sink) = 0;
+
+  // Process raw incoming RTP packets.
+  // TODO(nisse): DeliveryStatus is needed for the current handling of
+  // unsignalled ssrcs. Change return type to bool or void, once we do
+  // that via AddPayload instead.
+  virtual PacketReceiver::DeliveryStatus OnRtpPacket(
+      int64_t arrival_time_ms,
+      rtc::ArrayView<const uint8_t> packet) = 0;
+};
+
+}  // namespace webrtc
+
+#endif  // WEBRTC_CALL_RTP_TRANSPORT_CONTROLLER_RECEIVE_H_