From 3bff8acc97cdbbe762a9d39ca19bf54d09dbbfcb Mon Sep 17 00:00:00 2001
From: Hill Ma <mahiuchun@users.noreply.github.com>
Date: Wed, 28 Jul 2021 16:32:38 -0700
Subject: [PATCH] Fix a typo in VideoEncoder_TEST. (#231)

Signed-off-by: Hill Ma <hillma@google.com>
Signed-off-by: Michael Carroll <michael@openrobotics.org>

Co-authored-by: Michael Carroll <michael@openrobotics.org>
---
 av/src/VideoEncoder_TEST.cc | 97 ++++++++++++++++++++++++++++++++-----
 1 file changed, 86 insertions(+), 11 deletions(-)

diff --git a/av/src/VideoEncoder_TEST.cc b/av/src/VideoEncoder_TEST.cc
index bc9ef3b83..495b11a7d 100644
--- a/av/src/VideoEncoder_TEST.cc
+++ b/av/src/VideoEncoder_TEST.cc
@@ -32,10 +32,9 @@ class VideoEncoderTest : public common::testing::AutoLogFixture
 };
 
 /////////////////////////////////////////////////
-TEST_F(VideoEncoderTest, StartStop)
+TEST_F(VideoEncoderTest, StartStopDefault)
 {
   auto filePathMp4 = common::joinPaths(common::cwd(), "TMP_RECORDING.mp4");
-  auto filePathMpg = common::joinPaths(common::cwd(), "TMP_RECORDING.mpg");
 
   {
     VideoEncoder video;
@@ -44,33 +43,109 @@ TEST_F(VideoEncoderTest, StartStop)
     EXPECT_EQ(video.BitRate(), static_cast<unsigned int>(
           VIDEO_ENCODER_BITRATE_DEFAULT));
 
-    video.Start();
+    EXPECT_TRUE(video.Start());
     EXPECT_TRUE(video.IsEncoding());
     EXPECT_TRUE(common::exists(filePathMp4)) << filePathMp4;
     EXPECT_EQ(video.BitRate(), 920000u);
 
-    video.Stop();
+    EXPECT_TRUE(video.Stop());
     EXPECT_FALSE(video.IsEncoding());
-    EXPECT_FALSE(common::exists(filePathMpg)) << filePathMpg;
+  }
 
-    video.Start("mpg", "", 1024, 768);
+  // Check that temp files are removed when video goes out of scope
+  EXPECT_FALSE(common::exists(filePathMp4)) << filePathMp4;
+}
+
+/////////////////////////////////////////////////
+TEST_F(VideoEncoderTest, StartStopMpg)
+{
+  auto filePathMpg = common::joinPaths(common::cwd(), "TMP_RECORDING.mpg");
+
+  {
+    VideoEncoder video;
+    EXPECT_FALSE(video.IsEncoding());
+    EXPECT_STREQ(video.Format().c_str(), VIDEO_ENCODER_FORMAT_DEFAULT);
+    EXPECT_EQ(video.BitRate(), static_cast<unsigned int>(
+          VIDEO_ENCODER_BITRATE_DEFAULT));
+
+    EXPECT_TRUE(video.Start("mpg", "", 1024, 768));
     EXPECT_TRUE(video.IsEncoding());
     EXPECT_STREQ(video.Format().c_str(), "mpg");
     EXPECT_TRUE(common::exists(filePathMpg)) << filePathMpg;
+    EXPECT_TRUE(video.Stop());
+    EXPECT_FALSE(video.IsEncoding());
+  }
 
-    video.Start("mp4", "", 1024, 768);
-    EXPECT_TRUE(video.IsEncoding());
-    EXPECT_STREQ(video.Format().c_str(), "mpg");
+  EXPECT_FALSE(common::exists(filePathMpg)) << filePathMpg;
+}
+
+
+/////////////////////////////////////////////////
+TEST_F(VideoEncoderTest, StartStopMp4)
+{
+  auto filePathMp4 = common::joinPaths(common::cwd(), "TMP_RECORDING.mp4");
 
+  {
+    VideoEncoder video;
+    EXPECT_FALSE(video.IsEncoding());
+    EXPECT_STREQ(video.Format().c_str(), VIDEO_ENCODER_FORMAT_DEFAULT);
+    EXPECT_EQ(video.BitRate(), static_cast<unsigned int>(
+          VIDEO_ENCODER_BITRATE_DEFAULT));
+
+    EXPECT_TRUE(video.Start("mp4", "", 1024, 768));
+    EXPECT_TRUE(video.IsEncoding());
+    EXPECT_STREQ(video.Format().c_str(), "mp4");
+    EXPECT_TRUE(common::exists(filePathMp4)) << filePathMp4;
     video.Stop();
     EXPECT_FALSE(video.IsEncoding());
   }
 
-  // Check that temp files are removed when video goes out of scope
   EXPECT_FALSE(common::exists(filePathMp4)) << filePathMp4;
-  EXPECT_FALSE(common::exists(filePathMpg)) << filePathMpg;
 }
 
+/////////////////////////////////////////////////
+TEST_F(VideoEncoderTest, RepeatedStart)
+{
+  auto filePathMpg = common::joinPaths(common::cwd(), "TMP_RECORDING.mpg");
+  auto filePathMp4 = common::joinPaths(common::cwd(), "TMP_RECORDING.mp4");
+
+  {
+    VideoEncoder video;
+    EXPECT_FALSE(video.IsEncoding());
+    EXPECT_STREQ(video.Format().c_str(), VIDEO_ENCODER_FORMAT_DEFAULT);
+    EXPECT_EQ(video.BitRate(), static_cast<unsigned int>(
+          VIDEO_ENCODER_BITRATE_DEFAULT));
+
+    EXPECT_TRUE(video.Start("mp4", "", 1024, 768));
+    EXPECT_TRUE(video.IsEncoding());
+    EXPECT_STREQ(video.Format().c_str(), "mp4");
+    EXPECT_TRUE(common::exists(filePathMp4)) << filePathMp4;
+
+    // Calling start again should return false and not mutate any
+    // internal state of the VideoEncoder
+    EXPECT_FALSE(video.Start("mpg", "", 1024, 768));
+    EXPECT_TRUE(video.IsEncoding());
+    EXPECT_STREQ(video.Format().c_str(), "mp4");
+    EXPECT_TRUE(common::exists(filePathMp4)) << filePathMp4;
+    EXPECT_FALSE(common::exists(filePathMpg)) << filePathMpg;
+
+    EXPECT_TRUE(video.Stop());
+    EXPECT_FALSE(video.IsEncoding());
+
+    // Once the VideoEncoder has been stopped, a new run may start.
+    EXPECT_TRUE(video.Start("mpg", "", 1024, 768));
+    EXPECT_TRUE(video.IsEncoding());
+    EXPECT_STREQ(video.Format().c_str(), "mpg");
+    EXPECT_FALSE(common::exists(filePathMp4)) << filePathMp4;
+    EXPECT_TRUE(common::exists(filePathMpg)) << filePathMpg;
+  }
+
+  // All temporary files will be removed after exiting scope.
+  EXPECT_FALSE(common::exists(filePathMp4)) << filePathMp4;
+  EXPECT_FALSE(common::exists(filePathMpg)) << filePathMp4;
+}
+
+
 /////////////////////////////////////////////////
 TEST_F(VideoEncoderTest, Exists)
 {