Fixes (or at least reduces) the flakiness in the full stack test by making sure the different frame monitors are registered and deregistered in the right order. Also makes sure only local preview frames which are actually transmitted are rendered by moving the local preview rendering to an effect filter.

BUG=

Review URL: https://webrtc-codereview.appspot.com/969005

git-svn-id: http://webrtc.googlecode.com/svn/trunk@3157 4adac7df-926f-26a2-2b94-8c16560cd09d

webrtc/video_engine/test/auto_test/automated/vie_video_verification_test.cc

@@ -90,11 +90,8 @@ class ViEVideoVerificationTest : public testing::Test {
  private:
   void SetUpFileRenderer(ViEToFileRenderer* file_renderer,
                          const std::string& suffix) {
-    std::string test_case_name =
-        ::testing::UnitTest::GetInstance()->current_test_info()->name();
-
     std::string output_path = ViETest::GetResultOutputPath();
-    std::string filename = test_case_name + suffix;
+    std::string filename = "render_output" + suffix;
 
     if (!file_renderer->PrepareForRendering(output_path, filename)) {
       FAIL() << "Could not open output file " << filename <<
@@ -109,14 +106,40 @@ class ViEVideoVerificationTest : public testing::Test {
       if (test_failed) {
         // Leave the files for analysis if the test failed.
         file_renderer->SaveOutputFile("failed-");
-      } else {
-        // No reason to keep the files if we succeeded.
-        file_renderer->DeleteOutputFile();
       }
       delete file_renderer;
     }
 };
 
+class ParameterizedFullStackTest : public ViEVideoVerificationTest,
+                                   public ::testing::WithParamInterface<int> {
+ protected:
+  struct TestParameters {
+    int packet_loss_rate;
+    int round_trip_time;
+    int bitrate;
+    double avg_psnr_threshold;
+    double avg_ssim_threshold;
+  };
+
+  void SetUp() {
+    int i = 0;
+    parameter_table_[i].packet_loss_rate = 0;
+    parameter_table_[i].round_trip_time = 0;
+    parameter_table_[i].bitrate = 300;
+    parameter_table_[i].avg_psnr_threshold = 35;
+    parameter_table_[i].avg_ssim_threshold = 0.96;
+    ++i;
+    parameter_table_[i].packet_loss_rate = 5;
+    parameter_table_[i].round_trip_time = 50;
+    parameter_table_[i].bitrate = 300;
+    parameter_table_[i].avg_psnr_threshold = 35;
+    parameter_table_[i].avg_ssim_threshold = 0.96;
+  }
+
+  TestParameters parameter_table_[2];
+};
+
 // TODO(phoglund): Needs to be rewritten to use external transport. Currently
 // the new memory-safe decoder is too slow with I420 with the default packet
 // engine. See http://code.google.com/p/webrtc/issues/detail?id=1103.
@@ -155,12 +178,12 @@ TEST_F(ViEVideoVerificationTest, DISABLED_RunsBaseStandardTestWithoutErrors) {
 }
 
 // Runs a whole stack processing with tracking of which frames are dropped
-// in the encoder. The local and remote file will not be of equal size because
+// in the encoder. Tests show that they start at the same frame, which is
-// of unknown reasons. Tests show that they start at the same frame, which is
 // the important thing when doing frame-to-frame comparison with PSNR/SSIM.
-// TODO(phoglund): This is flaky and a bit incomplete - enable again when it has
+TEST_P(ParameterizedFullStackTest, RunsFullStackWithoutErrors)  {
-// been made more deterministic.
+  // Using CIF here since it's a more common resolution than QCIF, and higher
-TEST_F(ViEVideoVerificationTest, DISABLED_RunsFullStackWithoutErrors)  {
+  // resolutions shouldn't be a problem for a test using VP8.
+  input_file_ = webrtc::test::ResourcePath("foreman_cif", "yuv");
   FrameDropDetector detector;
   local_file_renderer_ = new ViEToFileRenderer();
   remote_file_renderer_ = new FrameDropMonitoringRemoteFileRenderer(&detector);
@@ -169,13 +192,15 @@ TEST_F(ViEVideoVerificationTest, DISABLED_RunsFullStackWithoutErrors)  {
 
   // Set a low bit rate so the encoder budget will be tight, causing it to drop
   // frames every now and then.
-  const int kBitRateKbps = 50;
+  const int kBitRateKbps = parameter_table_[GetParam()].bitrate;
-  const int kPacketLossPercent = 5;
+  const int kPacketLossPercent = parameter_table_[GetParam()].packet_loss_rate;
-  const int kNetworkDelayMs = 100;
+  const int kNetworkDelayMs = parameter_table_[GetParam()].round_trip_time;
+  int width = 352;
+  int height = 288;
   ViETest::Log("Bit rate     : %5d kbps", kBitRateKbps);
   ViETest::Log("Packet loss  : %5d %%", kPacketLossPercent);
   ViETest::Log("Network delay: %5d ms", kNetworkDelayMs);
-  tests_.TestFullStack(input_file_, kInputWidth, kInputHeight, kBitRateKbps,
+  tests_.TestFullStack(input_file_, width, height, kBitRateKbps,
                        kPacketLossPercent, kNetworkDelayMs,
                        local_file_renderer_, remote_file_renderer_, &detector);
   const std::string reference_file = local_file_renderer_->GetFullOutputPath();
@@ -185,12 +210,12 @@ TEST_F(ViEVideoVerificationTest, DISABLED_RunsFullStackWithoutErrors)  {
   detector.CalculateResults();
   detector.PrintReport();
 
-  if (detector.GetNumberOfFramesDroppedAt(FrameDropDetector::kRendered) !=
+  if (detector.GetNumberOfFramesDroppedAt(FrameDropDetector::kRendered) >
       detector.GetNumberOfFramesDroppedAt(FrameDropDetector::kDecoded)) {
     detector.PrintDebugDump();
   }
 
-  ASSERT_EQ(detector.GetNumberOfFramesDroppedAt(FrameDropDetector::kRendered),
+  ASSERT_GE(detector.GetNumberOfFramesDroppedAt(FrameDropDetector::kRendered),
       detector.GetNumberOfFramesDroppedAt(FrameDropDetector::kDecoded))
       << "The number of dropped frames on the decode and render steps are not "
       "equal. This may be because we have a major problem in the buffers of "
@@ -201,7 +226,7 @@ TEST_F(ViEVideoVerificationTest, DISABLED_RunsFullStackWithoutErrors)  {
   // To make the quality measurement correct, we must adjust the output file to
   // that by copying the last successful frame into the place where the dropped
   // frame would be, for all dropped frames.
-  const int frame_length_in_bytes = 3 * kInputHeight * kInputWidth / 2;
+  const int frame_length_in_bytes = 3 * width * height / 2;
   ViETest::Log("Frame length: %d bytes", frame_length_in_bytes);
   std::vector<Frame*> all_frames = detector.GetAllFrames();
   FixOutputFileForComparison(output_file, frame_length_in_bytes, all_frames);
@@ -213,19 +238,24 @@ TEST_F(ViEVideoVerificationTest, DISABLED_RunsFullStackWithoutErrors)  {
       "of frames multiplied by the frame size. This will likely affect "
       "PSNR/SSIM calculations in a bad way.";
 
+  TearDownFileRenderers();
+
   // We are running on a lower bitrate here so we need to settle for somewhat
   // lower PSNR and SSIM values.
   double actual_psnr = 0;
   double actual_ssim = 0;
   CompareFiles(reference_file, output_file, &actual_psnr, &actual_ssim);
 
-  TearDownFileRenderers();
+  const double kExpectedMinimumPSNR =
-
+      parameter_table_[GetParam()].avg_psnr_threshold;
-  const double kExpectedMinimumPSNR = 24;
+  const double kExpectedMinimumSSIM =
-  const double kExpectedMinimumSSIM = 0.7;
+      parameter_table_[GetParam()].avg_ssim_threshold;
 
   EXPECT_GE(actual_psnr, kExpectedMinimumPSNR);
   EXPECT_GE(actual_ssim, kExpectedMinimumSSIM);
 }
 
+INSTANTIATE_TEST_CASE_P(FullStackTests, ParameterizedFullStackTest,
+                        ::testing::Values(0, 1));
+
 }  // namespace

webrtc/video_engine/test/auto_test/primitives/framedrop_primitives.cc

@@ -24,23 +24,39 @@
 #include "video_engine/test/libvietest/include/tb_external_transport.h"
 #include "video_engine/test/libvietest/include/vie_to_file_renderer.h"
 
-// Tracks which frames are created on the local side and reports them to the
+enum { kWaitTimeForFinalDecodeMs = 100 };
+
+// Writes the frames to be encoded to file and tracks which frames are sent in
+// external transport on the local side and reports them to the
 // FrameDropDetector class.
-class CreatedTimestampEffectFilter : public webrtc::ViEEffectFilter {
+class LocalRendererEffectFilter : public webrtc::ViEEffectFilter {
  public:
-  explicit CreatedTimestampEffectFilter(FrameDropDetector* frame_drop_detector)
+  explicit LocalRendererEffectFilter(FrameDropDetector* frame_drop_detector,
-      : frame_drop_detector_(frame_drop_detector) {}
+                                     webrtc::ExternalRenderer* renderer)
-  virtual ~CreatedTimestampEffectFilter() {}
+      : width_(0), height_(0), frame_drop_detector_(frame_drop_detector),
+        renderer_(renderer) {}
+  virtual ~LocalRendererEffectFilter() {}
   virtual int Transform(int size, unsigned char* frameBuffer,
                         unsigned int timeStamp90KHz, unsigned int width,
                         unsigned int height) {
+    if (width != width_ || height_ != height) {
+      renderer_->FrameSizeChange(width, height, 1);
+      width_ = width;
+      height_ = height;
+    }
     frame_drop_detector_->ReportFrameState(FrameDropDetector::kCreated,
                                            timeStamp90KHz);
-    return 0;
+    return renderer_->DeliverFrame(frameBuffer,
+                                   size,
+                                   timeStamp90KHz,
+                                   webrtc::TickTime::MillisecondTimestamp());
   }
 
  private:
+  unsigned int width_;
+  unsigned int height_;
   FrameDropDetector* frame_drop_detector_;
+  webrtc::ExternalRenderer* renderer_;
 };
 
 // Tracks which frames are sent in external transport on the local side
@@ -102,7 +118,9 @@ void TestFullStack(const TbInterfaces& interfaces,
                    int bit_rate_kbps,
                    int packet_loss_percent,
                    int network_delay_ms,
-                   FrameDropDetector* frame_drop_detector) {
+                   FrameDropDetector* frame_drop_detector,
+                   ViEToFileRenderer* remote_file_renderer,
+                   ViEToFileRenderer* local_file_renderer) {
   webrtc::VideoEngine *video_engine_interface = interfaces.video_engine;
   webrtc::ViEBase *base_interface = interfaces.base;
   webrtc::ViECapture *capture_interface = interfaces.capture;
@@ -137,6 +155,7 @@ void TestFullStack(const TbInterfaces& interfaces,
   external_transport.RegisterReceiveFrameCallback(&frame_received_callback);
   EXPECT_EQ(0, network_interface->RegisterSendTransport(video_channel,
                                                         external_transport));
+  RenderToFile(interfaces.render, video_channel, remote_file_renderer);
   EXPECT_EQ(0, base_interface->StartReceive(video_channel));
 
   // Setup only the VP8 codec, which is what we'll use.
@@ -153,10 +172,13 @@ void TestFullStack(const TbInterfaces& interfaces,
       webrtc::ViEImageProcess::GetInterface(video_engine_interface);
   EXPECT_TRUE(image_process);
 
-  // Setup the effect filters
+  // Setup the effect filters.
-  CreatedTimestampEffectFilter create_filter(frame_drop_detector);
+  // Local rendering at the send-side is done in an effect filter to avoid
+  // synchronization issues with the remote renderer.
+  LocalRendererEffectFilter local_renderer_filter(frame_drop_detector,
+                                                  local_file_renderer);
   EXPECT_EQ(0, image_process->RegisterSendEffectFilter(video_channel,
-                                                       create_filter));
+                                                       local_renderer_filter));
   DecodedTimestampEffectFilter decode_filter(frame_drop_detector);
   EXPECT_EQ(0, image_process->RegisterRenderEffectFilter(video_channel,
                                                          decode_filter));
@@ -164,12 +186,36 @@ void TestFullStack(const TbInterfaces& interfaces,
   EXPECT_EQ(0, base_interface->StartSend(video_channel));
   AutoTestSleep(KAutoTestSleepTimeMs);
 
-  // Cleanup.
-  EXPECT_EQ(0, image_process->DeregisterSendEffectFilter(video_channel));
-  EXPECT_EQ(0, image_process->DeregisterRenderEffectFilter(video_channel));
-  image_process->Release();
   ViETest::Log("Done!");
 
+  // ***************************************************************
+  // Testing finished. Tear down Video Engine
+  // ***************************************************************
+  EXPECT_EQ(0, capture_interface->DisconnectCaptureDevice(video_channel));
+
+  // Wait for the last packet to arrive before we tear down the receiver.
+  AutoTestSleep(2*network_delay_ms);
+  EXPECT_EQ(0, base_interface->StopSend(video_channel));
+  while (!external_transport.EmptyQueue()) {
+    AutoTestSleep(network_delay_ms);
+  }
+  EXPECT_EQ(0, base_interface->StopReceive(video_channel));
+  EXPECT_EQ(0, network_interface->DeregisterSendTransport(video_channel));
+  // Wait for the last frame to be decoded and rendered. There is no guarantee
+  // this wait time will be long enough. Ideally we would wait for at least one
+  // "receive-side delay", which is what the video coding module calculates
+  // based on network statistics etc. We don't have access to that value here.
+  AutoTestSleep(kWaitTimeForFinalDecodeMs);
+  // Must stop the frame drop detectors in the right order to avoid getting
+  // frames which for instance are rendered but not decoded.
+  EXPECT_EQ(0, render_interface->StopRender(video_channel));
+  EXPECT_EQ(0, render_interface->RemoveRenderer(video_channel));
+  EXPECT_EQ(0, image_process->DeregisterRenderEffectFilter(video_channel));
+  EXPECT_EQ(0, image_process->DeregisterSendEffectFilter(video_channel));
+  image_process->Release();
+  EXPECT_EQ(0, base_interface->DeleteChannel(video_channel));
+
+  // Collect transport statistics.
   WebRtc_Word32 num_rtp_packets = 0;
   WebRtc_Word32 num_dropped_packets = 0;
   WebRtc_Word32 num_rtcp_packets = 0;
@@ -178,19 +224,6 @@ void TestFullStack(const TbInterfaces& interfaces,
   ViETest::Log("RTP packets    : %5d", num_rtp_packets);
   ViETest::Log("Dropped packets: %5d", num_dropped_packets);
   ViETest::Log("RTCP packets   : %5d", num_rtcp_packets);
-
-  // ***************************************************************
-  // Testing finished. Tear down Video Engine
-  // ***************************************************************
-  EXPECT_EQ(0, base_interface->StopSend(video_channel));
-  EXPECT_EQ(0, base_interface->StopReceive(video_channel));
-  EXPECT_EQ(0, network_interface->DeregisterSendTransport(video_channel));
-  EXPECT_EQ(0, render_interface->StopRender(capture_id));
-  EXPECT_EQ(0, render_interface->StopRender(video_channel));
-  EXPECT_EQ(0, render_interface->RemoveRenderer(capture_id));
-  EXPECT_EQ(0, render_interface->RemoveRenderer(video_channel));
-  EXPECT_EQ(0, capture_interface->DisconnectCaptureDevice(video_channel));
-  EXPECT_EQ(0, base_interface->DeleteChannel(video_channel));
 }
 
 void FixOutputFileForComparison(const std::string& output_file,
@@ -273,19 +306,19 @@ void FrameDropDetector::CalculateResults() {
   // Iterate over the maps from converted timestamps to the arrival timestamps.
   std::map<unsigned int, int64_t>::const_iterator it;
   for (it = sent_frames_.begin(); it != sent_frames_.end(); ++it) {
-    int created_timestamp = it->first - timestamp_diff_;
+    unsigned int created_timestamp = it->first - timestamp_diff_;
     created_frames_[created_timestamp]->sent_timestamp_in_us_ = it->second;
   }
   for (it = received_frames_.begin(); it != received_frames_.end(); ++it) {
-    int created_timestamp = it->first - timestamp_diff_;
+    unsigned int created_timestamp = it->first - timestamp_diff_;
     created_frames_[created_timestamp]->received_timestamp_in_us_ = it->second;
   }
   for (it = decoded_frames_.begin(); it != decoded_frames_.end(); ++it) {
-    int created_timestamp = it->first - timestamp_diff_;
+    unsigned int created_timestamp = it->first - timestamp_diff_;
     created_frames_[created_timestamp]->decoded_timestamp_in_us_ =it->second;
   }
   for (it = rendered_frames_.begin(); it != rendered_frames_.end(); ++it) {
-    int created_timestamp = it->first - timestamp_diff_;
+    unsigned int created_timestamp = it->first - timestamp_diff_;
     created_frames_[created_timestamp]->rendered_timestamp_in_us_ = it->second;
   }
   // Find out where the frames were not present in the different states.
@@ -435,7 +468,7 @@ void FrameDropDetector::PrintDebugDump() {
       "    Rendered ");
   for (std::vector<Frame*>::const_iterator it = created_frames_vector_.begin();
        it != created_frames_vector_.end(); ++it) {
-    ViETest::Log("%5d %11d %11d %11d %11d %11d %11d",
+    ViETest::Log("%5d %11u %11lld %11lld %11lld %11lld %11lld",
                  (*it)->number_,
                  (*it)->frame_timestamp_,
                  (*it)->created_timestamp_in_us_,
@@ -458,7 +491,7 @@ void FrameDropDetector::PrintDebugDump() {
     for (std::vector<int>::const_iterator it = mismatch_frame_num_list.begin();
          it != mismatch_frame_num_list.end(); ++it) {
       Frame* frame = created_frames_vector_[*it];
-      ViETest::Log("%5d %11d %11d %11d %11d %11d %11d",
+      ViETest::Log("%5d %11u %11lld %11lld %11lld %11lld %11lld",
                  frame->number_,
                  frame->frame_timestamp_,
                  frame->created_timestamp_in_us_,

webrtc/video_engine/test/auto_test/primitives/framedrop_primitives.h

@@ -35,7 +35,9 @@ void TestFullStack(const TbInterfaces& interfaces,
                    int bit_rate_kbps,
                    int packet_loss_percent,
                    int network_delay_ms,
-                   FrameDropDetector* frame_drop_detector);
+                   FrameDropDetector* frame_drop_detector,
+                   ViEToFileRenderer* remote_file_renderer,
+                   ViEToFileRenderer* local_file_renderer);
 
 // A frame in a video file. The four different points in the stack when
 // register the frame state are (in time order): created, transmitted, decoded,

webrtc/video_engine/test/auto_test/primitives/general_primitives.cc

@@ -89,6 +89,7 @@ void ConfigureRtpRtcp(webrtc::ViERTP_RTCP* rtcp_interface,
   EXPECT_EQ(0, rtcp_interface->SetKeyFrameRequestMethod(
       video_channel, webrtc::kViEKeyFrameRequestPliRtcp));
   EXPECT_EQ(0, rtcp_interface->SetTMMBRStatus(video_channel, true));
+  EXPECT_EQ(0, rtcp_interface->SetNACKStatus(video_channel, true));
 }
 
 bool FindSpecificCodec(webrtc::VideoCodecType of_type,

webrtc/video_engine/test/auto_test/source/vie_file_based_comparison_tests.cc

@@ -111,11 +111,10 @@ void ViEFileBasedComparisonTests::TestFullStack(
   EXPECT_EQ(0, interfaces.capture->ConnectCaptureDevice(
       capture_id, video_channel));
   ConfigureRtpRtcp(interfaces.rtp_rtcp, video_channel);
-  RenderToFile(interfaces.render, capture_id, local_file_renderer);
-  RenderToFile(interfaces.render, video_channel, remote_file_renderer);
 
   ::TestFullStack(interfaces, capture_id, video_channel, width, height,
                   bit_rate_kbps, packet_loss_percent, network_delay_ms,
-                  frame_drop_detector);
+                  frame_drop_detector, remote_file_renderer,
+                  local_file_renderer);
   EXPECT_TRUE(fake_camera.StopCamera());
 }

webrtc/video_engine/test/libvietest/include/tb_external_transport.h

@@ -98,6 +98,8 @@ public:
     void EnableSequenceNumberCheck();
     unsigned short GetFirstSequenceNumber();
 
+    bool EmptyQueue() const;
+
 protected:
     static bool ViEExternalTransportRun(void* object);
     bool ViEExternalTransportProcess();

webrtc/video_engine/test/libvietest/testbed/tb_external_transport.cc

@@ -327,6 +327,11 @@ unsigned short TbExternalTransport::GetFirstSequenceNumber()
     return _firstSequenceNumber;
 }
 
+bool TbExternalTransport::EmptyQueue() const {
+  webrtc::CriticalSectionScoped cs(&_crit);
+  return _rtpPackets.empty() && _rtcpPackets.empty();
+}
+
 bool TbExternalTransport::ViEExternalTransportRun(void* object)
 {
     return static_cast<TbExternalTransport*>
@@ -338,10 +343,10 @@ bool TbExternalTransport::ViEExternalTransportProcess()
 
     VideoPacket* packet = NULL;
 
+    _crit.Enter();
     while (!_rtpPackets.empty())
     {
         // Take first packet in queue
-        _crit.Enter();
         packet = _rtpPackets.front();
         WebRtc_Word64 timeToReceive = 0;
         if (packet)
@@ -418,11 +423,13 @@ bool TbExternalTransport::ViEExternalTransportProcess()
             delete packet;
             packet = NULL;
         }
+        _crit.Enter();
     }
+    _crit.Leave();
+    _crit.Enter();
     while (!_rtcpPackets.empty())
     {
         // Take first packet in queue
-        _crit.Enter();
         packet = _rtcpPackets.front();
         WebRtc_Word64 timeToReceive = 0;
         if (packet)
@@ -474,7 +481,9 @@ bool TbExternalTransport::ViEExternalTransportProcess()
             delete packet;
             packet = NULL;
         }
+        _crit.Enter();
     }
+    _crit.Leave();
     _event.Wait(waitTime + 1); // Add 1 ms to not call to early...
     return true;
 }