Issue metadata
Sign in to add a comment
|
WebRTC TestPacketBuffer.SeqNumWrapOneFrame failing due to clang update
Reported by
yvesg@webrtc.org,
Sep 20
|
||||||||||||||||||||||
Issue descriptionSteps to reproduce the problem: On android build either dbg or rel, run: % ./modules_unittests --gtest_filter='TestPacketBuffer.SeqNumWrapOneFrame' What is the expected behavior? Test passing. What went wrong? [FAIL] TestPacketBuffer.SeqNumWrapOneFrame: [ RUN ] TestPacketBuffer.SeqNumWrapOneFrame Main ../../modules/video_coding/video_packet_buffer_unittest.cc:80: Failure Value of: frame_it == frames_from_callback_.end() Actual: true Expected: false Could not find frame with first sequence number 65535. [ FAILED ] TestPacketBuffer.SeqNumWrapOneFrame (0 ms) [----------] 1 test from TestPacketBuffer (0 ms total) Did this work before? Yes d757e00e8cfe98f8c5ea8c7b710e2b1a569c4a1d Chrome version: 69.0.3497.92 Channel: n/a OS Version: Flash Version: You can see the failure: https://webrtc-review.googlesource.com/c/src/+/101104 I have manually checked it was introduced by this clang roll: https://chromium.googlesource.com/chromium/src/tools/+/1ee31b34bd46a000286f808662c2c051d41e2d48 The test currently passes thanks to the revert: https://chromium.googlesource.com/chromium/src/+/f6a8902e39eecb5d7a0be8dc61f432e451213023
,
Sep 20
Why do these tests not run on the chromium cq?
,
Sep 20
Also, chromium tests pass, any chance this is due to a code bug in the test that's just exposed by the new compiler?
,
Sep 20
,
Sep 20
> Why do these tests not run on the chromium cq? I guess because these are WebRTC own tests. > Any chance this is due to a code bug in the test that's just exposed by the new compiler? Yes that's what I've thought at first. Then I found this less likely after a quick inspection and considering nothing suspicious is detected by the compiler or the sanitizers. +philipel@ Could you double-check that? Also, in the meantime, if the tests are non critical, we might want to disable them to unblock the auto-roller.
,
Sep 21
I managed to boil down the issue to this simple test case:
uint16_t a = 0xFFFF;
uint16_t b = 0;
TEST_F(TestPacketBuffer, UInt16SanityCheck) {
if ( a != static_cast<uint16_t>(b-1) ) {
// Huge fail, shouldn't reach this point.
EXPECT_TRUE(0);
} else {
EXPECT_TRUE(1);
}
}
In my book, that's a compiler bug :)
See this example live here: https://webrtc-review.googlesource.com/c/src/+/101400
,
Sep 21
Urgh :-( What compiler flags does webrtc use? -Oz or -O2?
,
Sep 21
For Android that's -Oz. Size matters. Here are the flags used to compile the tests (video_packet_buffer_unittest.cc) defines = -DV8_DEPRECATION_WARNINGS -DDCHECK_ALWAYS_ON=1 -DNO_TCMALLOC -DSAFE_BROWSING_DB_REMOTE -DCHROMIUM_BUILD -DFIELDTRIAL_TESTING_ENABLED -D_GNU_SOURCE -DANDROID -DHAVE_SYS_UIO_H -DANDROID_NDK_VERSION_ROLL=r16_1 -DCR_CLANG_REVISION=\"340925-2\" -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D_FORTIFY_SOURCE=2 -D__GNU_SOURCE=1 -DCHROMIUM_CXX_TWEAK_INLINES -DNDEBUG -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DWEBRTC_ENABLE_PROTOBUF=1 -DWEBRTC_INCLUDE_INTERNAL_AUDIO_DEVICE -DHAVE_SCTP -DUSE_BUILTIN_SW_CODECS -DWEBRTC_ARCH_ARM -DWEBRTC_ARCH_ARM_V7 -DWEBRTC_HAS_NEON -DWEBRTC_LIBRARY_IMPL -DWEBRTC_NON_STATIC_TRACE_EVENT_HANDLERS=1 -DWEBRTC_POSIX -DWEBRTC_LINUX -DWEBRTC_ANDROID -DABSL_ALLOCATOR_NOTHROW=1 -DGTEST_API_= -DGTEST_HAS_POSIX_RE=0 -DGTEST_LANG_CXX11=1 -DGTEST_HAS_TR1_TUPLE=0 -DGTEST_HAS_ABSL=1 -DUNIT_TEST -DUNIT_TEST include_dirs = -I../.. -Igen -I../../third_party/abseil-cpp -I../../third_party/libyuv/include -I../../third_party/googletest/custom -I../../third_party/googletest/src/googlemock/include -I../../third_party/googletest/custom -I../../third_party/googletest/src/googletest/include -I../../third_party/libvpx/source/libvpx cflags = -fno-strict-aliasing --param=ssp-buffer-size=4 -fstack-protector -Wno-builtin-macro-redefined -D__DATE__= -D__TIME__= -D__TIMESTAMP__= -funwind-tables -fPIC -fcolor-diagnostics -fmerge-all-constants -no-canonical-prefixes -ffunction-sections -fno-short-enums --target=arm-linux-androideabi -isystem../../third_party/android_ndk/sysroot/usr/include/arm-linux-androideabi -D__ANDROID_API__=16 -DHAVE_PTHREAD_COND_TIMEDWAIT_MONOTONIC=1 -march=armv7-a -mfloat-abi=softfp -mtune=generic-armv7-a -mfpu=neon -mthumb -Wall -Werror -Wextra -Wimplicit-fallthrough -Wthread-safety -Wno-missing-field-initializers -Wno-unused-parameter -Wno-c++11-narrowing -Wno-covered-switch-default -Wno-unneeded-internal-declaration -Wno-undefined-var-template -Wno-nonportable-include-path -Wno-user-defined-warnings -Wno-unused-lambda-capture -Wno-null-pointer-arithmetic -Wno-enum-compare-switch -Wno-ignored-pragma-optimize -Oz -fno-ident -fdata-sections -ffunction-sections -fomit-frame-pointer -gdwarf-3 -g1 -fdebug-info-for-profiling -fvisibility=hidden -Wheader-hygiene -Wstring-conversion -Wtautological-overlap-compare -Wextra -Wno-unused-parameter -Wno-missing-field-initializers -Wc++11-narrowing -Wimplicit-fallthrough -Wthread-safety -Winconsistent-missing-override -Wundef -Wunused-lambda-capture -Wno-inconsistent-missing-override cflags_cc = -std=c++14 -fno-exceptions -fno-rtti --sysroot=../../third_party/android_ndk/sysroot -isystem../../third_party/android_ndk/sources/cxx-stl/llvm-libc++/include -isystem../../third_party/android_ndk/sources/cxx-stl/llvm-libc++abi/include -isystem../../third_party/android_ndk/sources/android/support/include -fvisibility-inlines-hidden -Wnon-virtual-dtor -Woverloaded-virtual
,
Sep 24
,
Sep 24
I'm looking at what changed in Clang now.
,
Sep 24
The following revision refers to this bug: https://webrtc.googlesource.com/src.git/+/03c592a1e9f9dbad02bfc9d1f55d8b8c5c499208 commit 03c592a1e9f9dbad02bfc9d1f55d8b8c5c499208 Author: Artem Titarenko <artit@webrtc.org> Date: Mon Sep 24 11:12:37 2018 Disabled TestPacketBuffer.SeqNumWrapOneFrame test due to clang update Until further investigation. Clang update: chromium:880827 Bug: chromium:887464 Change-Id: Id1fe85a013920e6ae8c6ac69efb0a0502b9dd6fe Reviewed-on: https://webrtc-review.googlesource.com/101561 Commit-Queue: Artem Titarenko <artit@webrtc.org> Reviewed-by: Artem Titarenko <artit@webrtc.org> Reviewed-by: Patrik Höglund <phoglund@webrtc.org> Cr-Commit-Position: refs/heads/master@{#24795} [modify] https://crrev.com/03c592a1e9f9dbad02bfc9d1f55d8b8c5c499208/modules/video_coding/video_packet_buffer_unittest.cc
,
Sep 24
,
Sep 24
,
Sep 24
The problem was caused by a new feature that hans reverted in r342873. hans, thanks for the upstream analysis. yvesg, thanks for the excellent reduced test case. This should be better after the next clang roll (tracked in issue 888476 ).
,
Sep 24
,
Sep 24
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/76346c9b0afb2febbf2bf3c8d2785ce342e8ad0e commit 76346c9b0afb2febbf2bf3c8d2785ce342e8ad0e Author: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Date: Mon Sep 24 23:40:00 2018 Roll src/third_party/webrtc 645512ba5966..a36631cdb571 (26 commits) https://webrtc.googlesource.com/src.git/+log/645512ba5966..a36631cdb571 git log 645512ba5966..a36631cdb571 --date=short --no-merges --format='%ad %ae %s' 2018-09-24 danilchap@webrtc.org Expect late delayed task is deleted. 2018-09-24 buildbot@webrtc.org Roll chromium_revision c92470b71e..695c43e432 (593506:593611) 2018-09-24 niklas.enbom@webrtc.org Revert "Bug in histogram metric reporting." 2018-09-24 srte@webrtc.org Don't reset initial constraints in congestion controller. 2018-09-24 orphis@webrtc.org Add dummy implementation for SetCodecPReferences. 2018-09-24 terelius@webrtc.org Add helper class to process RtcEventLog events in order. 2018-09-24 yvesg@webrtc.org ResultsContainer: Initialize fields. 2018-09-24 yvesg@webrtc.org Auto roller: [unittest cleanup] Proper patching mechanism. 2018-09-24 kwiberg@webrtc.org TestStereo: Don't rely on the ACM to create encoders 2018-09-24 gustaf@webrtc.org AEC3: Delay estimator adapts even when estimated echo saturates 2018-09-24 kron@webrtc.org Refactor to remove direct memory dependency on kMaxId 2018-09-24 aleloi@webrtc.org Bug in histogram metric reporting. 2018-09-24 buildbot@webrtc.org Roll chromium_revision a7544fa319..c92470b71e (592771:593506) 2018-09-24 sprang@webrtc.org Revert "Reland "Enable simulcast screenshare by default"" 2018-09-24 phensman@webrtc.org Include stringutils to allow build on chromium 2018-09-24 devicentepena@webrtc.org AEC3: Bounding the nearend spectrum used as input for the suppressor gain computation 2018-09-24 artit@webrtc.org Disabled TestPacketBuffer.SeqNumWrapOneFrame test due to clang update 2018-09-24 nisse@webrtc.org Delete unused Url class. 2018-09-24 nisse@webrtc.org Delete unused HttpData methods. 2018-09-24 kwiberg@webrtc.org AudioCodingModuleTest.TestStereo: Delete write-only variables 2018-09-24 mflodman@webrtc.org Don't throttle key-frame requests per layer. 2018-09-24 jonasolsson@webrtc.org Make fewer copies when using StringBuilder. 2018-09-24 asapersson@webrtc.org Android: Add maxFramerate to RtpParameters. 2018-09-24 artit@webrtc.org Revert "Added support of getting coverage on mac" 2018-09-24 phensman@webrtc.org Add WriteVideoToFile to video_file_reader. 2018-09-24 henrika@webrtc.org Ensures that ADM unittest uses default audio devices for all platforms. Created with: gclient setdep -r src/third_party/webrtc@a36631cdb571 The AutoRoll server is located here: https://autoroll.skia.org/r/webrtc-chromium-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff, who should be CC'd on the roll, and stop the roller if necessary. CQ_INCLUDE_TRYBOTS=luci.chromium.try:linux_chromium_archive_rel_ng;luci.chromium.try:mac_chromium_archive_rel_ng BUG=chromium:None,chromium:None,chromium:755660,chromium:855108,chromium:None,chromium:690537,chromium:887464,chromium:844647 TBR=webrtc-chromium-sheriffs-robots@google.com Change-Id: I906ee8f2b3f7798a61aef95bce71fbcbe0d81221 Reviewed-on: https://chromium-review.googlesource.com/1241517 Reviewed-by: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/master@{#593747} [modify] https://crrev.com/76346c9b0afb2febbf2bf3c8d2785ce342e8ad0e/DEPS
,
Oct 11
,
Oct 30
The following revision refers to this bug: https://webrtc.googlesource.com/src.git/+/97e35ce05d4b7921acd2d523780e3e1d2d402867 commit 97e35ce05d4b7921acd2d523780e3e1d2d402867 Author: Artem Titarenko <artit@webrtc.org> Date: Tue Oct 30 12:26:48 2018 Revert "Disabled TestPacketBuffer.SeqNumWrapOneFrame test due to clang update" This reverts commit 03c592a1e9f9dbad02bfc9d1f55d8b8c5c499208. Reason for revert: Problem with clang should be solved now Original change's description: > Disabled TestPacketBuffer.SeqNumWrapOneFrame test due to clang update > > Until further investigation. > Clang update: chromium:880827 > > Bug: chromium:887464 > Change-Id: Id1fe85a013920e6ae8c6ac69efb0a0502b9dd6fe > Reviewed-on: https://webrtc-review.googlesource.com/101561 > Commit-Queue: Artem Titarenko <artit@webrtc.org> > Reviewed-by: Artem Titarenko <artit@webrtc.org> > Reviewed-by: Patrik Höglund <phoglund@webrtc.org> > Cr-Commit-Position: refs/heads/master@{#24795} TBR=phoglund@webrtc.org,artit@webrtc.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: chromium:887464 Change-Id: Id4d1722b289d9f56ae2aebf576f28f3b02a4c942 Reviewed-on: https://webrtc-review.googlesource.com/c/108583 Reviewed-by: Artem Titarenko <artit@webrtc.org> Reviewed-by: Patrik Höglund <phoglund@webrtc.org> Commit-Queue: Artem Titarenko <artit@webrtc.org> Cr-Commit-Position: refs/heads/master@{#25428} [modify] https://crrev.com/97e35ce05d4b7921acd2d523780e3e1d2d402867/modules/video_coding/video_packet_buffer_unittest.cc
,
Oct 30
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8309477a707e831908b954a314b8055de7631499 commit 8309477a707e831908b954a314b8055de7631499 Author: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Date: Tue Oct 30 23:44:27 2018 Roll src/third_party/webrtc 3b149e4be880..1a92cd7312ca (22 commits) https://webrtc.googlesource.com/src.git/+log/3b149e4be880..1a92cd7312ca git log 3b149e4be880..1a92cd7312ca --date=short --no-merges --format='%ad %ae %s' 2018-10-30 chromium-webrtc-autoroll@webrtc-ci.iam.gserviceaccount.com Roll chromium_revision 34bb9a9162..0cb3899c4e (603839:603959) 2018-10-30 mellem@webrtc.org Create a MediaTransportState enum and add a state callback to MediaTransport. 2018-10-30 emircan@webrtc.org Remove event wait logic from DesktopConfigurationMonitor 2018-10-30 alessiob@webrtc.org AGC2: renaming GainCurveApplier to Limiter. 2018-10-30 ilnik@webrtc.org Revert "Use only first payload timestamp for RTCP SR generation for audio" 2018-10-30 crodbro@webrtc.org Fix for clock reset repair. 2018-10-30 nisse@webrtc.org Delete StreamInterface::ReadLine. 2018-10-30 Peter) Slatala Update media transport settings struct 2018-10-30 terelius@webrtc.org Add support for field trials in peerconnection_client|server 2018-10-30 ilnik@webrtc.org Use only first payload timestamp for RTCP SR generation for audio 2018-10-30 terelius@webrtc.org Add field trial to enable the new RTC event log format. 2018-10-30 artit@webrtc.org Revert "Disabled TestPacketBuffer.SeqNumWrapOneFrame test due to clang update" 2018-10-30 robert@bares.me Always call ConvertToI420 with positive crop_height 2018-10-30 nisse@webrtc.org Delete OptionsFile class. Refactored only user, TurnFileAuth. 2018-10-30 srte@webrtc.org Makes PacketResult::GetSentPacket const. 2018-10-30 chromium-webrtc-autoroll@webrtc-ci.iam.gserviceaccount.com Roll chromium_revision 89ed1da2c8..34bb9a9162 (603733:603839) 2018-10-30 nisse@webrtc.org Delete unused function rtc::Flow. 2018-10-30 artit@webrtc.org Add iOS SDK unit tests for nalu_rewriter 2018-10-30 kron@webrtc.org Propagate SDP negotiation of extmap-allow-mixed to RtpHeaderExtensionMap 2018-10-30 oprypin@webrtc.org Remove likely obsolete entries from WATCHLISTS 2018-10-30 chromium-webrtc-autoroll@webrtc-ci.iam.gserviceaccount.com Roll chromium_revision 03b56190ff..89ed1da2c8 (603619:603733) 2018-10-29 chromium-webrtc-autoroll@webrtc-ci.iam.gserviceaccount.com Roll chromium_revision 55624cc6cd..03b56190ff (603513:603619) Created with: gclient setdep -r src/third_party/webrtc@1a92cd7312ca The AutoRoll server is located here: https://autoroll.skia.org/r/webrtc-chromium-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff, who should be CC'd on the roll, and stop the roller if necessary. CQ_INCLUDE_TRYBOTS=luci.chromium.try:linux_chromium_archive_rel_ng;luci.chromium.try:mac_chromium_archive_rel_ng BUG=chromium:None,chromium:796889,chromium:none,chromium:887464,chromium:None,chromium:None,chromium:None,chromium:None TBR=webrtc-chromium-sheriffs-robots@google.com Change-Id: I6de52c483d648ac07efede2b984bc1b7228e6754 Reviewed-on: https://chromium-review.googlesource.com/c/1308696 Reviewed-by: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/master@{#604058} [modify] https://crrev.com/8309477a707e831908b954a314b8055de7631499/DEPS |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by yvesg@google.com
, Sep 20