Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(282)

Issue 2452293004: Simplify and extend RtpHeaderExtensionMap (Closed)

Created:
4 years, 1 month ago by danilchap
Modified:
4 years, 1 month ago
Reviewers:
sprang_webrtc
CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com, zhuangzesen_agora.io, danilchap, stefan-webrtc, mflodman, brandtr
Target Ref:
refs/pending/heads/master
Project:
webrtc
Visibility:
Public.

Description

Simplify and extend RtpHeaderExtensionMap Add register functions for various codepaths. Add initialize-list constructor to create usable const RtpHeaderExtensionMap Optimize implementation for GetId/GetType. BUG=webrtc:1994 Committed: https://crrev.com/d1d26fbeb37a69471a34004c6ac2d3fafde5d404 Cr-Commit-Position: refs/heads/master@{#14986}

Patch Set 1 #

Patch Set 2 : ... #

Patch Set 3 : work-around windows compiler #

Patch Set 4 : nits #

Patch Set 5 : Add initializer-list constructor #

Patch Set 6 : -test of (now) autogenerated code #

Patch Set 7 : added ugly hacks for backward compatability with bad tests #

Patch Set 8 : more nits #

Patch Set 9 : revert to runtime check of id instead of assert #

Patch Set 10 : Rebase #

Patch Set 11 : make kExtensions constexpr #

Total comments: 6

Patch Set 12 : Add DCHECKs to GetId/GetType #

Unified diffs Side-by-side diffs Delta from patch set Stats (+241 lines, -243 lines) Patch
M webrtc/modules/rtp_rtcp/source/rtp_header_extension.h View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +49 lines, -57 lines 0 comments Download
M webrtc/modules/rtp_rtcp/source/rtp_header_extension.cc View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +107 lines, -112 lines 0 comments Download
M webrtc/modules/rtp_rtcp/source/rtp_header_extension_unittest.cc View 1 2 3 4 5 6 7 8 1 chunk +68 lines, -56 lines 0 comments Download
M webrtc/modules/rtp_rtcp/source/rtp_header_extensions.h View 1 2 3 4 5 6 7 8 9 10 6 chunks +11 lines, -7 lines 0 comments Download
M webrtc/modules/rtp_rtcp/source/rtp_header_extensions.cc View 1 2 3 4 5 6 7 8 9 10 11 6 chunks +6 lines, -11 lines 0 comments Download

Messages

Total messages: 20 (12 generated)
danilchap
4 years, 1 month ago (2016-10-31 11:51:12 UTC) #10
sprang_webrtc
https://codereview.webrtc.org/2452293004/diff/240001/webrtc/modules/rtp_rtcp/source/rtp_header_extension.h File webrtc/modules/rtp_rtcp/source/rtp_header_extension.h (right): https://codereview.webrtc.org/2452293004/diff/240001/webrtc/modules/rtp_rtcp/source/rtp_header_extension.h#newcode57 webrtc/modules/rtp_rtcp/source/rtp_header_extension.h:57: RTPExtensionType GetType(uint8_t id) const { return types_[id]; } Maybe ...
4 years, 1 month ago (2016-11-08 16:13:37 UTC) #11
danilchap
https://codereview.webrtc.org/2452293004/diff/240001/webrtc/modules/rtp_rtcp/source/rtp_header_extension.h File webrtc/modules/rtp_rtcp/source/rtp_header_extension.h (right): https://codereview.webrtc.org/2452293004/diff/240001/webrtc/modules/rtp_rtcp/source/rtp_header_extension.h#newcode57 webrtc/modules/rtp_rtcp/source/rtp_header_extension.h:57: RTPExtensionType GetType(uint8_t id) const { return types_[id]; } On ...
4 years, 1 month ago (2016-11-08 17:13:41 UTC) #12
sprang_webrtc
lgtm https://codereview.webrtc.org/2452293004/diff/240001/webrtc/modules/rtp_rtcp/source/rtp_header_extension.h File webrtc/modules/rtp_rtcp/source/rtp_header_extension.h (right): https://codereview.webrtc.org/2452293004/diff/240001/webrtc/modules/rtp_rtcp/source/rtp_header_extension.h#newcode57 webrtc/modules/rtp_rtcp/source/rtp_header_extension.h:57: RTPExtensionType GetType(uint8_t id) const { return types_[id]; } ...
4 years, 1 month ago (2016-11-08 19:19:57 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/2452293004/260001
4 years, 1 month ago (2016-11-08 19:49:32 UTC) #15
commit-bot: I haz the power
Committed patchset #12 (id:260001)
4 years, 1 month ago (2016-11-08 20:20:47 UTC) #17
commit-bot: I haz the power
Patchset 12 (id:??) landed as https://crrev.com/d1d26fbeb37a69471a34004c6ac2d3fafde5d404 Cr-Commit-Position: refs/heads/master@{#14986}
4 years, 1 month ago (2016-11-08 20:40:09 UTC) #19
danilchap
4 years, 1 month ago (2016-11-08 21:36:25 UTC) #20
Message was sent while issue was closed.
A revert of this CL (patchset #12 id:260001) has been created in
https://codereview.webrtc.org/2484863007/ by danilchap@webrtc.org.

The reason for reverting is: breaks downstream project.

Powered by Google App Engine
This is Rietveld 408576698