New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 1 user
Status: Assigned
Owner:
Cc:
Components:
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment
DCHECK fails in RTPSender::SendToNetwork
Project Member Reported by skvlad@webrtc.org, Dec 14 2016 Back to list
One of the continuous build bots has hit a DCHECK failure: 
https://build.chromium.org/p/client.webrtc/builders/Linux64%20Release%20%5Blarge%20tests%5D/builds/10009/steps/voe_auto_test/logs/stdio

#
# Fatal error in ../../webrtc/modules/rtp_rtcp/source/rtp_sender.cc, line 909
# last system error: 11
# Check failed: ssrc == SSRC() (1008712414 vs. 4859056)
# 
#
This has only happened once so far. Looks brandtr@ added the DCHECK recently - assigning the bug to him for initial investigation.

 
dcheck.txt
30.2 KB View Download
Project Member Comment 1 by brandtr@webrtc.org, Dec 14 2016
Status: Started
Project Member Comment 3 by brandtr@webrtc.org, Jan 4 2017
Project Member Comment 4 by brandtr@webrtc.org, Jan 9 2017
Reason for the DCHECK firing sometimes
==
If a collision between the remote SSRC (obtained from a received RTP packet) and
local SSRC is detected in the RTP module, a new SSRC is generated:
https://cs.chromium.org/chromium/src/third_party/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc?l=833

This happens in particular in the voe_auto_test suite, where my feeling is that
a single RTP module is sending data to itself (not 100% certain this is the
case). This leads to repeated SSRC collisions, but only the first one leads to a
new SSRC being generated.

Short-term fix
==
Removing the DCHECK, and replacing it with a TODO.

Long-term fix
==
- Make the SSRC immutable in the RTPSender. Reassigning to danilchap@ for this.
- Investigate whether the voe_auto_test could be improved.
Project Member Comment 5 by brandtr@webrtc.org, Jan 9 2017
Owner: danilchap@webrtc.org
Status: Assigned
Project Member Comment 6 by bugdroid1@chromium.org, Jan 9 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/external/webrtc.git/+/075c6d7f7ed6586f7ccdf5c3eed77b0b0afdd434

commit 075c6d7f7ed6586f7ccdf5c3eed77b0b0afdd434
Author: brandtr <brandtr@webrtc.org>
Date: Mon Jan 09 13:11:09 2017

Temporarily remove SSRC DCHECK in RTPSender::SendToNetwork.

Removing the DCHECK due to (sometimes) failing voe_auto_test.
Long-term, this DCHECK should be readded. Before that can happen,
the SSRC in the RTPSender should be made immutable.

TESTED=No failures when running third_party/gtest-parallel/gtest-parallel --repeat=5000 --gtest_filter="VolumeTest.ManualInputMutingMutesMicrophone" out/Debug/voe_auto_test.
BUG=webrtc:6887

