From 2f6d88e168f36719ba00fb6db779bc9ca06001b2 Mon Sep 17 00:00:00 2001 From: "FeRD (Frank Dana)" Date: Fri, 27 Nov 2020 00:38:04 -0500 Subject: [PATCH 1/4] various Readers: Fix GetCache return type Tests: fix ImageWriter cache type --- src/ImageReader.h | 4 +++- src/QtHtmlReader.h | 4 +++- src/QtImageReader.h | 4 +++- src/QtTextReader.h | 4 +++- tests/ImageWriter_Tests.cpp | 2 +- 5 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/ImageReader.h b/src/ImageReader.h index aa96272fd..2fcb963aa 100644 --- a/src/ImageReader.h +++ b/src/ImageReader.h @@ -48,6 +48,8 @@ namespace openshot { + // Forward decls + class CacheBase; /** * @brief This class uses the ImageMagick++ libraries, to open image files, and return @@ -90,7 +92,7 @@ namespace openshot void Close() override; /// Get the cache object used by this reader (always returns NULL for this object) - CacheMemory* GetCache() override { return NULL; }; + CacheBase* GetCache() override { return NULL; }; /// Get an openshot::Frame object for a specific frame number of this reader. All numbers /// return the same Frame, since they all share the same image data. diff --git a/src/QtHtmlReader.h b/src/QtHtmlReader.h index 3d426afb0..cd5a2750a 100644 --- a/src/QtHtmlReader.h +++ b/src/QtHtmlReader.h @@ -49,6 +49,8 @@ class QImage; namespace openshot { + // Forward decls + class CacheBase; /** * @brief This class uses Qt libraries, to create frames with rendered HTML, and return @@ -115,7 +117,7 @@ namespace openshot void Close() override; /// Get the cache object used by this reader (always returns NULL for this object) - openshot::CacheMemory* GetCache() override { return NULL; }; + CacheBase* GetCache() override { return NULL; }; /// Get an openshot::Frame object for a specific frame number of this reader. All numbers /// return the same Frame, since they all share the same image data. diff --git a/src/QtImageReader.h b/src/QtImageReader.h index 5162b8f45..b61ca3af3 100644 --- a/src/QtImageReader.h +++ b/src/QtImageReader.h @@ -42,6 +42,8 @@ namespace openshot { + // Forward decl + class CacheBase; /** * @brief This class uses the Qt library, to open image files, and return @@ -88,7 +90,7 @@ namespace openshot void Close() override; /// Get the cache object used by this reader (always returns NULL for this object) - CacheMemory* GetCache() override { return NULL; }; + CacheBase* GetCache() override { return NULL; }; /// Get an openshot::Frame object for a specific frame number of this reader. All numbers /// return the same Frame, since they all share the same image data. diff --git a/src/QtTextReader.h b/src/QtTextReader.h index 2fd203ad4..d66681f31 100644 --- a/src/QtTextReader.h +++ b/src/QtTextReader.h @@ -49,6 +49,8 @@ class QImage; namespace openshot { + // Forward decls + class CacheBase; /** * @brief This class uses Qt libraries, to create frames with "Text", and return @@ -126,7 +128,7 @@ namespace openshot void Close() override; /// Get the cache object used by this reader (always returns NULL for this object) - openshot::CacheMemory* GetCache() override { return NULL; }; + CacheBase* GetCache() override { return NULL; }; /// Get an openshot::Frame object for a specific frame number of this reader. All numbers /// return the same Frame, since they all share the same image data. diff --git a/tests/ImageWriter_Tests.cpp b/tests/ImageWriter_Tests.cpp index d74907bf3..e4e500ddf 100644 --- a/tests/ImageWriter_Tests.cpp +++ b/tests/ImageWriter_Tests.cpp @@ -85,7 +85,7 @@ TEST(Gif) // Basic Reader state queries CHECK_EQUAL("ImageReader", r1.Name()); - CacheMemory* c = r1.GetCache(); + CacheBase* c = r1.GetCache(); CHECK_EQUAL(true, c == nullptr); CHECK_EQUAL(false, r1.IsOpen()); From 9c1ca0ca735df3ea700744019fe9b045a9958a1f Mon Sep 17 00:00:00 2001 From: "FeRD (Frank Dana)" Date: Fri, 27 Nov 2020 03:12:08 -0500 Subject: [PATCH 2/4] Clip: Mark methods as overrides --- src/Clip.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Clip.h b/src/Clip.h index 70a09f6f9..aae93c27b 100644 --- a/src/Clip.h +++ b/src/Clip.h @@ -186,7 +186,7 @@ namespace openshot { void AddEffect(openshot::EffectBase* effect); /// Close the internal reader - void Close(); + void Close() override; /// Return the list of effects on the timeline std::list Effects() { return effects; }; @@ -199,7 +199,7 @@ namespace openshot { /// /// @returns A new openshot::Frame object /// @param frame_number The frame number (starting at 1) of the clip or effect on the timeline. - std::shared_ptr GetFrame(int64_t frame_number); + std::shared_ptr GetFrame(int64_t frame_number) override; /// @brief Get an openshot::Frame object for a specific frame number of this timeline. The image size and number /// of samples can be customized to match the Timeline, or any custom output. Extra samples will be moved to the From ac73bd965d21acc4d7e6f2a83c16f83ba002d7bb Mon Sep 17 00:00:00 2001 From: Jonathan Thomas Date: Thu, 3 Dec 2020 10:52:27 -0600 Subject: [PATCH 3/4] Fixing an issue caused by timeline::GetClip returning a ClipBase instead of a Clip (broke waveform generation). Also adding a swig definition for the Caption effect. --- bindings/python/openshot.i | 1 + bindings/ruby/openshot.i | 1 + src/Timeline.cpp | 2 +- src/Timeline.h | 2 +- 4 files changed, 4 insertions(+), 2 deletions(-) diff --git a/bindings/python/openshot.i b/bindings/python/openshot.i index b2c62c74c..7197f7dd8 100644 --- a/bindings/python/openshot.i +++ b/bindings/python/openshot.i @@ -219,6 +219,7 @@ %include "effects/Bars.h" %include "effects/Blur.h" %include "effects/Brightness.h" +%include "effects/Caption.h" %include "effects/ChromaKey.h" %include "effects/ColorShift.h" %include "effects/Crop.h" diff --git a/bindings/ruby/openshot.i b/bindings/ruby/openshot.i index e7a6fa7b7..fe47f1bce 100644 --- a/bindings/ruby/openshot.i +++ b/bindings/ruby/openshot.i @@ -206,6 +206,7 @@ %include "effects/Bars.h" %include "effects/Blur.h" %include "effects/Brightness.h" +%include "effects/Caption.h" %include "effects/ChromaKey.h" %include "effects/ColorShift.h" %include "effects/Crop.h" diff --git a/src/Timeline.cpp b/src/Timeline.cpp index abc02b6f8..d7377a280 100644 --- a/src/Timeline.cpp +++ b/src/Timeline.cpp @@ -289,7 +289,7 @@ void Timeline::RemoveClip(Clip* clip) } // Look up a clip -openshot::ClipBase* Timeline::GetClip(const std::string& id) +openshot::Clip* Timeline::GetClip(const std::string& id) { // Find the matching clip (if any) for (const auto& clip : clips) { diff --git a/src/Timeline.h b/src/Timeline.h index 1d156153b..e12cf3ee5 100644 --- a/src/Timeline.h +++ b/src/Timeline.h @@ -265,7 +265,7 @@ namespace openshot { std::list Clips() { return clips; }; /// Look up a single clip by ID - openshot::ClipBase* GetClip(const std::string& id); + openshot::Clip* GetClip(const std::string& id); /// Look up a clip effect by ID openshot::EffectBase* GetClipEffect(const std::string& id); From e8b4dde32ec693fe3a6d4e60287dc2532684d575 Mon Sep 17 00:00:00 2001 From: Frank Dana Date: Fri, 4 Dec 2020 09:25:30 -0500 Subject: [PATCH 4/4] Timeline::GetClip: Add anti-slicing unit tests (#596) --- tests/Timeline_Tests.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tests/Timeline_Tests.cpp b/tests/Timeline_Tests.cpp index 536b21334..0956366f3 100644 --- a/tests/Timeline_Tests.cpp +++ b/tests/Timeline_Tests.cpp @@ -437,20 +437,26 @@ TEST(GetClip_by_id) std::string clip2_id("CLIP00002"); clip2.Id(clip2_id); clip2.Layer(2); + clip2.Waveform(true); t.AddClip(&clip1); t.AddClip(&clip2); - auto matched = t.GetClip(clip1_id); + // We explicitly want to get returned a Clip*, here + Clip* matched = t.GetClip(clip1_id); CHECK_EQUAL(clip1_id, matched->Id()); CHECK_EQUAL(1, matched->Layer()); - auto matched2 = t.GetClip(clip2_id); + Clip* matched2 = t.GetClip(clip2_id); CHECK_EQUAL(clip2_id, matched2->Id()); CHECK_EQUAL(false, matched2->Layer() < 2); - auto matched3 = t.GetClip("BAD_ID"); + Clip* matched3 = t.GetClip("BAD_ID"); CHECK_EQUAL(true, matched3 == nullptr); + + // Ensure we can access the Clip API interfaces after lookup + CHECK_EQUAL(false, matched->Waveform()); + CHECK_EQUAL(true, matched2->Waveform()); } TEST(GetClipEffect_by_id)