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

Issue 2774463003: Don't hardcode MediaType::ANY in FakeNetworkPipe. (Closed)

Created:
3 years, 9 months ago by nisse-webrtc
Modified:
3 years, 8 months ago
CC:
webrtc-reviews_webrtc.org, video-team_agora.io, yujie_mao (webrtc), zhengzhonghou_agora.io, stefan-webrtc, tlegrand-webrtc, tterriberry_mozilla.com, mflodman
Target Ref:
refs/heads/master
Project:
webrtc
Visibility:
Public.

Description

Don't hardcode MediaType::ANY in FakeNetworkPipe. Instead let each test set the appropriate media type. This simplifies demuxing in Call and later in RtpTransportController. BUG=webrtc:7135 Review-Url: https://codereview.webrtc.org/2774463003 Cr-Commit-Position: refs/heads/master@{#17418} Committed: https://chromium.googlesource.com/external/webrtc/+/9c47b00e24da2941eb095df5a4459c6d98a8a88d

Patch Set 1 #

Total comments: 2

Patch Set 2 : Keep backwards compatible constructors for DirectTransport. #

Patch Set 3 : Use MediaType::VIDEO for packets injected by video/replay.cc. #

Patch Set 4 : Update FakeCall::DeliverPacket, for consistency with Call::DeliverRtp. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+128 lines, -75 lines) Patch
M webrtc/audio/test/low_bandwidth_audio_test.cc View 1 chunk +3 lines, -1 line 0 comments Download
M webrtc/call/bitrate_estimator_tests.cc View 1 chunk +4 lines, -2 lines 0 comments Download
M webrtc/call/call.cc View 4 chunks +5 lines, -3 lines 0 comments Download
M webrtc/call/call_perf_tests.cc View 4 chunks +7 lines, -2 lines 0 comments Download
M webrtc/call/rampup_tests.cc View 1 chunk +1 line, -0 lines 0 comments Download
M webrtc/media/engine/fakewebrtccall.cc View 1 2 3 1 chunk +5 lines, -4 lines 0 comments Download
M webrtc/test/call_test.h View 1 chunk +1 line, -0 lines 0 comments Download
M webrtc/test/call_test.cc View 1 chunk +2 lines, -0 lines 0 comments Download
M webrtc/test/direct_transport.h View 1 2 chunks +12 lines, -3 lines 0 comments Download
M webrtc/test/direct_transport.cc View 1 chunk +4 lines, -4 lines 0 comments Download
M webrtc/test/fake_network_pipe.h View 3 chunks +5 lines, -2 lines 0 comments Download
M webrtc/test/fake_network_pipe.cc View 2 chunks +6 lines, -3 lines 0 comments Download
M webrtc/test/fake_network_pipe_unittest.cc View 15 chunks +26 lines, -26 lines 0 comments Download
M webrtc/test/layer_filtering_transport.cc View 1 chunk +1 line, -1 line 0 comments Download
M webrtc/test/rtp_rtcp_observer.h View 1 chunk +2 lines, -1 line 0 comments Download
M webrtc/video/end_to_end_tests.cc View 14 chunks +34 lines, -17 lines 0 comments Download
M webrtc/video/replay.cc View 1 2 1 chunk +2 lines, -2 lines 0 comments Download
M webrtc/video/video_quality_test.cc View 1 chunk +2 lines, -1 line 0 comments Download
M webrtc/video/video_send_stream_tests.cc View 3 chunks +6 lines, -3 lines 0 comments Download

Messages

Total messages: 30 (16 generated)
nisse-webrtc
Hi, this change is intended to make it simpler to demux incoming RTP packets, and ...
3 years, 9 months ago (2017-03-23 13:47:21 UTC) #2
the sun
https://codereview.webrtc.org/2774463003/diff/1/webrtc/call/call.cc File webrtc/call/call.cc (right): https://codereview.webrtc.org/2774463003/diff/1/webrtc/call/call.cc#newcode1175 webrtc/call/call.cc:1175: RTC_DCHECK(media_type == MediaType::AUDIO || media_type == MediaType::VIDEO); With this ...
3 years, 9 months ago (2017-03-23 14:32:42 UTC) #3
nisse-webrtc
https://codereview.webrtc.org/2774463003/diff/1/webrtc/call/call.cc File webrtc/call/call.cc (right): https://codereview.webrtc.org/2774463003/diff/1/webrtc/call/call.cc#newcode1175 webrtc/call/call.cc:1175: RTC_DCHECK(media_type == MediaType::AUDIO || media_type == MediaType::VIDEO); On 2017/03/23 ...
3 years, 9 months ago (2017-03-23 15:27:29 UTC) #4
nisse-webrtc
+pbos: Do you know what to do with replay.cc? I could try just replacing MediaType::ANY ...
3 years, 9 months ago (2017-03-24 12:49:55 UTC) #6
pbos-webrtc
On 2017/03/24 12:49:55, nisse-webrtc wrote: > +pbos: Do you know what to do with replay.cc? ...
3 years, 9 months ago (2017-03-24 15:44:23 UTC) #7
nisse-webrtc
On 2017/03/24 15:44:23, pbos-webrtc wrote: > On 2017/03/24 12:49:55, nisse-webrtc wrote: > > +pbos: Do ...
3 years, 9 months ago (2017-03-27 10:52:02 UTC) #8
nisse-webrtc
Ping? Refactoring of receive side demux is blocked by this. If more testing is needed ...
3 years, 9 months ago (2017-03-27 11:57:25 UTC) #14
the sun
On 2017/03/27 11:57:25, nisse-webrtc wrote: > Ping? > > Refactoring of receive side demux is ...
3 years, 9 months ago (2017-03-27 14:54:47 UTC) #15
pbos-webrtc
On 2017/03/27 10:52:02, nisse-webrtc wrote: > On 2017/03/24 15:44:23, pbos-webrtc wrote: > > On 2017/03/24 ...
3 years, 9 months ago (2017-03-27 16:02:44 UTC) #16
nisse-webrtc
+sprang@. Can you have a look? OWNER's approval needed for video/.
3 years, 9 months ago (2017-03-28 07:13:00 UTC) #18
sprang_webrtc
lgtm
3 years, 8 months ago (2017-03-28 11:31:07 UTC) #23
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/2774463003/60001
3 years, 8 months ago (2017-03-28 11:57:08 UTC) #26
commit-bot: I haz the power
Committed patchset #4 (id:60001) as https://chromium.googlesource.com/external/webrtc/+/9c47b00e24da2941eb095df5a4459c6d98a8a88d
3 years, 8 months ago (2017-03-28 11:59:45 UTC) #29
lliuu
3 years, 8 months ago (2017-03-28 16:40:46 UTC) #30
Message was sent while issue was closed.
A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.webrtc.org/2784543002/ by lliuu@webrtc.org.

The reason for reverting is: Reverting since this seems to break multiple WebRTC
Perf buildbots.

Powered by Google App Engine
This is Rietveld 408576698