From 06d45cb88aed5af39c08142147169eced2cf7f2b Mon Sep 17 00:00:00 2001 From: "FeRD (Frank Dana)" Date: Sat, 17 Oct 2020 05:56:02 -0400 Subject: [PATCH 1/4] ImageReader: Consolidate ctors using default arg --- include/ImageReader.h | 18 +++++++++--------- src/ImageReader.cpp | 9 +-------- 2 files changed, 10 insertions(+), 17 deletions(-) diff --git a/include/ImageReader.h b/include/ImageReader.h index 5aafcc8f7..aa96272fd 100644 --- a/include/ImageReader.h +++ b/include/ImageReader.h @@ -76,15 +76,15 @@ namespace openshot bool is_open; public: - - /// Constructor for ImageReader. This automatically opens the media file and loads - /// frame 1, or it throws one of the following exceptions. - ImageReader(std::string path); - - /// Constructor for ImageReader. This only opens the media file to inspect its properties - /// if inspect_reader=true. When not inspecting the media file, it's much faster, and useful - /// when you are inflating the object using JSON after instantiating it. - ImageReader(std::string path, bool inspect_reader); + /// @brief Constructor for ImageReader. + /// + /// Opens the media file to inspect its properties and loads frame 1, + /// iff inspect_reader == true (the default). Pass a false value in + /// the optional parameter to defer this initial Open()/Close() cycle. + /// + /// When not inspecting the media file, it's much faster, and useful + /// when you are inflating the object using JSON after instantiation. + ImageReader(const std::string& path, bool inspect_reader=true); /// Close File void Close() override; diff --git a/src/ImageReader.cpp b/src/ImageReader.cpp index 9a001879f..2a4d411ed 100644 --- a/src/ImageReader.cpp +++ b/src/ImageReader.cpp @@ -35,14 +35,7 @@ using namespace openshot; -ImageReader::ImageReader(std::string path) : path(path), is_open(false) -{ - // Open and Close the reader, to populate its attributes (such as height, width, etc...) - Open(); - Close(); -} - -ImageReader::ImageReader(std::string path, bool inspect_reader) : path(path), is_open(false) +ImageReader::ImageReader(const std::string& path, bool inspect_reader) : path(path), is_open(false) { // Open and Close the reader, to populate its attributes (such as height, width, etc...) if (inspect_reader) { From 6e4476dc6c34029544cc01eb660a954d6a76ed4d Mon Sep 17 00:00:00 2001 From: "FeRD (Frank Dana)" Date: Sat, 17 Oct 2020 05:56:39 -0400 Subject: [PATCH 2/4] Tests: Increase coverage for ImageReader/Writer --- tests/ImageWriter_Tests.cpp | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/tests/ImageWriter_Tests.cpp b/tests/ImageWriter_Tests.cpp index bdf73a07d..6d3a2e229 100644 --- a/tests/ImageWriter_Tests.cpp +++ b/tests/ImageWriter_Tests.cpp @@ -37,17 +37,33 @@ using namespace std; using namespace openshot; #ifdef USE_IMAGEMAGICK -TEST(ImageWriter_Test_Gif) +SUITE(ImageWriter) { - // Reader + +TEST(Gif) +{ + // Reader --------------- + + // Bad path + FFmpegReader bad_r("/tmp/bleeblorp.xls", false); + CHECK_THROW(bad_r.Open(), InvalidFile); + + // Good path stringstream path; path << TEST_MEDIA_PATH << "sintel_trailer-720p.mp4"; FFmpegReader r(path.str()); + + // Read-before-open error + CHECK_THROW(r.GetFrame(1), ReaderClosed); + r.Open(); /* WRITER ---------------- */ ImageWriter w("output1.gif"); + // Check for exception on write-before-open + CHECK_THROW(w.WriteFrame(&r, 500, 504), WriterClosed); + // Set the image output settings (format, fps, width, height, quality, loops, combine) w.SetVideoOptions("GIF", r.info.fps, r.info.width, r.info.height, 70, 1, true); @@ -82,4 +98,6 @@ TEST(ImageWriter_Test_Gif) CHECK_CLOSE(11, (int)pixels[pixel_index + 2], 5); CHECK_CLOSE(255, (int)pixels[pixel_index + 3], 5); } + +} // SUITE #endif From 6c656dd7f7c1f54b9d71cb428d1c9f32ecef1a7b Mon Sep 17 00:00:00 2001 From: "FeRD (Frank Dana)" Date: Sat, 17 Oct 2020 06:23:44 -0400 Subject: [PATCH 3/4] QtImageReader: Consolidate ctors --- include/QtImageReader.h | 18 +++++++++--------- src/QtImageReader.cpp | 7 ------- 2 files changed, 9 insertions(+), 16 deletions(-) diff --git a/include/QtImageReader.h b/include/QtImageReader.h index 3848065a1..5162b8f45 100644 --- a/include/QtImageReader.h +++ b/include/QtImageReader.h @@ -72,15 +72,15 @@ namespace openshot QSize max_size; ///> Current max_size as calculated with Clip properties public: - - /// Constructor for QtImageReader. This automatically opens the media file and loads - /// frame 1, or it throws one of the following exceptions. - QtImageReader(std::string path); - - /// Constructor for QtImageReader. This only opens the media file to inspect its properties - /// if inspect_reader=true. When not inspecting the media file, it's much faster, and useful - /// when you are inflating the object using JSON after instantiating it. - QtImageReader(std::string path, bool inspect_reader); + /// @brief Constructor for QtImageReader. + /// + /// Opens the media file to inspect its properties and loads frame 1, + /// iff inspect_reader == true (the default). Pass a false value in + /// the optional parameter to defer this initial Open()/Close() cycle. + /// + /// When not inspecting the media file, it's much faster, and useful + /// when you are inflating the object using JSON after instantiation. + QtImageReader(std::string path, bool inspect_reader=true); virtual ~QtImageReader(); diff --git a/src/QtImageReader.cpp b/src/QtImageReader.cpp index 2e8d06426..78c2b009a 100644 --- a/src/QtImageReader.cpp +++ b/src/QtImageReader.cpp @@ -44,13 +44,6 @@ using namespace openshot; -QtImageReader::QtImageReader(std::string path) : path{QString::fromStdString(path)}, is_open(false) -{ - // Open and Close the reader, to populate its attributes (such as height, width, etc...) - Open(); - Close(); -} - QtImageReader::QtImageReader(std::string path, bool inspect_reader) : path{QString::fromStdString(path)}, is_open(false) { // Open and Close the reader, to populate its attributes (such as height, width, etc...) From aff469535fea1c93362b8fd4c41f8a3ec74a9a06 Mon Sep 17 00:00:00 2001 From: "FeRD (Frank Dana)" Date: Sun, 18 Oct 2020 10:22:34 -0400 Subject: [PATCH 4/4] ImageReader/Writer.h: Goose coverage to 100% --- tests/ImageWriter_Tests.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/ImageWriter_Tests.cpp b/tests/ImageWriter_Tests.cpp index 6d3a2e229..eb7b31e98 100644 --- a/tests/ImageWriter_Tests.cpp +++ b/tests/ImageWriter_Tests.cpp @@ -61,6 +61,8 @@ TEST(Gif) /* WRITER ---------------- */ ImageWriter w("output1.gif"); + CHECK_EQUAL(false, w.IsOpen()); + // Check for exception on write-before-open CHECK_THROW(w.WriteFrame(&r, 500, 504), WriterClosed); @@ -79,7 +81,16 @@ TEST(Gif) // Open up the 5th frame from the newly created GIF ImageReader r1("output1.gif[4]"); + + // Basic Reader state queries + CHECK_EQUAL("ImageReader", r1.Name()); + + CacheMemory* c = r1.GetCache(); + CHECK_EQUAL(true, c == nullptr); + + CHECK_EQUAL(false, r1.IsOpen()); r1.Open(); + CHECK_EQUAL(true, r1.IsOpen()); // Verify various settings CHECK_EQUAL(r.info.width, r1.info.width);