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

Issue 2122863002: TransportFeedback must be able to start with dropped packets. (Closed)

Created:
4 years, 5 months ago by sprang_webrtc
Modified:
4 years, 5 months ago
Reviewers:
stefan-webrtc
CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com, zhuangzesen_agora.io, danilchap, stefan-webrtc, mflodman
Base URL:
https://chromium.googlesource.com/external/webrtc.git@master
Target Ref:
refs/pending/heads/master
Project:
webrtc
Visibility:
Public.

Description

TransportFeedback must be able to start with dropped packets. A bug in the transpot feedback adapter causes new feedback message to always start with a received packet. This makes it impossible for the receiver to distinguish from actual dropped packets and dropped feedback messages. BUG=webrtc:6073 R=stefan@webrtc.org Committed: https://chromium.googlesource.com/external/webrtc/+/956ed71a11296bcb7f22b66dbc9492efb921985f

Patch Set 1 #

Total comments: 4

Patch Set 2 : Nit #

Unified diffs Side-by-side diffs Delta from patch set Stats (+63 lines, -14 lines) Patch
M webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy.cc View 4 chunks +20 lines, -13 lines 0 comments Download
M webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc View 2 chunks +35 lines, -0 lines 0 comments Download
M webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.cc View 1 1 chunk +5 lines, -1 line 0 comments Download
M webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback_unittest.cc View 1 chunk +3 lines, -0 lines 0 comments Download

Messages

Total messages: 11 (5 generated)
sprang_webrtc
4 years, 5 months ago (2016-07-05 09:10:26 UTC) #2
stefan-webrtc
lgtm https://codereview.webrtc.org/2122863002/diff/1/webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy.cc File webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy.cc (right): https://codereview.webrtc.org/2122863002/diff/1/webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy.cc#newcode137 webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy.cc:137: // of the first received packet in the ...
4 years, 5 months ago (2016-07-05 09:20:24 UTC) #3
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/2122863002/20001
4 years, 5 months ago (2016-07-05 09:45:02 UTC) #6
sprang_webrtc
https://codereview.webrtc.org/2122863002/diff/1/webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy.cc File webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy.cc (right): https://codereview.webrtc.org/2122863002/diff/1/webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy.cc#newcode137 webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy.cc:137: // of the first received packet in the feedback. ...
4 years, 5 months ago (2016-07-05 09:45:08 UTC) #7
commit-bot: I haz the power
Patchset 2 (id:??) landed as https://crrev.com/956ed71a11296bcb7f22b66dbc9492efb921985f Cr-Commit-Position: refs/heads/master@{#13381}
4 years, 5 months ago (2016-07-05 10:01:14 UTC) #9
sprang_webrtc
4 years, 5 months ago (2016-07-05 10:01:16 UTC) #11
Message was sent while issue was closed.
Committed patchset #2 (id:20001) manually as
956ed71a11296bcb7f22b66dbc9492efb921985f (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698