Review-Url: https://codereview.webrtc.org/2610873002
Cr-Commit-Position: refs/heads/master@{#15962}

[modify] https://crrev.com/075c6d7f7ed6586f7ccdf5c3eed77b0b0afdd434/webrtc/modules/rtp_rtcp/source/rtp_sender.cc

Project Member Comment 7 by deadbeef@webrtc.org, Jan 30 2017
It looks like something similar is still happening:

#
# Fatal error in ../../webrtc/modules/rtp_rtcp/source/rtp_sender.cc, line 997
# last system error: 11
# Check failed: packet->Ssrc() == ssrc_ (1236638122 vs. 304373929)
# 
#

https://build.chromium.org/p/client.webrtc/builders/Linux64%20Release%20%5Blarge%20tests%5D/builds/10768/steps/voe_auto_test/logs/stdio
Project Member Comment 8 by bugdroid1@chromium.org, Feb 17 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/external/webrtc.git/+/b78d4d13835f628e722a57abae2bf06ba3655921

commit b78d4d13835f628e722a57abae2bf06ba3655921
Author: nisse <nisse@webrtc.org>
Date: Fri Feb 17 16:34:35 2017

Delete class SSRCDatabase, and its global ssrc registry,
and the method RTPSender::GenerateNewSSRC.

It's now mandatory for higher layers to call SetSSRC, RTPSender
no longer allocates any ssrc by default.

BUG= webrtc:4306 ,webrtc:6887

Review-Url: https://codereview.webrtc.org/2644303002
Cr-Commit-Position: refs/heads/master@{#16670}

[modify] https://crrev.com/b78d4d13835f628e722a57abae2bf06ba3655921/webrtc/logging/rtc_event_log/rtc_event_log.proto
[modify] https://crrev.com/b78d4d13835f628e722a57abae2bf06ba3655921/webrtc/modules/rtp_rtcp/BUILD.gn
[modify] https://crrev.com/b78d4d13835f628e722a57abae2bf06ba3655921/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
[modify] https://crrev.com/b78d4d13835f628e722a57abae2bf06ba3655921/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h
[modify] https://crrev.com/b78d4d13835f628e722a57abae2bf06ba3655921/webrtc/modules/rtp_rtcp/source/rtp_sender.cc
[modify] https://crrev.com/b78d4d13835f628e722a57abae2bf06ba3655921/webrtc/modules/rtp_rtcp/source/rtp_sender.h
[modify] https://crrev.com/b78d4d13835f628e722a57abae2bf06ba3655921/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc
[delete] https://crrev.com/1088248a4e6151dfbe8cc733fd771a79a80ad19d/webrtc/modules/rtp_rtcp/source/ssrc_database.cc
[delete] https://crrev.com/1088248a4e6151dfbe8cc733fd771a79a80ad19d/webrtc/modules/rtp_rtcp/source/ssrc_database.h
[modify] https://crrev.com/b78d4d13835f628e722a57abae2bf06ba3655921/webrtc/voice_engine/channel.cc
[modify] https://crrev.com/b78d4d13835f628e722a57abae2bf06ba3655921/webrtc/voice_engine/channel_manager.cc
[modify] https://crrev.com/b78d4d13835f628e722a57abae2bf06ba3655921/webrtc/voice_engine/channel_manager.h
[modify] https://crrev.com/b78d4d13835f628e722a57abae2bf06ba3655921/webrtc/voice_engine/test/auto_test/standard/rtp_rtcp_test.cc

Project Member Comment 9 by bugdroid1@chromium.org, Feb 18 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/external/webrtc.git/+/b5848ecbf5f7b310108546ec6b858fe93452f58e

commit b5848ecbf5f7b310108546ec6b858fe93452f58e
Author: kjellander <kjellander@google.com>
Date: Sat Feb 18 20:00:50 2017

Revert of Delete class SSRCDatabase, and its global ssrc registry. (patchset #20 id:370001 of https://codereview.webrtc.org/2644303002/ )

Reason for revert:
Breaks webrtc_perf_tests reliably:
https://build.chromium.org/p/client.webrtc.perf/builders/Android32%20Tests%20%28L%20Nexus5%29/builds/1780
https://build.chromium.org/p/client.webrtc.perf/builders/Android32%20Tests%20%28L%20Nexus4%29/builds/178

We're actively working on getting a quick version of webrtc_perf_tests up on the trybots again to prevent breakages like this: https://bugs.chromium.org/p/webrtc/issues/detail?id=7101

Original issue's description:
> Delete class SSRCDatabase, and its global ssrc registry,
> and the method RTPSender::GenerateNewSSRC.
>
> It's now mandatory for higher layers to call SetSSRC, RTPSender
> no longer allocates any ssrc by default.
>
> BUG= webrtc:4306 ,webrtc:6887
>
> Review-Url: https://codereview.webrtc.org/2644303002
> Cr-Commit-Position: refs/heads/master@{#16670}
> Committed: https://chromium.googlesource.com/external/webrtc/+/b78d4d13835f628e722a57abae2bf06ba3655921

TBR=solenberg@webrtc.org,stefan@webrtc.org,danilchap@webrtc.org,ivoc@webrtc.org,nisse@webrtc.org
NOTRY=True
BUG= webrtc:4306 ,webrtc:6887

Review-Url: https://codereview.webrtc.org/2700413002
Cr-Commit-Position: refs/heads/master@{#16693}

[modify] https://crrev.com/b5848ecbf5f7b310108546ec6b858fe93452f58e/webrtc/logging/rtc_event_log/rtc_event_log.proto
[modify] https://crrev.com/b5848ecbf5f7b310108546ec6b858fe93452f58e/webrtc/modules/rtp_rtcp/BUILD.gn
[modify] https://crrev.com/b5848ecbf5f7b310108546ec6b858fe93452f58e/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
[modify] https://crrev.com/b5848ecbf5f7b310108546ec6b858fe93452f58e/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h
[modify] https://crrev.com/b5848ecbf5f7b310108546ec6b858fe93452f58e/webrtc/modules/rtp_rtcp/source/rtp_sender.cc
[modify] https://crrev.com/b5848ecbf5f7b310108546ec6b858fe93452f58e/webrtc/modules/rtp_rtcp/source/rtp_sender.h
[modify] https://crrev.com/b5848ecbf5f7b310108546ec6b858fe93452f58e/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc
[add] https://crrev.com/b5848ecbf5f7b310108546ec6b858fe93452f58e/webrtc/modules/rtp_rtcp/source/ssrc_database.cc
[add] https://crrev.com/b5848ecbf5f7b310108546ec6b858fe93452f58e/webrtc/modules/rtp_rtcp/source/ssrc_database.h
[modify] https://crrev.com/b5848ecbf5f7b310108546ec6b858fe93452f58e/webrtc/voice_engine/channel.cc
[modify] https://crrev.com/b5848ecbf5f7b310108546ec6b858fe93452f58e/webrtc/voice_engine/channel_manager.cc
[modify] https://crrev.com/b5848ecbf5f7b310108546ec6b858fe93452f58e/webrtc/voice_engine/channel_manager.h
[modify] https://crrev.com/b5848ecbf5f7b310108546ec6b858fe93452f58e/webrtc/voice_engine/test/auto_test/standard/rtp_rtcp_test.cc

Project Member Comment 10 by bugdroid1@chromium.org, Feb 21 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/external/webrtc.git/+/7d59f6b1c46cf551d2b55849ed061756ba25f1b0

commit 7d59f6b1c46cf551d2b55849ed061756ba25f1b0
Author: nisse <nisse@webrtc.org>
Date: Tue Feb 21 11:40:24 2017

Reland of Delete class SSRCDatabase, and its global ssrc registry. (patchset #1 id:1 of https://codereview.webrtc.org/2700413002/ )

Reason for revert:
Intend to fix perf problem and reland.

Original issue's description:
> Revert of Delete class SSRCDatabase, and its global ssrc registry. (patchset #20 id:370001 of https://codereview.webrtc.org/2644303002/ )
>
> Reason for revert:
> Breaks webrtc_perf_tests reliably:
> https://build.chromium.org/p/client.webrtc.perf/builders/Android32%20Tests%20%28L%20Nexus5%29/builds/1780
> https://build.chromium.org/p/client.webrtc.perf/builders/Android32%20Tests%20%28L%20Nexus4%29/builds/178
>
> We're actively working on getting a quick version of webrtc_perf_tests up on the trybots again to prevent breakages like this: https://bugs.chromium.org/p/webrtc/issues/detail?id=7101
>
> Original issue's description:
> > Delete class SSRCDatabase, and its global ssrc registry,
> > and the method RTPSender::GenerateNewSSRC.
> >
> > It's now mandatory for higher layers to call SetSSRC, RTPSender
> > no longer allocates any ssrc by default.
> >
> > BUG= webrtc:4306 ,webrtc:6887
> >
> > Review-Url: https://codereview.webrtc.org/2644303002
> > Cr-Commit-Position: refs/heads/master@{#16670}
> > Committed: https://chromium.googlesource.com/external/webrtc/+/b78d4d13835f628e722a57abae2bf06ba3655921
>
> TBR=solenberg@webrtc.org,stefan@webrtc.org,danilchap@webrtc.org,ivoc@webrtc.org,nisse@webrtc.org
> NOTRY=True
> BUG= webrtc:4306 ,webrtc:6887
>
> Review-Url: https://codereview.webrtc.org/2700413002
> Cr-Commit-Position: refs/heads/master@{#16693}
> Committed: https://chromium.googlesource.com/external/webrtc/+/b5848ecbf5f7b310108546ec6b858fe93452f58e

TBR=solenberg@webrtc.org,stefan@webrtc.org,danilchap@webrtc.org,ivoc@webrtc.org,kjellander@webrtc.org,kjellander@google.com
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= webrtc:4306 ,webrtc:6887

Review-Url: https://codereview.webrtc.org/2702203002
Cr-Commit-Position: refs/heads/master@{#16737}

[modify] https://crrev.com/7d59f6b1c46cf551d2b55849ed061756ba25f1b0/webrtc/logging/rtc_event_log/rtc_event_log.proto
[modify] https://crrev.com/7d59f6b1c46cf551d2b55849ed061756ba25f1b0/webrtc/modules/rtp_rtcp/BUILD.gn
[modify] https://crrev.com/7d59f6b1c46cf551d2b55849ed061756ba25f1b0/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
[modify] https://crrev.com/7d59f6b1c46cf551d2b55849ed061756ba25f1b0/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h
[modify] https://crrev.com/7d59f6b1c46cf551d2b55849ed061756ba25f1b0/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc
[modify] https://crrev.com/7d59f6b1c46cf551d2b55849ed061756ba25f1b0/webrtc/modules/rtp_rtcp/source/rtp_sender.cc
[modify] https://crrev.com/7d59f6b1c46cf551d2b55849ed061756ba25f1b0/webrtc/modules/rtp_rtcp/source/rtp_sender.h
[modify] https://crrev.com/7d59f6b1c46cf551d2b55849ed061756ba25f1b0/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc
[delete] https://crrev.com/531100dc7a9c58d8cbe200cf3a65d1254070861d/webrtc/modules/rtp_rtcp/source/ssrc_database.cc
[delete] https://crrev.com/531100dc7a9c58d8cbe200cf3a65d1254070861d/webrtc/modules/rtp_rtcp/source/ssrc_database.h
[modify] https://crrev.com/7d59f6b1c46cf551d2b55849ed061756ba25f1b0/webrtc/modules/rtp_rtcp/test/testAPI/test_api.cc
[modify] https://crrev.com/7d59f6b1c46cf551d2b55849ed061756ba25f1b0/webrtc/voice_engine/channel.cc
[modify] https://crrev.com/7d59f6b1c46cf551d2b55849ed061756ba25f1b0/webrtc/voice_engine/channel_manager.cc
[modify] https://crrev.com/7d59f6b1c46cf551d2b55849ed061756ba25f1b0/webrtc/voice_engine/channel_manager.h
[modify] https://crrev.com/7d59f6b1c46cf551d2b55849ed061756ba25f1b0/webrtc/voice_engine/test/auto_test/standard/rtp_rtcp_test.cc

Project Member Comment 11 by danilchap@webrtc.org, Apr 21 2017
Components: -Video Network>RTP
Sign in to add a comment