From 8ff3ff1a8b8759fa757f0a5e051d5ace93c95c31 Mon Sep 17 00:00:00 2001 From: "phoglund@webrtc.org" Date: Mon, 1 Oct 2012 10:04:26 +0000 Subject: [PATCH] Made ViE standard tests runnable under valgrind. Ensured there are bugs for all open valgrind issues in the standard tests and suppressed the known issues. This way, we can get it running in continuous integration and keep new issues from entering. Removed bad check in codec test, added suppressions. Fixed simple memory leaks in tests. BUG=Related to bug 329 TEST=Ran the vie_auto_test standard suite many times under valgrind to root out flakiness. Ran the standard suite without valgrind to ensure I didn't break anything. Review URL: https://webrtc-codereview.appspot.com/843005 git-svn-id: http://webrtc.googlecode.com/svn/trunk@2854 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../main/source/Linux/device_info_linux.cc | 2 +- .../test/auto_test/interface/vie_autotest.h | 36 +++--- .../test/auto_test/source/vie_autotest.cc | 4 + .../auto_test/source/vie_autotest_capture.cc | 1 + .../auto_test/source/vie_autotest_codec.cc | 18 +-- .../auto_test/source/vie_autotest_file.cc | 5 + .../auto_test/source/vie_autotest_rtp_rtcp.cc | 59 +++++----- .../valgrind-webrtc/memcheck/suppressions.txt | 107 +++++++++++++++++- 8 files changed, 171 insertions(+), 61 deletions(-) diff --git a/src/modules/video_capture/main/source/Linux/device_info_linux.cc b/src/modules/video_capture/main/source/Linux/device_info_linux.cc index 653ee1601d..239a29267c 100644 --- a/src/modules/video_capture/main/source/Linux/device_info_linux.cc +++ b/src/modules/video_capture/main/source/Linux/device_info_linux.cc @@ -142,7 +142,7 @@ WebRtc_Word32 DeviceInfoLinux::GetDeviceName( if (cap.bus_info[0] != 0) // may not available in all drivers { - // copy device id + // copy device id if (deviceUniqueIdUTF8Length >= strlen((const char*) cap.bus_info)) { memset(deviceUniqueIdUTF8, 0, deviceUniqueIdUTF8Length); diff --git a/src/video_engine/test/auto_test/interface/vie_autotest.h b/src/video_engine/test/auto_test/interface/vie_autotest.h index 06087010ee..f137fdfca0 100644 --- a/src/video_engine/test/auto_test/interface/vie_autotest.h +++ b/src/video_engine/test/auto_test/interface/vie_autotest.h @@ -16,24 +16,22 @@ #define WEBRTC_VIDEO_ENGINE_MAIN_TEST_AUTOTEST_INTERFACE_VIE_AUTOTEST_H_ #include "common_types.h" - -#include "voe_base.h" -#include "voe_codec.h" -#include "voe_hardware.h" -#include "voe_audio_processing.h" - -#include "vie_base.h" -#include "vie_capture.h" -#include "vie_codec.h" -#include "vie_file.h" -#include "vie_network.h" -#include "vie_render.h" -#include "vie_rtp_rtcp.h" -#include "vie_defines.h" -#include "vie_errors.h" -#include "video_render_defines.h" - -#include "vie_autotest_defines.h" +#include "gflags/gflags.h" +#include "modules/video_render/main/interface/video_render_defines.h" +#include "video_engine/test/auto_test/interface/vie_autotest_defines.h" +#include "video_engine/include/vie_base.h" +#include "video_engine/include/vie_capture.h" +#include "video_engine/include/vie_codec.h" +#include "video_engine/include/vie_file.h" +#include "video_engine/include/vie_network.h" +#include "video_engine/include/vie_render.h" +#include "video_engine/include/vie_rtp_rtcp.h" +#include "video_engine/include/vie_errors.h" +#include "video_engine/vie_defines.h" +#include "voice_engine/include/voe_audio_processing.h" +#include "voice_engine/include/voe_base.h" +#include "voice_engine/include/voe_codec.h" +#include "voice_engine/include/voe_hardware.h" #ifndef WEBRTC_ANDROID #include @@ -44,6 +42,8 @@ class TbInterfaces; class TbVideoChannel; class ViEToFileRenderer; +DECLARE_bool(include_timing_dependent_tests); + // This class provides a bunch of methods, implemented across several .cc // files, which runs tests on the video engine. All methods will report // errors using standard googletest macros, except when marked otherwise. diff --git a/src/video_engine/test/auto_test/source/vie_autotest.cc b/src/video_engine/test/auto_test/source/vie_autotest.cc index 28bedc9151..e81e6edeb5 100644 --- a/src/video_engine/test/auto_test/source/vie_autotest.cc +++ b/src/video_engine/test/auto_test/source/vie_autotest.cc @@ -25,6 +25,10 @@ #include "video_engine/test/libvietest/include/tb_interfaces.h" #include "video_engine/test/libvietest/include/tb_video_channel.h" +DEFINE_bool(include_timing_dependent_tests, true, + "If true, we will include tests / parts of tests that are known " + "to break in slow execution environments (such as valgrind)."); + // ViETest implementation FILE* ViETest::log_file_ = NULL; char* ViETest::log_str_ = NULL; diff --git a/src/video_engine/test/auto_test/source/vie_autotest_capture.cc b/src/video_engine/test/auto_test/source/vie_autotest_capture.cc index 8a7887cbf5..f232d89eb4 100644 --- a/src/video_engine/test/auto_test/source/vie_autotest_capture.cc +++ b/src/video_engine/test/auto_test/source/vie_autotest_capture.cc @@ -237,6 +237,7 @@ void ViEAutoTest::ViECaptureStandardTest() { /// ************************************************************** // Testing finished. Tear down Video Engine /// ************************************************************** + delete dev_info; // Stop all started capture devices. for (int device_index = 0; device_index < number_of_capture_devices; diff --git a/src/video_engine/test/auto_test/source/vie_autotest_codec.cc b/src/video_engine/test/auto_test/source/vie_autotest_codec.cc index b1cb960d26..b9a64809d2 100644 --- a/src/video_engine/test/auto_test/source/vie_autotest_codec.cc +++ b/src/video_engine/test/auto_test/source/vie_autotest_codec.cc @@ -203,20 +203,10 @@ void ViEAutoTest::ViECodecStandardTest() { EXPECT_EQ(video_codec.codecType, codec_observer.incoming_codec_.codecType); - int max_number_of_possible_frames = video_codec.maxFramerate - * KAutoTestSleepTimeMs / 1000; - - if (video_codec.codecType == webrtc::kVideoCodecI420) { - // Don't expect too much from I420, it requires a lot of bandwidth. - EXPECT_GT(frame_counter.num_frames_, 0); - } else { -#ifdef WEBRTC_ANDROID - // To get the autotest to pass on some slow devices - EXPECT_GT(frame_counter.num_frames_, max_number_of_possible_frames / 6); -#else - EXPECT_GT(frame_counter.num_frames_, max_number_of_possible_frames / 4); -#endif - } + // This requirement is quite relaxed, but it's hard to say what's an + // acceptable number of received frames when we take into account the + // wide variety of devices (and that we run under valgrind). + EXPECT_GT(frame_counter.num_frames_, 0); EXPECT_EQ(0, image_process->DeregisterRenderEffectFilter( video_channel)); diff --git a/src/video_engine/test/auto_test/source/vie_autotest_file.cc b/src/video_engine/test/auto_test/source/vie_autotest_file.cc index 17b4d0cd5a..4d5ee74d42 100644 --- a/src/video_engine/test/auto_test/source/vie_autotest_file.cc +++ b/src/video_engine/test/auto_test/source/vie_autotest_file.cc @@ -271,6 +271,7 @@ void ViEAutoTest::ViEFileStandardTest() AutoTestSleep(TEST_SPACING); // GetCaptureDeviceSnapshot + if (FLAGS_include_timing_dependent_tests) { ViETest::Log("Testing GetCaptureDeviceSnapshot(int, ViEPicture)"); ViETest::Log("Taking a picture to use for displaying ViEPictures " @@ -318,6 +319,7 @@ void ViEAutoTest::ViEFileStandardTest() AutoTestSleep(TEST_SPACING); // GetCaptureDeviceSnapshot + if (FLAGS_include_timing_dependent_tests) { ViETest::Log("Testing GetCaptureDeviceSnapshot(int, char*)"); ViETest::Log("Taking snapshot from capture device %d", captureId); @@ -346,6 +348,7 @@ void ViEAutoTest::ViEFileStandardTest() AutoTestSleep(TEST_SPACING); // Testing: SetCaptureDeviceImage + if (FLAGS_include_timing_dependent_tests) { ViETest::Log("Testing SetCaptureDeviceImage(int, ViEPicture)"); EXPECT_EQ(0, ptrViECapture->StopCapture(captureId)); @@ -361,6 +364,7 @@ void ViEAutoTest::ViEFileStandardTest() AutoTestSleep(TEST_SPACING); // testing SetRenderStartImage(videoChannel, renderStartImage); + if (FLAGS_include_timing_dependent_tests) { ViETest::Log("Testing SetRenderStartImage(int, char*)"); // set render image, then stop capture and stop render to display it @@ -406,6 +410,7 @@ void ViEAutoTest::ViEFileStandardTest() // testing SetRenderTimeoutImage(videoChannel, renderTimeoutFile, // RENDER_TIMEOUT); + if (FLAGS_include_timing_dependent_tests) { ViETest::Log("Testing SetRenderTimeoutImage(int, char*)"); ViETest::Log("Stopping capture device to induce timeout of %d ms", diff --git a/src/video_engine/test/auto_test/source/vie_autotest_rtp_rtcp.cc b/src/video_engine/test/auto_test/source/vie_autotest_rtp_rtcp.cc index 57eac00678..858a34af97 100644 --- a/src/video_engine/test/auto_test/source/vie_autotest_rtp_rtcp.cc +++ b/src/video_engine/test/auto_test/source/vie_autotest_rtp_rtcp.cc @@ -8,19 +8,16 @@ * be found in the AUTHORS file in the root of the source tree. */ -// -// vie_autotest_rtp_rtcp.cc -// #include #include "engine_configurations.h" -#include "tb_capture_device.h" -#include "tb_external_transport.h" -#include "tb_interfaces.h" -#include "tb_video_channel.h" -#include "testsupport/fileutils.h" -#include "vie_autotest.h" -#include "vie_autotest_defines.h" +#include "video_engine/test/libvietest/include/tb_capture_device.h" +#include "video_engine/test/libvietest/include/tb_external_transport.h" +#include "video_engine/test/libvietest/include/tb_interfaces.h" +#include "video_engine/test/libvietest/include/tb_video_channel.h" +#include "test/testsupport/fileutils.h" +#include "video_engine/test/auto_test/interface/vie_autotest.h" +#include "video_engine/test/auto_test/interface/vie_autotest_defines.h" class ViERtpObserver: public webrtc::ViERTPObserver { @@ -150,11 +147,13 @@ void ViEAutoTest::ViERtpRtcpStandardTest() AutoTestSleep(1000); - char remoteCName[webrtc::ViERTP_RTCP::KMaxRTCPCNameLength]; - memset(remoteCName, 0, webrtc::ViERTP_RTCP::KMaxRTCPCNameLength); - EXPECT_EQ(0, ViE.rtp_rtcp->GetRemoteRTCPCName( - tbChannel.videoChannel, remoteCName)); - EXPECT_STRCASEEQ(sendCName, remoteCName); + if (FLAGS_include_timing_dependent_tests) { + char remoteCName[webrtc::ViERTP_RTCP::KMaxRTCPCNameLength]; + memset(remoteCName, 0, webrtc::ViERTP_RTCP::KMaxRTCPCNameLength); + EXPECT_EQ(0, ViE.rtp_rtcp->GetRemoteRTCPCName( + tbChannel.videoChannel, remoteCName)); + EXPECT_STRCASEEQ(sendCName, remoteCName); + } // // Statistics @@ -228,10 +227,12 @@ void ViEAutoTest::ViERtpRtcpStandardTest() &estimated_bandwidth)); EXPECT_GT(estimated_bandwidth, 0u); - EXPECT_EQ(0, ViE.rtp_rtcp->GetEstimatedReceiveBandwidth( - tbChannel.videoChannel, - &estimated_bandwidth)); - EXPECT_GT(estimated_bandwidth, 0u); + if (FLAGS_include_timing_dependent_tests) { + EXPECT_EQ(0, ViE.rtp_rtcp->GetEstimatedReceiveBandwidth( + tbChannel.videoChannel, + &estimated_bandwidth)); + EXPECT_GT(estimated_bandwidth, 0u); + } // Check that rec stats extended max is greater than what we've sent. EXPECT_GE(recExtendedMax, sentExtendedMax); @@ -298,16 +299,20 @@ void ViEAutoTest::ViERtpRtcpStandardTest() AutoTestSleep(2000); unsigned int receivedSSRC = myTransport.ReceivedSSRC(); ViETest::Log("Received SSRC %u\n", receivedSSRC); - EXPECT_EQ(setSSRC, receivedSSRC); - unsigned int localSSRC = 0; - EXPECT_EQ(0, ViE.rtp_rtcp->GetLocalSSRC(tbChannel.videoChannel, localSSRC)); - EXPECT_EQ(setSSRC, localSSRC); + if (FLAGS_include_timing_dependent_tests) { + EXPECT_EQ(setSSRC, receivedSSRC); - unsigned int remoteSSRC = 0; - EXPECT_EQ(0, ViE.rtp_rtcp->GetRemoteSSRC( - tbChannel.videoChannel, remoteSSRC)); - EXPECT_EQ(setSSRC, remoteSSRC); + unsigned int localSSRC = 0; + EXPECT_EQ(0, ViE.rtp_rtcp->GetLocalSSRC( + tbChannel.videoChannel, localSSRC)); + EXPECT_EQ(setSSRC, localSSRC); + + unsigned int remoteSSRC = 0; + EXPECT_EQ(0, ViE.rtp_rtcp->GetRemoteSSRC( + tbChannel.videoChannel, remoteSSRC)); + EXPECT_EQ(setSSRC, remoteSSRC); + } EXPECT_EQ(0, ViE.base->StopSend(tbChannel.videoChannel)); diff --git a/tools/valgrind-webrtc/memcheck/suppressions.txt b/tools/valgrind-webrtc/memcheck/suppressions.txt index 34941da8f7..faa594633d 100644 --- a/tools/valgrind-webrtc/memcheck/suppressions.txt +++ b/tools/valgrind-webrtc/memcheck/suppressions.txt @@ -21,4 +21,109 @@ fun:_ZN6webrtc14ViECaptureImpl12StartCaptureEiRKNS_17CaptureCapabilityE fun:_ZN15TbCaptureDeviceC1ER12TbInterfaces fun:_ZN12_GLOBAL__N_114ViERtpFuzzTest5SetUpEv -} \ No newline at end of file +} + +{ + bug_329_1 + Memcheck:Unaddressable + fun:I422ToARGBRow_SSSE3 + fun:I420ToARGB + fun:ConvertFromI420 + fun:_ZN6webrtc15ConvertFromI420EPKhiNS_9VideoTypeEiiiPh + fun:_ZN6webrtc15VideoX11Channel12DeliverFrameEPhij + fun:_ZN6webrtc15VideoX11Channel11RenderFrameEjRNS_10VideoFrameE + fun:_ZN6webrtc19IncomingVideoStream26IncomingVideoStreamProcessEv + fun:_ZN6webrtc19IncomingVideoStream28IncomingVideoStreamThreadFunEPv + fun:_ZN6webrtc11ThreadPosix3RunEv + fun:StartThread +} + +{ + bug_329_2 + Memcheck:Leak + fun:_Znw* + fun:_ZN6webrtc18videocapturemodule16VideoCaptureImpl16CreateDeviceInfoEi + fun:_ZN6webrtc19VideoCaptureFactory16CreateDeviceInfoEi + fun:_ZN11ViEAutoTest22ViECaptureStandardTestEv + fun:_ZN12_GLOBAL__N_160ViEStandardIntegrationTest_RunsCaptureTestWithoutErrors_Test8TestBodyEv +} + +{ + bug_329_3 + Memcheck:Unaddressable + fun:I422ToARGBRow_SSSE3 + fun:I420ToARGB + fun:ConvertFromI420 + fun:_ZN6webrtc15ConvertFromI420EPKhiNS_9VideoTypeEiiiPh + fun:_ZN6webrtc15VideoX11Channel12DeliverFrameEPhij + fun:_ZN6webrtc15VideoX11Channel11RenderFrameEjRNS_10VideoFrameE + fun:_ZN6webrtc19IncomingVideoStream26IncomingVideoStreamProcessEv + fun:_ZN6webrtc19IncomingVideoStream28IncomingVideoStreamThreadFunEPv + fun:_ZN6webrtc11ThreadPosix3RunEv + fun:StartThread +} + +{ + bug_329_4 + Memcheck:Param + socketcall.sendto(msg) + obj:/lib/x86_64-linux-gnu/libpthread-2.15.so + fun:_ZN6webrtc14UdpSocketPosix6SendToEPKaiRKNS_13SocketAddressE + fun:_ZN6webrtc16UdpTransportImpl10SendPacketEiPKvi + fun:_ZN6webrtc9ViESender10SendPacketEiPKvi + fun:_ZN6webrtc9RTPSender13SendToNetworkEPhttlNS_11StorageTypeE + fun:_ZN6webrtc14RTPSenderVideo15SendVideoPacketEPhttlNS_11StorageTypeEb + fun:_ZN6webrtc14RTPSenderVideo7SendVP8ENS_9FrameTypeEajlPKhjPKNS_22RTPFragmentationHeaderEPKNS_18RTPVideoTypeHeaderE + fun:_ZN6webrtc14RTPSenderVideo9SendVideoENS_18RtpVideoCodecTypesENS_9FrameTypeEajlPKhjPKNS_22RTPFragmentationHeaderEPNS_21VideoCodecInformationEPKNS_18RTPVideoTypeHeaderE + fun:_ZN6webrtc9RTPSender16SendOutgoingDataENS_9FrameTypeEajlPKhjPKNS_22RTPFragmentationHeaderEPNS_21VideoCodecInformationEPKNS_18RTPVideoTypeHeaderE + fun:_ZN6webrtc17ModuleRtpRtcpImpl16SendOutgoingDataENS_9FrameTypeEajlPKhjPKNS_22RTPFragmentationHeaderEPKNS_14RTPVideoHeaderE + fun:_ZN6webrtc17ModuleRtpRtcpImpl16SendOutgoingDataENS_9FrameTypeEajlPKhjPKNS_22RTPFragmentationHeaderEPKNS_14RTPVideoHeaderE + fun:_ZN6webrtc10ViEEncoder8SendDataENS_9FrameTypeEhjlPKhjRKNS_22RTPFragmentationHeaderEPKNS_14RTPVideoHeaderE + fun:_ZN6webrtc23VCMEncodedFrameCallback7EncodedERNS_12EncodedImageEPKNS_17CodecSpecificInfoEPKNS_22RTPFragmentationHeaderE + fun:_ZN6webrtc10VP8Encoder20GetEncodedPartitionsERKNS_10VideoFrameE + fun:_ZN6webrtc10VP8Encoder6EncodeERKNS_10VideoFrameEPKNS_17CodecSpecificInfoENS_14VideoFrameTypeE + fun:_ZN6webrtc17VCMGenericEncoder6EncodeERKNS_10VideoFrameEPKNS_17CodecSpecificInfoENS_9FrameTypeE + fun:_ZN6webrtc21VideoCodingModuleImpl13AddVideoFrameERKNS_10VideoFrameEPKNS_19VideoContentMetricsEPKNS_17CodecSpecificInfoE + fun:_ZN6webrtc10ViEEncoder12DeliverFrameEiPNS_10VideoFrameEiPKj + fun:_ZN6webrtc20ViEFrameProviderBase12DeliverFrameEPNS_10VideoFrameEiPKj + fun:_ZN6webrtc11ViECapturer16DeliverI420FrameEPNS_10VideoFrameE + fun:_ZN6webrtc11ViECapturer17ViECaptureProcessEv +} + +{ + bug_329_5 + Memcheck:Param + socketcall.sendto(msg) + obj:/lib/x86_64-linux-gnu/libpthread-2.15.so + fun:_ZN6webrtc14UdpSocketPosix6SendToEPKaiRKNS_13SocketAddressE + fun:_ZN6webrtc16UdpTransportImpl14SendRTCPPacketEiPKvi + fun:_ZN6webrtc9ViESender14SendRTCPPacketEiPKvi + fun:_ZN6webrtc10RTCPSender13SendToNetworkEPKht + fun:_ZN6webrtc10RTCPSender8SendRTCPEjiPKtbm + fun:_ZN6webrtc17ModuleRtpRtcpImpl7ProcessEv + fun:_ZN6webrtc17ProcessThreadImpl7ProcessEv + fun:_ZN6webrtc17ProcessThreadImpl3RunEPv + fun:_ZN6webrtc11ThreadPosix3RunEv + fun:StartThread +} + +{ + bug_891 + Memcheck:Unaddressable + fun:XShmPutImage + fun:_ZN6webrtc15VideoX11Channel12DeliverFrameEPhij + fun:_ZN6webrtc15VideoX11Channel11RenderFrameEjRNS_10VideoFrameE + fun:_ZN6webrtc19IncomingVideoStream26IncomingVideoStreamProcessEv + fun:_ZN6webrtc19IncomingVideoStream28IncomingVideoStreamThreadFunEPv + fun:_ZN6webrtc11ThreadPosix3RunEv + fun:StartThread +} + +{ + ignore_common_benevolent_trace_library_errors + Memcheck:Uninitialized + ... + fun:vsnprintf + fun:_ZN6webrtc5Trace3AddENS_10TraceLevelENS_11TraceModuleEiPKcz + ... +}