From 3cb5e5bea261af3986ee15e99642ee0455623f39 Mon Sep 17 00:00:00 2001
From: Yves Gerey <yvesg@webrtc.org>
Date: Thu, 21 Mar 2019 11:22:42 +0100
Subject: [PATCH] Fix VideoFileWriterTest flakiness by using unique file path.

Both reference and tested videos were created
via a file whose path was fixed.
So, when tests were launched in parallel, race conditions ensued.
This CL creates an unique temporary filename for each video.

Bug: webrtc:10156
Change-Id: Ie3abf85abdfa95735cb86880bbd6a59393e609c1
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/127883
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Reviewed-by: Paulina Hensman <phensman@webrtc.org>
Commit-Queue: Yves Gerey <yvesg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#27219}
---
 rtc_tools/video_file_writer.cc          | 29 +++++++++-
 rtc_tools/video_file_writer.h           | 10 ++++
 rtc_tools/video_file_writer_unittest.cc | 74 ++++++++++++++++---------
 3 files changed, 86 insertions(+), 27 deletions(-)

diff --git a/rtc_tools/video_file_writer.cc b/rtc_tools/video_file_writer.cc
index e7a649fe8b..dc7a69953e 100644
--- a/rtc_tools/video_file_writer.cc
+++ b/rtc_tools/video_file_writer.cc
@@ -20,17 +20,19 @@
 
 namespace webrtc {
 namespace test {
+namespace {
 
 void WriteVideoToFile(const rtc::scoped_refptr<Video>& video,
                       const std::string& file_name,
-                      int fps) {
+                      int fps,
+                      bool isY4m) {
+  RTC_CHECK(video);
   FILE* output_file = fopen(file_name.c_str(), "wb");
   if (output_file == nullptr) {
     RTC_LOG(LS_ERROR) << "Could not open file for writing: " << file_name;
     return;
   }
 
-  bool isY4m = absl::EndsWith(file_name, ".y4m");
   if (isY4m) {
     fprintf(output_file, "YUV4MPEG2 W%d H%d F%d:1 C420\n", video->width(),
             video->height(), fps);
@@ -41,6 +43,8 @@ void WriteVideoToFile(const rtc::scoped_refptr<Video>& video,
       fwrite(frame.c_str(), 1, 6, output_file);
     }
     rtc::scoped_refptr<I420BufferInterface> buffer = video->GetFrame(i);
+    RTC_CHECK(buffer) << "Frame: " << i
+                      << "\nWhile trying to create: " << file_name;
     const uint8_t* data_y = buffer->DataY();
     int stride = buffer->StrideY();
     for (int i = 0; i < video->height(); ++i) {
@@ -63,5 +67,26 @@ void WriteVideoToFile(const rtc::scoped_refptr<Video>& video,
   fclose(output_file);
 }
 
+}  // Anonymous namespace
+
+void WriteVideoToFile(const rtc::scoped_refptr<Video>& video,
+                      const std::string& file_name,
+                      int fps) {
+  WriteVideoToFile(video, file_name, fps,
+                   /*isY4m=*/absl::EndsWith(file_name, ".y4m"));
+}
+
+void WriteY4mVideoToFile(const rtc::scoped_refptr<Video>& video,
+                         const std::string& file_name,
+                         int fps) {
+  WriteVideoToFile(video, file_name, fps, /*isY4m=*/true);
+}
+
+void WriteYuvVideoToFile(const rtc::scoped_refptr<Video>& video,
+                         const std::string& file_name,
+                         int fps) {
+  WriteVideoToFile(video, file_name, fps, /*isY4m=*/false);
+}
+
 }  // namespace test
 }  // namespace webrtc
diff --git a/rtc_tools/video_file_writer.h b/rtc_tools/video_file_writer.h
index ab8db81353..9fbb2844d2 100644
--- a/rtc_tools/video_file_writer.h
+++ b/rtc_tools/video_file_writer.h
@@ -23,6 +23,16 @@ void WriteVideoToFile(const rtc::scoped_refptr<Video>& video,
                       const std::string& file_name,
                       int fps);
 
+// Writes Y4M video to file.
+void WriteY4mVideoToFile(const rtc::scoped_refptr<Video>& video,
+                         const std::string& file_name,
+                         int fps);
+
+// Writes YUV video to file.
+void WriteYuvVideoToFile(const rtc::scoped_refptr<Video>& video,
+                         const std::string& file_name,
+                         int fps);
+
 }  // namespace test
 }  // namespace webrtc
 
diff --git a/rtc_tools/video_file_writer_unittest.cc b/rtc_tools/video_file_writer_unittest.cc
index 9a74b5f442..fea01bafef 100644
--- a/rtc_tools/video_file_writer_unittest.cc
+++ b/rtc_tools/video_file_writer_unittest.cc
@@ -24,11 +24,11 @@ namespace test {
 class VideoFileWriterTest : public ::testing::Test {
  public:
   void SetUp() override {
-    const std::string filename =
-        webrtc::test::OutputPath() + "test_video_file.y4m";
+    video_filename_ =
+        TempFilename(webrtc::test::OutputPath(), "test_video_file.y4m");
 
     // Create simple test video of size 6x4.
-    FILE* file = fopen(filename.c_str(), "wb");
+    FILE* file = fopen(video_filename_.c_str(), "wb");
     ASSERT_TRUE(file != nullptr);
     fprintf(file, "YUV4MPEG2 W6 H4 F60:1 C420 dummyParam\n");
     fprintf(file, "FRAME\n");
@@ -44,61 +44,85 @@ class VideoFileWriterTest : public ::testing::Test {
     fclose(file);
 
     // Open the newly created file.
-    video = webrtc::test::OpenY4mFile(filename);
-    ASSERT_TRUE(video);
+    video_ = webrtc::test::OpenY4mFile(video_filename_);
+    ASSERT_TRUE(video_);
+    ASSERT_EQ(video_->number_of_frames(), 2u);
+  }
+
+  void TearDown() override {
+    if (!video_filename_.empty()) {
+      RemoveFile(video_filename_);
+    }
+    if (!written_video_filename_.empty()) {
+      RemoveFile(written_video_filename_);
+    }
   }
 
   // Write and read Y4M file.
   void WriteVideoY4m() {
-    const std::string filename =
-        webrtc::test::OutputPath() + "test_video_file2.y4m";
-    webrtc::test::WriteVideoToFile(video, filename, fps);
-    written_video = webrtc::test::OpenY4mFile(filename);
-    ASSERT_TRUE(written_video);
+    // Cleanup existing file if any.
+    if (!written_video_filename_.empty()) {
+      RemoveFile(written_video_filename_);
+    }
+    // Create an unique filename, e.g. test_video_file2.y4mZapata.
+    written_video_filename_ =
+        TempFilename(webrtc::test::OutputPath(), "test_video_file2.y4m");
+    webrtc::test::WriteY4mVideoToFile(video_, written_video_filename_, fps);
+    written_video_ = webrtc::test::OpenY4mFile(written_video_filename_);
+    ASSERT_TRUE(written_video_);
   }
 
   // Write and read YUV file.
   void WriteVideoYuv() {
-    const std::string filename =
-        webrtc::test::OutputPath() + "test_video_file2.yuv";
-    webrtc::test::WriteVideoToFile(video, filename, fps);
-    written_video = webrtc::test::OpenYuvFile(filename, width, height);
-    ASSERT_TRUE(written_video);
+    // Cleanup existing file if any.
+    if (!written_video_filename_.empty()) {
+      RemoveFile(written_video_filename_);
+    }
+    // Create an unique filename, e.g. test_video_file2.yuvZapata.
+    written_video_filename_ =
+        TempFilename(webrtc::test::OutputPath(), "test_video_file2.yuv");
+    webrtc::test::WriteYuvVideoToFile(video_, written_video_filename_, fps);
+    written_video_ =
+        webrtc::test::OpenYuvFile(written_video_filename_, width, height);
+    ASSERT_TRUE(written_video_);
   }
 
   const int width = 6;
   const int height = 4;
   const int fps = 60;
-  rtc::scoped_refptr<webrtc::test::Video> video;
-  rtc::scoped_refptr<webrtc::test::Video> written_video;
+  rtc::scoped_refptr<webrtc::test::Video> video_;
+  rtc::scoped_refptr<webrtc::test::Video> written_video_;
+  // Each video object must be backed by file!
+  std::string video_filename_;
+  std::string written_video_filename_;
 };
 
 TEST_F(VideoFileWriterTest, TestParsingFileHeaderY4m) {
   WriteVideoY4m();
-  EXPECT_EQ(video->width(), written_video->width());
-  EXPECT_EQ(video->height(), written_video->height());
+  EXPECT_EQ(video_->width(), written_video_->width());
+  EXPECT_EQ(video_->height(), written_video_->height());
 }
 
 TEST_F(VideoFileWriterTest, TestParsingFileHeaderYuv) {
   WriteVideoYuv();
-  EXPECT_EQ(video->width(), written_video->width());
-  EXPECT_EQ(video->height(), written_video->height());
+  EXPECT_EQ(video_->width(), written_video_->width());
+  EXPECT_EQ(video_->height(), written_video_->height());
 }
 
 TEST_F(VideoFileWriterTest, TestParsingNumberOfFramesY4m) {
   WriteVideoY4m();
-  EXPECT_EQ(video->number_of_frames(), written_video->number_of_frames());
+  EXPECT_EQ(video_->number_of_frames(), written_video_->number_of_frames());
 }
 
 TEST_F(VideoFileWriterTest, TestParsingNumberOfFramesYuv) {
   WriteVideoYuv();
-  EXPECT_EQ(video->number_of_frames(), written_video->number_of_frames());
+  EXPECT_EQ(video_->number_of_frames(), written_video_->number_of_frames());
 }
 
 TEST_F(VideoFileWriterTest, TestPixelContentY4m) {
   WriteVideoY4m();
   int cnt = 0;
-  for (const rtc::scoped_refptr<I420BufferInterface> frame : *written_video) {
+  for (const rtc::scoped_refptr<I420BufferInterface> frame : *written_video_) {
     for (int i = 0; i < width * height; ++i, ++cnt)
       EXPECT_EQ(cnt, frame->DataY()[i]);
     for (int i = 0; i < width / 2 * height / 2; ++i, ++cnt)
@@ -111,7 +135,7 @@ TEST_F(VideoFileWriterTest, TestPixelContentY4m) {
 TEST_F(VideoFileWriterTest, TestPixelContentYuv) {
   WriteVideoYuv();
   int cnt = 0;
-  for (const rtc::scoped_refptr<I420BufferInterface> frame : *written_video) {
+  for (const rtc::scoped_refptr<I420BufferInterface> frame : *written_video_) {
     for (int i = 0; i < width * height; ++i, ++cnt)
       EXPECT_EQ(cnt, frame->DataY()[i]);
     for (int i = 0; i < width / 2 * height / 2; ++i, ++cnt)