Simplify and extend RtpHeaderExtensionMap

webrtc/modules/rtp_rtcp/source/rtp_header_extension.h

 /*
  *  Copyright (c) 2012 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_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 {
 
 const uint16_t kRtpOneByteHeaderExtensionId = 0xBEDE;
 
 const size_t kRtpOneByteHeaderLength = 4;
 const size_t kTransmissionTimeOffsetLength = 4;
 const size_t kAudioLevelLength = 2;
 const size_t kAbsoluteSendTimeLength = 4;
 const size_t kVideoRotationLength = 2;
 const size_t kTransportSequenceNumberLength = 3;
 const size_t kPlayoutDelayLength = 4;
 
 // Playout delay in milliseconds. A playout delay limit (min or max)
 // has 12 bits allocated. This allows a range of 0-4095 values which translates
 // to a range of 0-40950 in milliseconds.
 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();
+  RtpHeaderExtensionMap(std::initializer_list<RtpExtension>);
 
-  void Erase();
-
-  int32_t Register(RTPExtensionType type, uint8_t id);
-
-  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;
+  // Return kInvalidType if not found.
+  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 { return ids_[type]; }

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

dito

 
-  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;
-  // Return kInvalidId if not found.
-  uint8_t GetId(RTPExtensionType type) const;
   size_t GetTotalLengthInBytes() const;
 
-  void GetCopy(RtpHeaderExtensionMap* map) 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
 
 #endif  // WEBRTC_MODULES_RTP_RTCP_SOURCE_RTP_HEADER_EXTENSION_H_