Simplify and extend RtpHeaderExtensionMap

webrtc/modules/rtp_rtcp/source/rtp_header_extension.h

Index: webrtc/modules/rtp_rtcp/source/rtp_header_extension.h
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_header_extension.h b/webrtc/modules/rtp_rtcp/source/rtp_header_extension.h
index b053f7f8ca2910c3c55eb2bfbbfa78a4263ddacb..f1a02f612a37843cf2e7028c8e1ccf722a122294 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_header_extension.h
+++ b/webrtc/modules/rtp_rtcp/source/rtp_header_extension.h
@@ -11,10 +11,11 @@
 #ifndef WEBRTC_MODULES_RTP_RTCP_SOURCE_RTP_HEADER_EXTENSION_H_
 #define WEBRTC_MODULES_RTP_RTCP_SOURCE_RTP_HEADER_EXTENSION_H_
 
-#include <map>
-
+#include <initializer_list>
+#include <string>
+#include "webrtc/base/basictypes.h"
+#include "webrtc/config.h"
 #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h"
-#include "webrtc/typedefs.h"
 
 namespace webrtc {
 
@@ -35,74 +36,53 @@ const int kPlayoutDelayGranularityMs = 10;
 // Maximum playout delay value in milliseconds.
 const int kPlayoutDelayMaxMs = 40950;
 
-struct HeaderExtension {
-  explicit HeaderExtension(RTPExtensionType extension_type)
-      : type(extension_type), length(0) {
-    Init();
-  }
-
-  void Init() {
-    // TODO(solenberg): Create handler classes for header extensions so we can
-    // get rid of switches like these as well as handling code spread out all
-    // over.
-    switch (type) {
-      case kRtpExtensionTransmissionTimeOffset:
-        length = kTransmissionTimeOffsetLength;
-        break;
-      case kRtpExtensionAudioLevel:
-        length = kAudioLevelLength;
-        break;
-      case kRtpExtensionAbsoluteSendTime:
-        length = kAbsoluteSendTimeLength;
-        break;
-      case kRtpExtensionVideoRotation:
-        length = kVideoRotationLength;
-        break;
-      case kRtpExtensionTransportSequenceNumber:
-        length = kTransportSequenceNumberLength;
-        break;
-      case kRtpExtensionPlayoutDelay:
-        length = kPlayoutDelayLength;
-        break;
-      default:
-        assert(false);
-    }
-  }
-
-  const RTPExtensionType type;
-  uint8_t length;
-};
-
 class RtpHeaderExtensionMap {
  public:
   static constexpr RTPExtensionType kInvalidType = kRtpExtensionNone;
   static constexpr uint8_t kInvalidId = 0;
-  RtpHeaderExtensionMap();
-  ~RtpHeaderExtensionMap();
 
-  void Erase();
-
-  int32_t Register(RTPExtensionType type, uint8_t id);
+  RtpHeaderExtensionMap();
+  RtpHeaderExtensionMap(std::initializer_list<RtpExtension>);
 
-  int32_t Deregister(RTPExtensionType type);
+  template <typename Extension>
+  bool Register(uint8_t id) {
+    return Register(id, Extension::kId, Extension::kValueSizeBytes,
+                    Extension::kUri);
+  }
+  bool RegisterByType(uint8_t id, RTPExtensionType type);
+  bool RegisterByUri(uint8_t id, const std::string& uri);
 
   bool IsRegistered(RTPExtensionType type) const;
-
-  int32_t GetType(uint8_t id, RTPExtensionType* type) const;
   // Return kInvalidType if not found.
-  RTPExtensionType GetType(uint8_t id) const;
-
-  int32_t GetId(const RTPExtensionType type, uint8_t* id) const;
+  RTPExtensionType GetType(uint8_t id) const { return types_[id]; }

[sprang_webrtc, 2016/11/08 16:13:37]

Maybe we should dcheck id.
Also, could we return an rtc::Optional<RTPExtensionType> instead? I like the
explicitness in that.

[danilchap, 2016/11/08 17:13:41]

Done.
I tried to add rtc::Optional, but it seems more complicated that way:
it force to constantly check  if value is valid: first inside GetId/GetType,
then when processing result of that functions. Sometimes that two times too many
(e.g. in rtp::Packet::IdentifyExtensions there is no need to check if the result
is valid type, invalid type just copied over and checked way later)

both id and type have natural value of 0 to represent invalid values.

To be more explicit one may use IsRegistered function.

[sprang_webrtc, 2016/11/08 19:19:57]

Fair enough. Thanks!

   // Return kInvalidId if not found.
-  uint8_t GetId(RTPExtensionType type) const;
-  size_t GetTotalLengthInBytes() const;
+  uint8_t GetId(RTPExtensionType type) const { return ids_[type]; }

[sprang_webrtc, 2016/11/08 16:13:37]

dito

 
-  void GetCopy(RtpHeaderExtensionMap* map) const;
+  size_t GetTotalLengthInBytes() const;
 
-  int32_t Size() const;
+  // TODO(danilchap): Remove use of the functions below.
+  void Erase() { *this = RtpHeaderExtensionMap(); }
+  int32_t Register(RTPExtensionType type, uint8_t id) {
+    return RegisterByType(id, type) ? 0 : -1;
+  }
+  int32_t Deregister(RTPExtensionType type);
+  int32_t GetType(uint8_t id, RTPExtensionType* type) const {
+    *type = GetType(id);
+    return *type == kInvalidType ? -1 : 0;
+  }
+  void GetCopy(RtpHeaderExtensionMap* copy) const { *copy = *this; }
 
  private:
-  std::map<uint8_t, HeaderExtension*> extensionMap_;
+  static constexpr uint8_t kMinId = 1;
+  static constexpr uint8_t kMaxId = 14;
+  bool Register(uint8_t id,
+                RTPExtensionType type,
+                size_t value_size,
+                const char* uri);
+
+  size_t total_values_size_bytes_ = 0;
+  RTPExtensionType types_[kMaxId + 1];
+  uint8_t ids_[kRtpExtensionNumberOfExtensions];
 };
 }  // namespace webrtc