Bug 368681

Summary:	git master 2016-09-12:Applying speed effect causes corruption in fades
Product:	[Applications] kdenlive	Reporter:	Evert Vorster <evorster>
Component:	User Interface & Miscellaneous	Assignee:	Jean-Baptiste Mardelle <jb>
Status:	RESOLVED FIXED
Severity:	major	CC:	wegwerf-1-2-3
Priority:	NOR
Version First Reported In:	unspecified
Target Milestone:	---
Platform:	Compiled Sources
OS:	Linux
Latest Commit:		Version Fixed In:
Sentry Crash Report:
Attachments:	Project with the corruption Render of the first four clips Change clip speed job

Description Evert Vorster 2016-09-12 09:37:33 UTC

I have this rather long project in which I am mixing 30fps and 25fps footage.
I have decided to slow down the 30fps footage by 83% so that I do not get the pulldown effect.
The actual wanted speed is 83.333333333%, but there is no way of specifying that.

Unfortunately, the many issues with the speed effect strikes again.

Reproducible: Always

Steps to Reproduce:
1. Put several 30fps clips in a 25fps project
2. Apply fade to black on them
3. Apply 83% speed effect
4. Shrink clips to original timespan
5. Play back the video

Actual Results:
It now appears as if there are "extra" fades on the video clips, where the clip would fade to black in the middle.
I automatically split out my audio. One can't resize the video part of a clip unless ungrouped from the audio.
Un-grouping keyboard shortcut does not always work.

Expected Results:
Keyboard commands to be applied consistently
Speed effect to be applied to audio and video (seperate bug files for this one)
Fades to be where they are visually represented in the clip

The implementation of the speed effect needs to be re-thought. In it's current form it completely breaks the user experience of kdenlive.
Kdenlive has been quite stable as of late, and the many open issues with the speed effect is negating all the hard work.

In my mind the speed effect should not even be an effect, it should be a clip job, the same as stabilize et. al.
There is no reason not to have it both as a clip job and as a effect.
The reverse clip clip job already does what we need, all that is needed is to be able to set the speedup/slowdown when the .mlt is created.
One last niggle... there is no way of cleanly specifying 30/25 slowdown in percentages, resulting in a dropped frame every 100 frames.. Annoying.

Comment 1 Evert Vorster 2016-09-12 09:40:04 UTC

Created attachment 101052 [details]
Project with the corruption

Here is a project file that shows the corruption

I only edited the first four clips.

Comment 2 Evert Vorster 2016-09-12 10:09:37 UTC

Created attachment 101053 [details]
Render of the first four clips

Render of the first four clips...

Towards the end, there is a fade to black in the middle of a clip that is not visible on the timeline, and only appeared after I started messing with the speed of the clips involved.

Comment 3 Jean-Baptiste Mardelle 2016-09-12 22:51:03 UTC

Created attachment 101062 [details]
Change clip speed job

Before investigating this bug, I agree that we can easily change the "reverse clip" job to make it also handle any speed, and creating .mlt files. I have a working patch, you can see a screenshot attached. The idea is to replace the "Reverse clip" entry in clip jobs with a "duplicate clip with speed change" entry, allowing any speed. The option menu on the right of the speed combobox allows to set the speed to reverse (-100%) as well as some standards (50%, 200%). Would that be ok for you ? I will also investigate the timeline problem later.

Comment 4 Evert Vorster 2016-09-13 04:00:29 UTC

Hi there!

Thanks for looking into this. 
Would I be able to set any speed within the limits of the mlt producer?

If this method works well, you might be able to simplify the code by depreciating and putting a warning on the speed effect. In time you should be able to remove it completely. 
The speed effect has been the source of un-ending bugs. I have more than a handful of open bugs that I would like to test against the clip job method. 

Kind regards,
Evert

Comment 5 Jean-Baptiste Mardelle 2016-09-13 06:05:22 UTC

I will push the change in master so that you can test it. In its current implementation, the slider ranges from 1% to 1000%, but you can manually enter values in the text box between -10'000% and 10'000%. Waiting for your feedback

Comment 6 Jean-Baptiste Mardelle 2016-09-13 06:06:00 UTC

Git commit cee1588f74b51c30b53f5a60217484751797c77f by Jean-Baptiste Mardelle.
Committed on 13/09/2016 at 06:05.
Pushed by mardelle into branch 'master'.

Update "Reverse Clip" Bin job to handle any speed

M  +1    -0    src/CMakeLists.txt
M  +1    -1    src/mainwindow.cpp
M  +1    -0    src/project/dialogs/CMakeLists.txt
A  +98   -0    src/project/dialogs/clipspeed.cpp     [License: GPL (v2+)]
A  +52   -0    src/project/dialogs/clipspeed.h     [License: GPL (v2+)]
M  +1    -1    src/project/dialogs/slideshowclip.cpp
M  +24   -3    src/project/jobs/filterjob.cpp
A  +164  -0    src/ui/clipspeed_ui.ui

http://commits.kde.org/kdenlive/cee1588f74b51c30b53f5a60217484751797c77f

Comment 7 Evert Vorster 2016-09-13 18:25:55 UTC

This works quite well, thank you very much!

I can set my desired speed beforehand, and no more corruptions on the timeline. 

As an added bonus audio is handled properly, and no weirdly sized audio clips. It even splits out automatically, and the pre-render has no problem with the video. Nice. 

Thank you!

I will test this against all the open bugs I have on the speed effect, and comment in each of those bugs on how well it works. 

Thank you for implementing this!

Comment 8 Wegwerf 2016-09-15 17:29:16 UTC

Hi Evert, glad to hear! So, is this particular bug for this report now sufficiently fixed? If yes, can you please be so kind as to close this bug report? Thank you very much for your cooperation!

Comment 9 Evert Vorster 2016-09-15 18:13:55 UTC

I left open the bug a little longer just in case I found a flaw in the implementation. 

I have now used this method on a rather large project, and it works flawlessly every time. 

A big thanks to JBM for implementing this!