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}diff --git a/rtc_tools/video_file_writer.cc b/rtc_tools/video_file_writer.cc
index e7a649f..dc7a699 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 @@
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 @@
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 ab8db81..9fbb284 100644
--- a/rtc_tools/video_file_writer.h
+++ b/rtc_tools/video_file_writer.h
@@ -23,6 +23,16 @@
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 9a74b5f..fea01ba 100644
--- a/rtc_tools/video_file_writer_unittest.cc
+++ b/rtc_tools/video_file_writer_unittest.cc
@@ -24,11 +24,11 @@
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 @@
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, 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)
