diff --git a/av/src/AudioDecoder_TEST.cc b/av/src/AudioDecoder_TEST.cc index 4f50c23fe..066b0b270 100644 --- a/av/src/AudioDecoder_TEST.cc +++ b/av/src/AudioDecoder_TEST.cc @@ -17,6 +17,7 @@ #include #include +#include #include "test/util.hh" #include "test_config.h" @@ -88,7 +89,7 @@ TEST(AudioDecoder, NoCodec) } ///////////////////////////////////////////////// -TEST(AudioDecoder, CheerFile) +TEST(AudioDecoder, IGN_UTILS_TEST_DISABLED_ON_WIN32(CheerFile)) { common::AudioDecoder audio; @@ -106,7 +107,7 @@ TEST(AudioDecoder, CheerFile) EXPECT_FALSE(audio.SetFile(path)); unsigned int dataBufferSize; - uint8_t *dataBuffer = NULL; + uint8_t *dataBuffer = nullptr; // WAV { diff --git a/av/src/CMakeLists.txt b/av/src/CMakeLists.txt index fb78e8c7a..3e95b259c 100644 --- a/av/src/CMakeLists.txt +++ b/av/src/CMakeLists.txt @@ -11,4 +11,8 @@ target_link_libraries(${av_target} AVUTIL::AVUTIL) ign_build_tests(TYPE UNIT SOURCES ${gtest_sources} - LIB_DEPS ${av_target}) + LIB_DEPS + ${av_target} + ignition-cmake${IGN_CMAKE_VER}::utilities +) + diff --git a/av/src/VideoEncoder.cc b/av/src/VideoEncoder.cc index 54e6b7b2a..883234600 100644 --- a/av/src/VideoEncoder.cc +++ b/av/src/VideoEncoder.cc @@ -23,6 +23,18 @@ #include "ignition/common/Console.hh" #include "ignition/common/VideoEncoder.hh" +// Fix averr2str +// https://github.com/joncampbell123/composite-video-simulator/issues/5#issuecomment-611885908 +#ifdef av_err2str +#undef av_err2str +av_always_inline char *av_err2str(int _errnum) +{ + thread_local char str[AV_ERROR_MAX_STRING_SIZE]; + memset(str, 0, sizeof(str)); + return av_make_error_string(str, AV_ERROR_MAX_STRING_SIZE, _errnum); +} +#endif + using namespace ignition; using namespace common; @@ -136,7 +148,14 @@ bool VideoEncoder::Start(const std::string &_format, // Remove old temp file, if it exists. if (common::exists(this->dataPtr->filename)) - std::remove(this->dataPtr->filename.c_str()); + { + auto success = removeFile(this->dataPtr->filename.c_str()); + if (!success) + { + ignerr << "Failed to remove temp file [" << this->dataPtr->filename + << "]" << std::endl; + } + } // Calculate a good bitrate if the _bitRate argument is zero if (_bitRate == 0) @@ -174,7 +193,7 @@ bool VideoEncoder::Start(const std::string &_format, this->dataPtr->frameCount = 0; this->dataPtr->filename = _filename; - // Create a default filenamae if the provided filename is empty. + // Create a default filename if the provided filename is empty. if (this->dataPtr->filename.empty()) { if (this->dataPtr->format.compare("v4l2") == 0) @@ -186,8 +205,8 @@ bool VideoEncoder::Start(const std::string &_format, } else { - this->dataPtr->filename = common::cwd() + "/TMP_RECORDING." + - this->dataPtr->format; + this->dataPtr->filename = joinPaths(common::cwd(), "TMP_RECORDING." + + this->dataPtr->format); } } @@ -212,8 +231,13 @@ bool VideoEncoder::Start(const std::string &_format, if (this->dataPtr->format.compare(outputFormat->name) == 0) { // Allocate the context using the correct outputFormat - avformat_alloc_output_context2(&this->dataPtr->formatCtx, + auto result = avformat_alloc_output_context2(&this->dataPtr->formatCtx, outputFormat, nullptr, this->dataPtr->filename.c_str()); + if (result < 0) + { + ignerr << "Failed to allocate AV context [" << av_err2str(result) + << "]" << std::endl; + } break; } } @@ -256,8 +280,13 @@ bool VideoEncoder::Start(const std::string &_format, #endif #else - avformat_alloc_output_context2(&this->dataPtr->formatCtx, nullptr, nullptr, - this->dataPtr->filename.c_str()); + auto result = avformat_alloc_output_context2(&this->dataPtr->formatCtx, + nullptr, nullptr, this->dataPtr->filename.c_str()); + if (result < 0) + { + ignerr << "Failed to allocate AV context [" << av_err2str(result) + << "]" << std::endl; + } #endif } @@ -768,7 +797,14 @@ void VideoEncoder::Reset() // Remove old temp file, if it exists. if (common::exists(this->dataPtr->filename)) - std::remove(this->dataPtr->filename.c_str()); + { + auto success = removeFile(this->dataPtr->filename.c_str()); + if (!success) + { + ignerr << "Failed to remove temp file [" << this->dataPtr->filename + << "]" << std::endl; + } + } // set default values this->dataPtr->frameCount = 0; @@ -780,4 +816,5 @@ void VideoEncoder::Reset() this->dataPtr->format = VIDEO_ENCODER_FORMAT_DEFAULT; this->dataPtr->timePrev = std::chrono::steady_clock::time_point(); this->dataPtr->timeStart = std::chrono::steady_clock::time_point(); + this->dataPtr->filename.clear(); } diff --git a/av/src/VideoEncoder_TEST.cc b/av/src/VideoEncoder_TEST.cc index f67dd868e..2d02d363d 100644 --- a/av/src/VideoEncoder_TEST.cc +++ b/av/src/VideoEncoder_TEST.cc @@ -15,65 +15,92 @@ */ #include +#include "ignition/common/Console.hh" #include "ignition/common/VideoEncoder.hh" +#include #include "test_config.h" #include "test/util.hh" using namespace ignition; using namespace common; -class VideoEncoderTest : public ignition::testing::AutoLogFixture { }; +class VideoEncoderTest : public ignition::testing::AutoLogFixture +{ + // Documentation inherited + protected: void SetUp() override + { + Console::SetVerbosity(4); + } +}; ///////////////////////////////////////////////// TEST_F(VideoEncoderTest, StartStop) { - VideoEncoder video; - EXPECT_FALSE(video.IsEncoding()); - EXPECT_STREQ(video.Format().c_str(), VIDEO_ENCODER_FORMAT_DEFAULT); - EXPECT_EQ(video.BitRate(), static_cast( - VIDEO_ENCODER_BITRATE_DEFAULT)); - auto filePathMp4 = common::joinPaths(common::cwd(), "TMP_RECORDING.mp4"); auto filePathMpg = common::joinPaths(common::cwd(), "TMP_RECORDING.mpg"); - video.Start(); - EXPECT_TRUE(video.IsEncoding()); - EXPECT_TRUE(common::exists(filePathMp4)); - EXPECT_EQ(video.BitRate(), 920000u); + { + VideoEncoder video; + EXPECT_FALSE(video.IsEncoding()); + EXPECT_STREQ(video.Format().c_str(), VIDEO_ENCODER_FORMAT_DEFAULT); + EXPECT_EQ(video.BitRate(), static_cast( + VIDEO_ENCODER_BITRATE_DEFAULT)); + + video.Start(); + EXPECT_TRUE(video.IsEncoding()); + EXPECT_TRUE(common::exists(filePathMp4)) << filePathMp4; + EXPECT_EQ(video.BitRate(), 920000u); + + video.Stop(); + EXPECT_FALSE(video.IsEncoding()); + EXPECT_FALSE(common::exists(filePathMpg)) << filePathMpg; - video.Stop(); - EXPECT_FALSE(video.IsEncoding()); - EXPECT_FALSE(common::exists(filePathMpg)); + video.Start("mpg", "", 1024, 768); + EXPECT_TRUE(video.IsEncoding()); + EXPECT_STREQ(video.Format().c_str(), "mpg"); + EXPECT_TRUE(common::exists(filePathMpg)) << filePathMpg; - video.Start("mpg", "", 1024, 768); - EXPECT_TRUE(video.IsEncoding()); - EXPECT_STREQ(video.Format().c_str(), "mpg"); - EXPECT_TRUE(common::exists(filePathMpg)); + video.Start("mp4", "", 1024, 768); + EXPECT_TRUE(video.IsEncoding()); + EXPECT_STREQ(video.Format().c_str(), "mpg"); - video.Start("mp4", "", 1024, 768); - EXPECT_TRUE(video.IsEncoding()); - EXPECT_STREQ(video.Format().c_str(), "mpg"); + video.Stop(); + EXPECT_FALSE(video.IsEncoding()); + } - video.Stop(); - EXPECT_FALSE(video.IsEncoding()); + // Check that temp files are removed when video goes out of scope + // Fails on Windows with `Permission denied` + // https://github.com/ignitionrobotics/ign-common/issues/148 +#ifndef _WIN32 + EXPECT_FALSE(common::exists(filePathMp4)) << filePathMp4; + EXPECT_FALSE(common::exists(filePathMpg)) << filePathMpg; +#endif } ///////////////////////////////////////////////// -TEST_F(VideoEncoderTest, Exists) +TEST_F(VideoEncoderTest, IGN_UTILS_TEST_DISABLED_ON_WIN32(Exists)) { - VideoEncoder video; - EXPECT_FALSE(video.IsEncoding()); - EXPECT_STREQ(video.Format().c_str(), VIDEO_ENCODER_FORMAT_DEFAULT); - auto filePathMp4 = common::joinPaths(common::cwd(), "TMP_RECORDING.mp4"); auto filePathMpg = common::joinPaths(common::cwd(), "TMP_RECORDING.mpg"); - EXPECT_FALSE(common::exists(filePathMp4)); - EXPECT_FALSE(common::exists(filePathMpg)); + { + VideoEncoder video; + EXPECT_FALSE(video.IsEncoding()); + EXPECT_STREQ(video.Format().c_str(), VIDEO_ENCODER_FORMAT_DEFAULT); + + EXPECT_FALSE(common::exists(filePathMp4)) << filePathMp4; + EXPECT_FALSE(common::exists(filePathMpg)) << filePathMpg; + + video.Start(); + EXPECT_TRUE(common::exists(filePathMp4)); - video.Start(); - EXPECT_TRUE(common::exists(filePathMp4)); + video.Reset(); + EXPECT_FALSE(common::exists(filePathMp4)); + } - video.Reset(); - EXPECT_FALSE(common::exists(filePathMp4)); + // Check that temp files are removed when video goes out of scope + // Fails on Windows with `Permission denied` + // https://github.com/ignitionrobotics/ign-common/issues/148 + EXPECT_FALSE(common::exists(filePathMp4)) << filePathMp4; + EXPECT_FALSE(common::exists(filePathMpg)) << filePathMpg; } diff --git a/graphics/src/CMakeLists.txt b/graphics/src/CMakeLists.txt index 000a0ce98..dcd41d8ef 100644 --- a/graphics/src/CMakeLists.txt +++ b/graphics/src/CMakeLists.txt @@ -22,7 +22,6 @@ target_link_libraries(${graphics_target} ign_build_tests(TYPE UNIT SOURCES ${gtest_sources} LIB_DEPS ${graphics_target}) - if(USE_EXTERNAL_TINYXML2) # If we are using an external copy of tinyxml2, add its imported target diff --git a/src/Filesystem_TEST.cc b/src/Filesystem_TEST.cc index 734208f0b..aaaff77f0 100644 --- a/src/Filesystem_TEST.cc +++ b/src/Filesystem_TEST.cc @@ -207,7 +207,7 @@ bool create_new_file_hardlink(const std::string &_hardlink, return ::CreateHardLinkA(_hardlink.c_str(), _target.c_str(), nullptr) == TRUE; } -#endif +#endif // _WIN32 #include // NOLINT #include "ignition/common/Console.hh"