Release branch for libopenshot 0.2.5 (SO 19)

[jonoomph]

Highlights

Fixed Broken Color-Shift and Shift Effects
Updated Documentation and Examples (C++)
Saturation Effect: Optimize and Parallelize
Hue Effect: Optimize and Parallelize
Blur Effect: Optimize and Parallelize
Wave Effect: Optimize and Parallelize
Brightness Effect: Optimize and Parallelize
Pixelate Effect: Rewrite effect to use QPainter/QRect
Frame: Fix interlaced AddImage
Raise Preview Cache to CPUs X 8 Frames (max 64)
FindRESVG CMake: Modernize with Targets
Enhance Json Data Handling

[codecov-io]

Codecov Report

❗ No coverage uploaded for pull request base (master@f9b4fe5). Click here to learn what that means.
The diff coverage is 33.97%.

@@            Coverage Diff            @@
##             master     #452   +/-   ##
=========================================
  Coverage          ?   43.22%           
=========================================
  Files             ?      130           
  Lines             ?    12932           
  Branches          ?        0           
=========================================
  Hits              ?     5590           
  Misses            ?     7342           
  Partials          ?        0

Impacted Files	Coverage Δ
include/effects/Shift.h	0% <ø> (ø)
include/ImageReader.h	25% <ø> (ø)
include/DummyReader.h	0% <ø> (ø)
include/WriterBase.h	100% <ø> (ø)
include/effects/Saturation.h	0% <ø> (ø)
include/effects/Brightness.h	0% <ø> (ø)
include/effects/Mask.h	0% <ø> (ø)
include/QtImageReader.h	33.33% <ø> (ø)
include/effects/Pixelate.h	0% <ø> (ø)
include/effects/Negate.h	100% <ø> (ø)
... and 68 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f9b4fe5...0910f22. Read the comment docs.

[jonoomph]

@ferdnyc Looks like ColorShift is broken. The effect params do not affect the image. I'm digging into it now, but I know you've touched this effect a bit during some of your JSON refactoring. Now sure when it stopped working though.

[ferdnyc]

Did I? I thought I did, too, but now all I can find is #443 which only changes the initial values.

I'm not home yet, but I will be in an hour or so. I'll take a look at the history, revert whatever needs reverting. (Or submit whatever unpushed commits I may have lying around, maybe I already fixed it locally! Who can say? (Certainly not me.)

[ferdnyc]

The other thing I did recently was correct the class name in its metadata. But there are still spots in both libopenshot and openshot-qt that refer to "Color Shift", two words. Maybe those need to be adjusted to follow the class_name change.

[ferdnyc]

@jonoomph Yup, name just needed to be fixed in EffectInfo. All sorted out now.

I'm surprised you even got the effect to apply at all. Before I fixed the name, I tried to drag a Color Shift effect onto a Clip and OpenShot threw this at me:

timeline_webview:INFO addEffect: ['ColorShift', 'ColorShift', 'ColorShift', 'ColorShift', 'ColorShift'] at PyQt5.QtCore.QPointF(213.0, 76.0)
timeline_webview:INFO Applying effect to clip
timeline_webview:INFO <classes.query.Clip object at 0x7f5b23e0fb90>
  exceptions:ERROR Unhandled Exception
Traceback (most recent call last):
  File "/home/ferd/rpmbuild/REPOS/openshot-qt/src/windows/views/timeline_webview.py", line 2855, in dropEvent
    self.addEffect(data, pos)
  File "/home/ferd/rpmbuild/REPOS/openshot-qt/src/windows/views/timeline_webview.py", line 2819, in addEffect
    effect.Id(get_app().project.generate_id())
AttributeError: 'NoneType' object has no attribute 'Id'

[ferdnyc]

Well, now I'm so confused, because apparently you already did that in c7fe363. But I didn't see the change there, and my commit to do it again applied. (Then again, I made that change on another branch, so maybe it was already here.)

At least in my testing, though, that does fix the effect. With EffectInfo edited to use "ColorShift" instead of "Color Shift", the effect #WorksForMe.

[ferdnyc]

( @jonoomph Maybe you tested a build off of develop, instead of this branch? The EffectInfo fix wasn't present there yet, I just merged #453 to remedy that.)

[jonoomph]

@ferdnyc Yeah, I changed the name in this release branch, once I noticed that. But even after that, I'm not seeing any adjustment to the Frame's image when sliding the ColorShift params around. It's very odd, and I'm not sure why yet. It appears all the values are getting to the Effect instance just fine though.

[jonoomph]

@ferdnyc Found my issue! In the Shift and ColorShift effects, abs() was truncating a value to 0. Needed fabs instead. Now all works great. But why did this work before? Very confused. Could this be related a build change perhaps? Maybe previous build flags were handling this invalid condition, but now it doesn't? No biggie though, fabs is what we needed.

[ferdnyc]

@ferdnyc Found my issue! In the Shift and ColorShift effects, abs() was truncating a value to 0. Needed fabs instead. Now all works great. But why did this work before? Very confused. Could this be related a build change perhaps? Maybe previous build flags were handling this invalid condition, but now it doesn't? No biggie though, fabs is what we needed.

I think I know how: It was the using namespace std; in the header(s). Watch:

https://coliru.stacked-crooked.com/a/8ba393c42db0a49e

Input

#include <iostream>
#include <cmath>
 
int main()
{

    std::cout << "\n\n";
    std::cout << "abs(0.001) = " << abs(0.001) << '\n'
              << "std::abs(0.001) = " << std::abs(0.001);

    using namespace std;
    
    std::cout << "\n\n";
    std::cout << "abs(0.001) = " << abs(0.001) << '\n'
              << "std::abs(0.001) = " << std::abs(0.001);
}

Output

g++ -std=c++11 -pthread  -O2 -Wall -Wextra -pedantic -pthread -pedantic-errors main.cpp -lm  -latomic  && ./a.out


abs(0.001) = 0
std::abs(0.001) = 0.001

abs(0.001) = 0.001
std::abs(0.001) = 0.001

[SuslikV]

@ferdnyc it is strange that by applying using namespace std; you are still using std::cout

[ferdnyc]

@ferdnyc it is strange that by applying using namespace std; you are still using std::cout

Not at all. Remember that I don't like using namespace std;, and it's never wrong to fully specify — then, your code works the way you expect it to regardless what foolish using namespace directives people have added to the code.

In fact, as I demonstrated, if the abs() calls had been written std::abs() to begin with, then they would've always worked correctly from the start, with or without using namespace std;.