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

Issue 2919313005: Revert of Only compare sequence numbers from the same SSRC in ForwardErrorCorrection. (Closed)

Created:
3 years, 6 months ago by brandtr
Modified:
3 years, 6 months ago
Reviewers:
holmer, stefan-webrtc
CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com, zhuangzesen_agora.io, danilchap, mflodman
Target Ref:
refs/heads/master
Project:
webrtc
Visibility:
Public.

Description

Revert of Only compare sequence numbers from the same SSRC in ForwardErrorCorrection. (patchset #5 id:120001 of https://codereview.webrtc.org/2893293003/ ) Reason for revert: Breaks fuzzer. Original issue's description: > Only compare sequence numbers from the same SSRC in ForwardErrorCorrection. > > Prior to this CL, the ForwardErrorCorrection state would be reset whenever > the difference in sequence numbers of the last recovered media packet > and the new packet (media or FEC) was too large. This comparison did not > take into account that FlexFEC uses a different SSRC for the FEC packets, > meaning that the the state would be reset very frequently when FlexFEC > is used. This should not have led to any major problems, except for a > decreased decoding efficiency. > > This CL verifies that whenever we compare sequence numbers in > ForwardErrorCorrection, they do indeed belong to the same SSRC. > > BUG=webrtc:5654 > > Review-Url: https://codereview.webrtc.org/2893293003 > Cr-Commit-Position: refs/heads/master@{#18399} > Committed: https://chromium.googlesource.com/external/webrtc/+/1476a9d789db03457595cf7dbea7e362972f2a4d TBR=stefan@webrtc.org,holmer@google.com # Not skipping CQ checks because original CL landed more than 1 days ago. BUG=webrtc:5654 Review-Url: https://codereview.webrtc.org/2919313005 Cr-Commit-Position: refs/heads/master@{#18446} Committed: https://chromium.googlesource.com/external/webrtc/+/92732ecc5ccc81e17c1dea75ecc5e34c7ff6274f

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+112 lines, -196 lines) Patch
M webrtc/modules/rtp_rtcp/source/fec_test_helper.h View 2 chunks +2 lines, -2 lines 0 comments Download
M webrtc/modules/rtp_rtcp/source/fec_test_helper.cc View 2 chunks +3 lines, -3 lines 0 comments Download
M webrtc/modules/rtp_rtcp/source/forward_error_correction.h View 3 chunks +9 lines, -5 lines 0 comments Download
M webrtc/modules/rtp_rtcp/source/forward_error_correction.cc View 9 chunks +27 lines, -56 lines 0 comments Download
M webrtc/modules/rtp_rtcp/source/rtp_fec_unittest.cc View 7 chunks +58 lines, -90 lines 0 comments Download
M webrtc/modules/rtp_rtcp/source/ulpfec_receiver_impl.cc View 2 chunks +0 lines, -2 lines 0 comments Download
M webrtc/modules/rtp_rtcp/test/testFec/test_fec.cc View 7 chunks +13 lines, -38 lines 0 comments Download

Messages

Total messages: 7 (3 generated)
brandtr
Created Revert of Only compare sequence numbers from the same SSRC in ForwardErrorCorrection.
3 years, 6 months ago (2017-06-05 13:58:20 UTC) #2
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/2919313005/1
3 years, 6 months ago (2017-06-05 13:58:28 UTC) #3
commit-bot: I haz the power
Committed patchset #1 (id:1) as https://chromium.googlesource.com/external/webrtc/+/92732ecc5ccc81e17c1dea75ecc5e34c7ff6274f
3 years, 6 months ago (2017-06-05 14:25:06 UTC) #6
brandtr
3 years, 6 months ago (2017-06-05 14:27:40 UTC) #7
Message was sent while issue was closed.
A revert of this CL (patchset #1 id:1) has been created in
https://codereview.webrtc.org/2918333002/ by brandtr@webrtc.org.

The reason for reverting is: Do not accept packets with incorrect SSRC..

Powered by Google App Engine
This is Rietveld 408576698