Report minimum PSNR in VideoQualityTest and save corresponding frame to file

webrtc/video/video_quality_test.cc

Index: webrtc/video/video_quality_test.cc
diff --git a/webrtc/video/video_quality_test.cc b/webrtc/video/video_quality_test.cc
index 1719595d3a7e2fa3094ffae43918d8b7306a92ed..91106f422e026feb419dd680b438ba36c0762e90 100644
--- a/webrtc/video/video_quality_test.cc
+++ b/webrtc/video/video_quality_test.cc
@@ -45,6 +45,7 @@
 #include "webrtc/test/run_loop.h"
 #include "webrtc/test/statistics.h"
 #include "webrtc/test/testsupport/fileutils.h"
+#include "webrtc/test/testsupport/frame_writer.h"
 #include "webrtc/test/vcm_capturer.h"
 #include "webrtc/test/video_renderer.h"
 #include "webrtc/voice_engine/include/voe_base.h"
@@ -803,6 +804,11 @@ class VideoAnalyzer : public PacketReceiver,
     PrintResult("fec_bitrate", fec_bitrate_bps_, " bps");
     PrintResult("send_bandwidth", send_bandwidth_bps_, " bps");
 
+    if (min_psnr_) {
+      printf("RESULT min_psnr: %s = %lf dB\n", test_label_.c_str(),
+             *min_psnr_);
+    }
+
     if (receive_stream_ != nullptr) {
       PrintResult("decode_time", decode_time_ms_, " ms");
     }
@@ -819,6 +825,23 @@ class VideoAnalyzer : public PacketReceiver,
     PrintResult("memory_usage", memory_usage_, " bytes");
 #endif
 
+    if (bad_frame_) {
+      test::Y4mFrameWriterImpl frame_writer(test_label_ + ".y4m",
+                                            bad_frame_->width(),
+                                            bad_frame_->height(), 1);
+      frame_writer.Init();

[sprang_webrtc, 2017/07/24 14:24:09]

Maybe DCHECK the return value?

[ilnik, 2017/07/25 10:34:54]

Done.

+      size_t length =
+          CalcBufferSize(VideoType::kI420, bad_frame_->width(),
+                         bad_frame_->height());
+      rtc::Buffer extracted_buffer(length);
+      size_t extracted_length = ExtractBuffer(
+          bad_frame_->video_frame_buffer()->ToI420(), length,
+          extracted_buffer.data());
+      RTC_DCHECK_EQ(extracted_length, frame_writer.FrameLength());
+      frame_writer.WriteFrame(extracted_buffer.data());

[sprang_webrtc, 2017/07/24 14:24:09]

DCHECK return value

[ilnik, 2017/07/25 10:34:54]

Done.

+      frame_writer.Close();
+    }
+
     //  Disable quality check for quick test, as quality checks may fail
     //  because too few samples were collected.
     if (!is_quick_test_enabled_) {
@@ -837,6 +860,14 @@ class VideoAnalyzer : public PacketReceiver,
     }
 
     rtc::CritScope crit(&comparison_lock_);
+
+    if (psnr >= 0.0) {
+      if (!min_psnr_ || *min_psnr_ > psnr) {

[sprang_webrtc, 2017/07/24 14:24:09]

Merge these nested ifs into one

[ilnik, 2017/07/25 10:34:54]

Done.

+        min_psnr_.emplace(psnr);
+        bad_frame_.emplace(*comparison.render);
+      }
+    }
+
     if (graph_data_output_file_) {
       samples_.push_back(Sample(
           comparison.dropped, comparison.input_time_ms, comparison.send_time_ms,
@@ -1039,6 +1070,8 @@ class VideoAnalyzer : public PacketReceiver,
   test::Statistics fec_bitrate_bps_ GUARDED_BY(comparison_lock_);
   test::Statistics send_bandwidth_bps_ GUARDED_BY(comparison_lock_);
   test::Statistics memory_usage_ GUARDED_BY(comparison_lock_);
+  rtc::Optional<double> min_psnr_ GUARDED_BY(comparison_lock_);
+  rtc::Optional<VideoFrame> bad_frame_ GUARDED_BY(comparison_lock_);

[sprang_webrtc, 2017/07/24 14:24:09]

Maybe you could make a struct for these two instead. Comment in that on the
purpose of these two values, and have a single optional.

[ilnik, 2017/07/25 10:34:54]

Done.

 
   size_t last_fec_bytes_;