From 0d29e734ac7618afb171c091150c35b52f6b5b63 Mon Sep 17 00:00:00 2001 From: Daniel Jour Date: Sat, 22 Feb 2020 09:57:57 +0100 Subject: [PATCH 01/12] Use REQURE CHECK to abort test if no Frame --- tests/Frame_Tests.cpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/Frame_Tests.cpp b/tests/Frame_Tests.cpp index a92906a3d..a925902a8 100644 --- a/tests/Frame_Tests.cpp +++ b/tests/Frame_Tests.cpp @@ -45,8 +45,7 @@ TEST(Default_Constructor) { // Create a "blank" default Frame std::shared_ptr f1(new Frame()); - - CHECK(f1 != nullptr); // Test aborts here if we didn't get a Frame + REQUIRE CHECK(f1 != nullptr); // Check basic default parameters CHECK_EQUAL(1, f1->GetHeight()); @@ -78,8 +77,7 @@ TEST(Data_Access) // Get first frame std::shared_ptr f1 = c1.GetFrame(1); - - CHECK(f1 != nullptr); + REQUIRE CHECK(f1 != nullptr); CHECK_EQUAL(1, f1->number); CHECK_EQUAL(1280, f1->GetWidth()); @@ -91,13 +89,13 @@ TEST(AddImage_QImage) { // Create a "blank" default Frame std::shared_ptr f1(new Frame()); + REQUIRE CHECK(f1 != nullptr); // Load an image std::stringstream path; path << TEST_MEDIA_PATH << "front.png"; std::shared_ptr i1(new QImage(QString::fromStdString(path.str()))) ; - CHECK(f1 != nullptr); // Test aborts here if we didn't get a Frame CHECK_EQUAL(false, i1->isNull()); f1->AddImage(i1); From 529fc1110db8b40a02c82080f510aa271703c6b9 Mon Sep 17 00:00:00 2001 From: Daniel Jour Date: Sat, 22 Feb 2020 10:05:01 +0100 Subject: [PATCH 02/12] Remove unneccessary indirection through smart pointer --- tests/Frame_Tests.cpp | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/tests/Frame_Tests.cpp b/tests/Frame_Tests.cpp index a925902a8..9ce9cfa00 100644 --- a/tests/Frame_Tests.cpp +++ b/tests/Frame_Tests.cpp @@ -44,26 +44,25 @@ SUITE(Frame_Tests) TEST(Default_Constructor) { // Create a "blank" default Frame - std::shared_ptr f1(new Frame()); - REQUIRE CHECK(f1 != nullptr); + Frame f1; // Check basic default parameters - CHECK_EQUAL(1, f1->GetHeight()); - CHECK_EQUAL(1, f1->GetWidth()); - CHECK_EQUAL(44100, f1->SampleRate()); - CHECK_EQUAL(2, f1->GetAudioChannelsCount()); + CHECK_EQUAL(1, f1.GetHeight()); + CHECK_EQUAL(1, f1.GetWidth()); + CHECK_EQUAL(44100, f1.SampleRate()); + CHECK_EQUAL(2, f1.GetAudioChannelsCount()); // Should be false until we load or create contents - CHECK_EQUAL(false, f1->has_image_data); - CHECK_EQUAL(false, f1->has_audio_data); + CHECK_EQUAL(false, f1.has_image_data); + CHECK_EQUAL(false, f1.has_audio_data); // Calling GetImage() paints a blank frame, by default - std::shared_ptr i1 = f1->GetImage(); + std::shared_ptr i1 = f1.GetImage(); CHECK(i1 != nullptr); - CHECK_EQUAL(true,f1->has_image_data); - CHECK_EQUAL(false,f1->has_audio_data); + CHECK_EQUAL(true,f1.has_image_data); + CHECK_EQUAL(false,f1.has_audio_data); } @@ -88,8 +87,7 @@ TEST(Data_Access) TEST(AddImage_QImage) { // Create a "blank" default Frame - std::shared_ptr f1(new Frame()); - REQUIRE CHECK(f1 != nullptr); + Frame f1; // Load an image std::stringstream path; @@ -98,12 +96,12 @@ TEST(AddImage_QImage) CHECK_EQUAL(false, i1->isNull()); - f1->AddImage(i1); + f1.AddImage(i1); // Check loaded image parameters - CHECK_EQUAL(i1->height(), f1->GetHeight()); - CHECK_EQUAL(i1->width(), f1->GetWidth()); - CHECK_EQUAL(true, f1->has_image_data); + CHECK_EQUAL(i1->height(), f1.GetHeight()); + CHECK_EQUAL(i1->width(), f1.GetWidth()); + CHECK_EQUAL(true, f1.has_image_data); } From 3edda09c1e3f15ff52d928c43d4e1bedbaabe7d1 Mon Sep 17 00:00:00 2001 From: Daniel Jour Date: Sat, 22 Feb 2020 10:08:58 +0100 Subject: [PATCH 03/12] Split GetImage call into separate test --- tests/Frame_Tests.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/Frame_Tests.cpp b/tests/Frame_Tests.cpp index 9ce9cfa00..23342599c 100644 --- a/tests/Frame_Tests.cpp +++ b/tests/Frame_Tests.cpp @@ -55,8 +55,13 @@ TEST(Default_Constructor) // Should be false until we load or create contents CHECK_EQUAL(false, f1.has_image_data); CHECK_EQUAL(false, f1.has_audio_data); +} - // Calling GetImage() paints a blank frame, by default +TEST(GetImage) +{ + Frame f1; + // Calling GetImage() paints a blank frame, by default. + // It also adds image data to the frame! std::shared_ptr i1 = f1.GetImage(); CHECK(i1 != nullptr); From c79ded25b1d89cc7d6497b8f65e81fb4cd1a12c1 Mon Sep 17 00:00:00 2001 From: Daniel Jour Date: Sat, 22 Feb 2020 10:21:54 +0100 Subject: [PATCH 04/12] Add test for image only constructor - Some tests are commented out; behavior is strange / unexpected / inconsistent at times; this probably needs to be fixed, thus do not capture it in test for now. --- tests/Frame_Tests.cpp | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/Frame_Tests.cpp b/tests/Frame_Tests.cpp index 23342599c..7b8fa0fe5 100644 --- a/tests/Frame_Tests.cpp +++ b/tests/Frame_Tests.cpp @@ -57,6 +57,21 @@ TEST(Default_Constructor) CHECK_EQUAL(false, f1.has_audio_data); } +TEST(ImageOnly_Constructor) +{ + Frame f(42, 100, 111, "#FAFBFC"); + + CHECK_EQUAL(100, f.GetWidth()); + CHECK_EQUAL(111, f.GetHeight()); + CHECK_EQUAL(44100, f.SampleRate()); + CHECK_EQUAL(2, f.GetAudioChannelsCount()); + + CHECK_EQUAL(false, f.has_audio_data); + // CHECK_EQUAL(true, f.has_image_data); // TODO: This would be expected? + // CHECK(f.GetPixels(0)); // TODO: crashes + // CHECK(f.CheckPixel(0, 0, 0xFA, 0xFB, 0xFC, 0, 0)); +} + TEST(GetImage) { Frame f1; From e18523c714bbbdf51fad9a16aca6a74243b934f1 Mon Sep 17 00:00:00 2001 From: Daniel Jour Date: Sat, 22 Feb 2020 10:27:56 +0100 Subject: [PATCH 05/12] Test audio only constructor - Strange / unexpected / probably wrong behavior; this tests needs to be expanded once there's a clear API --- tests/Frame_Tests.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/Frame_Tests.cpp b/tests/Frame_Tests.cpp index 7b8fa0fe5..8edabe67c 100644 --- a/tests/Frame_Tests.cpp +++ b/tests/Frame_Tests.cpp @@ -72,6 +72,17 @@ TEST(ImageOnly_Constructor) // CHECK(f.CheckPixel(0, 0, 0xFA, 0xFB, 0xFC, 0, 0)); } +TEST(AudioOnly_Constructor) +{ + Frame f(42, 100, 1); + CHECK_EQUAL(1, f.GetHeight()); + CHECK_EQUAL(1, f.GetWidth()); + CHECK_EQUAL(44100, f.SampleRate()); + CHECK_EQUAL(1, f.GetAudioChannelsCount()); + + // CHECK_EQUAL(true, f.has_audio_data); // TODO: This would be expected? +} + TEST(GetImage) { Frame f1; From 52256028fd497ac36084461b9cc56a9a1ca4a523 Mon Sep 17 00:00:00 2001 From: Daniel Jour Date: Sat, 22 Feb 2020 10:32:53 +0100 Subject: [PATCH 06/12] Test for SetFrameNumber --- tests/Frame_Tests.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/Frame_Tests.cpp b/tests/Frame_Tests.cpp index 8edabe67c..c6a737b43 100644 --- a/tests/Frame_Tests.cpp +++ b/tests/Frame_Tests.cpp @@ -83,6 +83,15 @@ TEST(AudioOnly_Constructor) // CHECK_EQUAL(true, f.has_audio_data); // TODO: This would be expected? } +TEST(SetFrameNumber) +{ + Frame f; + f.SetFrameNumber(42); + CHECK_EQUAL(42, f.number); + f.SetFrameNumber(21); + CHECK_EQUAL(21, f.number); +} + TEST(GetImage) { Frame f1; From 94277d2da6e3e77627cb05ca9f8a9edfab8e8735 Mon Sep 17 00:00:00 2001 From: Daniel Jour Date: Sat, 22 Feb 2020 10:35:21 +0100 Subject: [PATCH 07/12] Test sample rate change - This test needs to be expanded to test how (if) this affects underlying audio data. --- tests/Frame_Tests.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/Frame_Tests.cpp b/tests/Frame_Tests.cpp index c6a737b43..00427b66c 100644 --- a/tests/Frame_Tests.cpp +++ b/tests/Frame_Tests.cpp @@ -92,6 +92,14 @@ TEST(SetFrameNumber) CHECK_EQUAL(21, f.number); } +TEST(SampleRate_Change) +{ + Frame f; + f.SampleRate(44800); + CHECK_EQUAL(44800, f.SampleRate()); + // TODO: Test how this affects audio data? +} + TEST(GetImage) { Frame f1; From 094cce4532fa9270e12baaab81a03c04de52b90a Mon Sep 17 00:00:00 2001 From: Daniel Jour Date: Sat, 22 Feb 2020 10:40:52 +0100 Subject: [PATCH 08/12] Correct comments regarding default values in Frame.h --- include/Frame.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/include/Frame.h b/include/Frame.h index f4ff54d44..1a5aa03e5 100644 --- a/include/Frame.h +++ b/include/Frame.h @@ -73,17 +73,17 @@ namespace openshot * There are many ways to create an instance of an openshot::Frame: * @code * - * // Most basic: a blank frame (300x200 blank image, 48kHz audio silence) + * // Most basic: a blank frame (1x1 blank image, 44.2kHz audio silence) * Frame(); * - * // Image only settings (48kHz audio silence) + * // Image only settings (44.2kHz audio silence) * Frame(1, // Frame number * 720, // Width of image * 480, // Height of image * "#000000" // HTML color code of background color * ); * - * // Audio only (300x200 blank image) + * // Audio only (1x1 blank image) * Frame(number, // Frame number * 44100, // Sample rate of audio stream * 2 // Number of audio channels @@ -131,13 +131,13 @@ namespace openshot bool has_image_data; ///< This frame has been loaded with pixel data - /// Constructor - blank frame (300x200 blank image, 48kHz audio silence) + /// Constructor - blank frame (1x1 blank image, 44.2kHz audio silence) Frame(); - /// Constructor - image only (48kHz audio silence) + /// Constructor - image only (44.2kHz audio silence) Frame(int64_t number, int width, int height, std::string color); - /// Constructor - audio only (300x200 blank image) + /// Constructor - audio only (1x1 blank image) Frame(int64_t number, int samples, int channels); /// Constructor - image & audio From e428d2ddfeb4c5122ae2caecb231f6101a1f6d39 Mon Sep 17 00:00:00 2001 From: "FeRD (Frank Dana)" Date: Sat, 22 Feb 2020 17:59:43 -0500 Subject: [PATCH 09/12] Frame_Tests: Support older UnitTest++ --- tests/Frame_Tests.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/Frame_Tests.cpp b/tests/Frame_Tests.cpp index 00427b66c..8cc911420 100644 --- a/tests/Frame_Tests.cpp +++ b/tests/Frame_Tests.cpp @@ -30,6 +30,11 @@ */ #include "UnitTest++.h" +// Work around older versions of UnitTest++ without REQUIRE +#ifndef REQUIRE + #define REQUIRE +#endif + // Prevent name clashes with juce::UnitTest #define DONT_SET_USING_JUCE_NAMESPACE 1 #include "../include/OpenShot.h" From 86f0f422e4940cbf213509cb0162ed1b710e1542 Mon Sep 17 00:00:00 2001 From: Daniel Jour Date: Sat, 22 Feb 2020 10:40:52 +0100 Subject: [PATCH 10/12] Correct comments regarding default values in Frame.h --- include/Frame.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/include/Frame.h b/include/Frame.h index f4ff54d44..a122b2122 100644 --- a/include/Frame.h +++ b/include/Frame.h @@ -73,17 +73,17 @@ namespace openshot * There are many ways to create an instance of an openshot::Frame: * @code * - * // Most basic: a blank frame (300x200 blank image, 48kHz audio silence) + * // Most basic: a blank frame (1x1 blank image, 44.1kHz audio silence) * Frame(); * - * // Image only settings (48kHz audio silence) + * // Image only settings (44.1kHz audio silence) * Frame(1, // Frame number * 720, // Width of image * 480, // Height of image * "#000000" // HTML color code of background color * ); * - * // Audio only (300x200 blank image) + * // Audio only (1x1 blank image) * Frame(number, // Frame number * 44100, // Sample rate of audio stream * 2 // Number of audio channels @@ -131,13 +131,13 @@ namespace openshot bool has_image_data; ///< This frame has been loaded with pixel data - /// Constructor - blank frame (300x200 blank image, 48kHz audio silence) + /// Constructor - blank frame (1x1 blank image, 44.1kHz audio silence) Frame(); - /// Constructor - image only (48kHz audio silence) + /// Constructor - image only (44.1kHz audio silence) Frame(int64_t number, int width, int height, std::string color); - /// Constructor - audio only (300x200 blank image) + /// Constructor - audio only (1x1 blank image) Frame(int64_t number, int samples, int channels); /// Constructor - image & audio From 92e52a7077269084df8e6122d356580c16b39b51 Mon Sep 17 00:00:00 2001 From: "FeRD (Frank Dana)" Date: Sat, 22 Feb 2020 17:59:43 -0500 Subject: [PATCH 11/12] Frame_Tests: Support older UnitTest++ --- tests/Frame_Tests.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/Frame_Tests.cpp b/tests/Frame_Tests.cpp index 00427b66c..8cc911420 100644 --- a/tests/Frame_Tests.cpp +++ b/tests/Frame_Tests.cpp @@ -30,6 +30,11 @@ */ #include "UnitTest++.h" +// Work around older versions of UnitTest++ without REQUIRE +#ifndef REQUIRE + #define REQUIRE +#endif + // Prevent name clashes with juce::UnitTest #define DONT_SET_USING_JUCE_NAMESPACE 1 #include "../include/OpenShot.h" From 113317eac3707a92bfae1b51701bb1bdb8534e71 Mon Sep 17 00:00:00 2001 From: "FeRD (Frank Dana)" Date: Tue, 10 Mar 2020 22:33:38 -0400 Subject: [PATCH 12/12] WIP: Frame tests --- tests/Frame_Tests.cpp | 45 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/tests/Frame_Tests.cpp b/tests/Frame_Tests.cpp index 8cc911420..b4ab7f8da 100644 --- a/tests/Frame_Tests.cpp +++ b/tests/Frame_Tests.cpp @@ -46,6 +46,35 @@ using namespace openshot; SUITE(Frame_Tests) { +class FrameFixture +{ +protected: + Clip* testClip; +public: + Frame* testFrame; + + FrameFixture() { + // Open our test video clip + std::stringstream path; + path << TEST_MEDIA_PATH << "sintel_trailer-720p.mp4"; + testClip = new Clip(path.str()); + testClip->Open(); + } + + ~FrameFixture() { + testClip->Close(); + delete testClip; + if (testFrame != nullptr) { + delete testFrame; + } + } + + void LoadFrame() { + // Extract and return frame 125. + testFrame = testClip->GetFrame(125).get(); + } +}; + TEST(Default_Constructor) { // Create a "blank" default Frame @@ -196,4 +225,20 @@ TEST(Copy_Constructor) CHECK_EQUAL(f1.GetAudioSamplesCount(), f2.GetAudioSamplesCount()); } +#ifdef USE_IMAGEMAGICK + +TEST_FIXTURE(FrameFixture, GetMagickImage) { + LoadFrame(); + Frame f = *testFrame; + + REQUIRE CHECK_EQUAL(125, f.number); + + auto magick = f.GetMagickImage(); + + CHECK_EQUAL(f.GetWidth(), magick->columns()); + CHECK_EQUAL(f.GetHeight(), magick->rows()); +} + +#endif // USE_IMAGEMAGICK + } // SUITE(Frame_Tests)