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

Issue 3012963002: Reland of Use RtxReceiveStream. (Closed)

Created:
3 years, 3 months ago by nisse-webrtc
Modified:
3 years, 3 months ago
CC:
webrtc-reviews_webrtc.org, interface-changes_webrtc.org, video-team_agora.io, yujie_mao (webrtc), zhengzhonghou_agora.io, tterriberry_mozilla.com, qiang.lu, niklas.enbom, peah-webrtc, the sun, mflodman
Target Ref:
refs/heads/master
Project:
webrtc
Visibility:
Public.

Description

Reland of Use RtxReceiveStream. (patchset #1 id:1 of https://codereview.webrtc.org/3007303002/ ) Reason for revert: Identified a configuration problem in the video quality tests. Intend to fix and reland. Original issue's description: > Revert of Use RtxReceiveStream. (patchset #5 id:320001 of https://codereview.webrtc.org/3006063002/ ) > > Reason for revert: > This change appears to break ulpfec, with severe regressions, e.g., for webrtc_perf_test FullStackTest.ForemanCifPlr5Ulpfec > > Original issue's description: > > Reland of Use RtxReceiveStream. (patchset #1 id:1 of https://codereview.webrtc.org/3010983002/ ) > > > > Reason for revert: > > Intend to fix perf failures and reland. > > > > Original issue's description: > > > Revert of Use RtxReceiveStream. (patchset #5 id:80001 of https://codereview.webrtc.org/3008773002/ ) > > > > > > Reason for revert: > > > A few perf tests broken, including > > > > > > RampUpTest.UpDownUpAbsSendTimeSimulcastRedRtx > > > RampUpTest.UpDownUpTransportSequenceNumberRtx > > > RampUpTest.UpDownUpTransportSequenceNumberPacketLoss > > > > > > > > > Original issue's description: > > > > Use RtxReceiveStream. > > > > > > > > This also has the beneficial side-effect that when a media stream > > > > which is protected by FlexFEC receives an RTX retransmission, the > > > > retransmitted media packet is passed into the FlexFEC machinery, > > > > which should improve its ability to recover packets via FEC. > > > > > > > > BUG=webrtc:7135 > > > > > > > > Review-Url: https://codereview.webrtc.org/3008773002 > > > > Cr-Commit-Position: refs/heads/master@{#19649} > > > > Committed: https://chromium.googlesource.com/external/webrtc/+/5c0f6c62ea3b1d2c43f8fc152961af27033475f7 > > > > > > TBR=brandtr@webrtc.org,danilchap@webrtc.org,stefan@webrtc.org,magjed@webrtc.org > > > # Skipping CQ checks because original CL landed less than 1 days ago. > > > NOPRESUBMIT=true > > > NOTREECHECKS=true > > > NOTRY=true > > > BUG=webrtc:7135 > > > > > > Review-Url: https://codereview.webrtc.org/3010983002 > > > Cr-Commit-Position: refs/heads/master@{#19653} > > > Committed: https://chromium.googlesource.com/external/webrtc/+/3c39c0137afa274d1d524b150b50304b38a2847b > > > > TBR=brandtr@webrtc.org,danilchap@webrtc.org,stefan@webrtc.org,magjed@webrtc.org > > # Skipping CQ checks because original CL landed less than 1 days ago. > > NOPRESUBMIT=true > > NOTREECHECKS=true > > BUG=webrtc:7135 > > > > Review-Url: https://codereview.webrtc.org/3006063002 > > Cr-Commit-Position: refs/heads/master@{#19715} > > Committed: https://chromium.googlesource.com/external/webrtc/+/35713eaf565c0fef07c8afc158d7b8fdf7ec3d78 > > TBR=brandtr@webrtc.org,danilchap@webrtc.org,stefan@webrtc.org,magjed@webrtc.org > # Not skipping CQ checks because original CL landed more than 1 days ago. > BUG=webrtc:7135 > > Review-Url: https://codereview.webrtc.org/3007303002 > Cr-Commit-Position: refs/heads/master@{#19744} > Committed: https://chromium.googlesource.com/external/webrtc/+/8e7eee035178a7f10e19883681b5eaa4a7523107 TBR=brandtr@webrtc.org,danilchap@webrtc.org,stefan@webrtc.org,magjed@webrtc.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG=webrtc:7135 Review-Url: https://codereview.webrtc.org/3012963002 Cr-Commit-Position: refs/heads/master@{#19765} Committed: https://chromium.googlesource.com/external/webrtc/+/ca5706d8b5576ba48eb34a7b4366a2aa6f164d0e

Patch Set 1 #

Patch Set 2 : Fix reland conflicts. #

Patch Set 3 : Fix receive stream configuration in video quality test. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+120 lines, -68 lines) Patch
M webrtc/call/rampup_tests.cc View 1 1 chunk +3 lines, -2 lines 0 comments Download
M webrtc/call/rtx_receive_stream.h View 1 2 chunks +9 lines, -1 line 0 comments Download
M webrtc/call/rtx_receive_stream.cc View 1 2 chunks +14 lines, -4 lines 0 comments Download
M webrtc/call/video_receive_stream.h View 1 1 chunk +5 lines, -0 lines 0 comments Download
M webrtc/media/engine/webrtcvideoengine.cc View 1 1 chunk +5 lines, -0 lines 0 comments Download
M webrtc/media/engine/webrtcvideoengine_unittest.cc View 1 5 chunks +41 lines, -13 lines 0 comments Download
M webrtc/video/BUILD.gn View 1 1 chunk +3 lines, -0 lines 0 comments Download
M webrtc/video/end_to_end_tests.cc View 1 1 chunk +3 lines, -1 line 0 comments Download
M webrtc/video/rtp_video_stream_receiver.h View 1 2 chunks +2 lines, -3 lines 0 comments Download
M webrtc/video/rtp_video_stream_receiver.cc View 1 5 chunks +5 lines, -38 lines 0 comments Download
M webrtc/video/rtp_video_stream_receiver_unittest.cc View 1 2 chunks +7 lines, -3 lines 0 comments Download
M webrtc/video/video_quality_test.cc View 1 2 2 chunks +7 lines, -0 lines 0 comments Download
M webrtc/video/video_receive_stream.h View 1 3 chunks +6 lines, -0 lines 0 comments Download
M webrtc/video/video_receive_stream.cc View 1 6 chunks +10 lines, -3 lines 0 comments Download

Messages

Total messages: 11 (6 generated)
nisse-webrtc
Created Reland of Use RtxReceiveStream.
3 years, 3 months ago (2017-09-11 07:06:50 UTC) #1
nisse-webrtc
With the fix in ps#3, number of dropped frames in FullStackTest.ForemanCifPlr5Ulpfec (webrtc_perf_tests)goes back from 378 ...
3 years, 3 months ago (2017-09-11 07:20:34 UTC) #2
brandtr
fix in ps#3 lgtm
3 years, 3 months ago (2017-09-11 08:52:06 UTC) #5
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/3012963002/280001
3 years, 3 months ago (2017-09-11 09:01:34 UTC) #8
commit-bot: I haz the power
3 years, 3 months ago (2017-09-11 09:32:24 UTC) #11
Message was sent while issue was closed.
Committed patchset #3 (id:280001) as
https://chromium.googlesource.com/external/webrtc/+/ca5706d8b5576ba48eb34a7b4...

Powered by Google App Engine
This is Rietveld 408576698