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

Issue 2554003002: Make ostream<< for enum class H264PacketizationMode (Closed)

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

Description

Make ostream<< for enum class H264PacketizationMode This makes it possible to use << and RTC_CHECK_EQ with this class. BUG=none Committed: https://crrev.com/ac382f3adcf64b31e594ec1c0ad427cfc031bf14 Cr-Commit-Position: refs/heads/master@{#15456}

Patch Set 1 #

Total comments: 3

Patch Set 2 : Added TODO and bug #

Patch Set 3 : Swap RTC_CHECK_EQ arguments #

Total comments: 1

Patch Set 4 : Fix bug number #

Unified diffs Side-by-side diffs Delta from patch set Stats (+19 lines, -6 lines) Patch
M webrtc/modules/include/module_common_types.h View 1 2 3 1 chunk +16 lines, -0 lines 0 comments Download
M webrtc/modules/rtp_rtcp/source/rtp_format_h264.cc View 1 2 2 chunks +3 lines, -6 lines 0 comments Download

Messages

Total messages: 26 (12 generated)
hta-webrtc
Review & CC to the people who commented on this in CL 2528343002.
4 years ago (2016-12-06 11:12:44 UTC) #3
hta-webrtc
Adding kwiberg for OWNER approval
4 years ago (2016-12-06 11:14:50 UTC) #5
sprang_webrtc
https://codereview.webrtc.org/2554003002/diff/1/webrtc/modules/include/module_common_types.h File webrtc/modules/include/module_common_types.h (right): https://codereview.webrtc.org/2554003002/diff/1/webrtc/modules/include/module_common_types.h#newcode275 webrtc/modules/include/module_common_types.h:275: // .cc file it should belong to. Could you ...
4 years ago (2016-12-06 11:32:55 UTC) #6
kwiberg-webrtc
https://codereview.webrtc.org/2554003002/diff/1/webrtc/modules/include/module_common_types.h File webrtc/modules/include/module_common_types.h (right): https://codereview.webrtc.org/2554003002/diff/1/webrtc/modules/include/module_common_types.h#newcode275 webrtc/modules/include/module_common_types.h:275: // .cc file it should belong to. I'd really ...
4 years ago (2016-12-06 11:40:19 UTC) #7
hta-webrtc
On 2016/12/06 11:40:19, kwiberg-webrtc wrote: > https://codereview.webrtc.org/2554003002/diff/1/webrtc/modules/include/module_common_types.h > File webrtc/modules/include/module_common_types.h (right): > > https://codereview.webrtc.org/2554003002/diff/1/webrtc/modules/include/module_common_types.h#newcode275 > ...
4 years ago (2016-12-06 12:01:01 UTC) #8
kwiberg-webrtc
On 2016/12/06 12:01:01, hta-webrtc wrote: > On 2016/12/06 11:40:19, kwiberg-webrtc wrote: > > > https://codereview.webrtc.org/2554003002/diff/1/webrtc/modules/include/module_common_types.h ...
4 years ago (2016-12-06 12:08:43 UTC) #9
magjed_webrtc
lgtm https://codereview.webrtc.org/2554003002/diff/1/webrtc/modules/rtp_rtcp/source/rtp_format_h264.cc File webrtc/modules/rtp_rtcp/source/rtp_format_h264.cc (right): https://codereview.webrtc.org/2554003002/diff/1/webrtc/modules/rtp_rtcp/source/rtp_format_h264.cc#newcode272 webrtc/modules/rtp_rtcp/source/rtp_format_h264.cc:272: RTC_CHECK_EQ(packetization_mode_, H264PacketizationMode::NonInterleaved); nit: You should swap the arguments, ...
4 years ago (2016-12-06 15:31:34 UTC) #10
hta-webrtc
The TODO for refactoring has been added, and the bug filed. Does this CL seem ...
4 years ago (2016-12-06 22:06:52 UTC) #13
kwiberg-webrtc
lgtm, provided you point to the right bug https://codereview.webrtc.org/2554003002/diff/40001/webrtc/modules/include/module_common_types.h File webrtc/modules/include/module_common_types.h (right): https://codereview.webrtc.org/2554003002/diff/40001/webrtc/modules/include/module_common_types.h#newcode276 webrtc/modules/include/module_common_types.h:276: // ...
4 years ago (2016-12-06 22:16:10 UTC) #14
hta-webrtc
On 2016/12/06 22:16:10, kwiberg-webrtc wrote: > lgtm, provided you point to the right bug > ...
4 years ago (2016-12-07 07:20:33 UTC) #17
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/2554003002/60001
4 years ago (2016-12-07 07:20:40 UTC) #20
commit-bot: I haz the power
Committed patchset #4 (id:60001)
4 years ago (2016-12-07 07:43:54 UTC) #23
commit-bot: I haz the power
Patchset 4 (id:??) landed as https://crrev.com/ac382f3adcf64b31e594ec1c0ad427cfc031bf14 Cr-Commit-Position: refs/heads/master@{#15456}
4 years ago (2016-12-07 07:44:09 UTC) #25
sprang_webrtc
4 years ago (2016-12-07 08:46:45 UTC) #26
Message was sent while issue was closed.
lgtm

Powered by Google App Engine
This is Rietveld 408576698