After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 593738 - [patch] Ripple trimming (edit) on left side of clip should move stuff on right side
[patch] Ripple trimming (edit) on left side of clip should move stuff on righ...
Status: RESOLVED FIXED
Product: pitivi
Classification: Other
Component: Timeline
Git
Other Linux
: Normal normal
: 0.91
Assigned To: Pitivi maintainers
Pitivi maintainers
Depends on:
Blocks:
 
 
Reported: 2009-09-01 01:23 UTC by Odin Hørthe Omdal
Modified: 2012-10-22 17:21 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
After trimming, you get a gap at the beginning (522.76 KB, video/ogg)
2009-09-01 01:23 UTC, Odin Hørthe Omdal
  Details
Mockup showing how the behaviour should be (200.02 KB, video/ogg)
2009-09-01 01:27 UTC, Odin Hørthe Omdal
  Details
Fix left ripple edit (1.75 KB, patch)
2010-04-24 20:04 UTC, Odin Hørthe Omdal
none Details | Review
Ripple edit bug, the bug surfaces at the end of the clip (795.25 KB, video/ogg)
2010-09-18 15:23 UTC, Odin Hørthe Omdal
  Details
Ripple edit patch, version 4 (4.06 KB, patch)
2010-09-18 15:24 UTC, Odin Hørthe Omdal
rejected Details | Review

Description Odin Hørthe Omdal 2009-09-01 01:23:23 UTC
Created attachment 142184 [details]
After trimming, you get a gap at the beginning

Ripple trimming should never move the left side of the clip, that's considered
finished.

PiTiVi does that right now and it makes for some strange artifacts:

* When you make the clip shorter, say 12 second, your whole movie suddenly gets
a 12 second gap at the beginning.
* When you make a clip longer, your movie gets trimmed by that amount in the
beginning, potentially shaving off your beginning. Actually, this does not
happen in PiTiVi, because it won't allow you to stretch your clip if it needed
to shave another clip.

These two errors will begone with moving the right side instead of the left.
Like PiTiVi already does for ripple trimming on the right side of clips.
Comment 1 Odin Hørthe Omdal 2009-09-01 01:27:24 UTC
Created attachment 142185 [details]
Mockup showing how the behaviour should be

The guides is there for the help of the user.

If you move your mouse right, you get less of a clip. If you move you mouse left you get more of a clip. The center (where the trim started) is always still.

Also note that the viewer window updates with what the trim will be, e.g. is showing what is at the center at all times (because that's what I'm interested in to cut to).
Comment 2 Odin Hørthe Omdal 2009-09-18 11:28:51 UTC
Any status/discussion? :-)
Comment 3 Jean-François Fortin Tam 2010-04-09 00:09:09 UTC
I'm trying to understand what you're describing... I played with ripple trimming while looking at this bug report and couldn't figure out anything that was wrong with it in pitivi 0.13.4/pitivi git?
Comment 4 Odin Hørthe Omdal 2010-04-11 13:10:26 UTC
It's still broken.

Watch the videos. What's the diff from my first attachement, and my second?

The first one shrink clip 2 like this:

1: [...clip 1...][...clip 2...][...clip 3...]
2: [...clip 1...][=>.clip 2...][...clip 3...]
3:       [...clip 1...][clip 2][...clip 3...]

while the second one shrink the clip 2 like this:

1: [...clip 1...][...clip 2...][...clip 3...]
2: [...clip 1...][=>.clip 2...][...clip 3...]
3: [...clip 1...][clip 2][...clip 3...]

The second one is the correct way to handle it. The first method is what PiTiVi currently does.
Comment 5 Jean-François Fortin Tam 2010-04-20 01:52:47 UTC
Huh, I would have expected method #1 to be the correct behavior. Is #2 the way it is done with all the other editors?
Comment 6 Odin Hørthe Omdal 2010-04-23 15:39:21 UTC
Yes, #2 is the way it's done in *all* other editors.

It's counter intuitive, that when you shorten a clip, that the length of the movie will be the same. It should be smaller then.

A more frightening example with PiTiVi's currently strange ripple trim; EXPANDING clip2. Here's how PiTiVi does it:

1: [...clip 1...][...clip 2...][...clip 3...]
2: [...clip 1...][<=.clip 2...][...clip 3...]
3: [clip 1][......clip 2......][...clip 3...]

Here's how it should be (the length get's longer when expanding clip2):

1: [...clip 1...][...clip 2...][...clip 3...]
2: [...clip 1...][<=.clip 2...][...clip 3...]
3: [...clip 1...][......clip 2......][...clip 3...]


As you can see, in #1 PiTiVi actually cut's the startingpoint of the edit/the first clip to make room for clip2. *Very unintuitive*. If I make edits inside my film, I don't suddently want the starting of the film to suffer from it.
Comment 7 Odin Hørthe Omdal 2010-04-23 15:44:07 UTC
Actually, PiTiVi doesn't do what I said in the last comment anymore. So that's wrong.

But anyway, what I said in comment 4 is still the same way, and as I said, *all other* editing software that I've used (Avid, Final Cut Pro, Premiere, Liquid (former Vegas)) does it that way.

I think even Cinelerra gets it correct.
Comment 8 Odin Hørthe Omdal 2010-04-23 15:47:22 UTC
(In reply to comment #6)
> 1: [...clip 1...][...clip 2...][...clip 3...]
> 2: [...clip 1...][<=.clip 2...][...clip 3...]
> 3: [clip 1][......clip 2......][...clip 3...]
> 
> Here's how it should be (the length get's longer when expanding clip2):
> 
> 1: [...clip 1...][...clip 2...][...clip 3...]
> 2: [...clip 1...][<=.clip 2...][...clip 3...]
> 3: [...clip 1...][......clip 2......][...clip 3...]

Oh my, -- there is a bug there anyways, yes. Because PiTiVi actually does do #1 now, but it does this:

[1234567][OLD]
[1234][newOLD]

Instead of this (as I presumed):

[1234567][OLD]
[4567][newOLD]

Mark however, that both are broken behaviour, the correct way to ripple trim this is:

[1234567][OLD]
[1234567][newOLD]
Comment 9 Jean-François Fortin Tam 2010-04-23 23:45:04 UTC
My brain hurts from seeing all those scenarios, but retriaging the bug anyway since you do seem to know exactly how it should behave :) I am not the one who can implement this though, so I don't know when this would be fixed and if it is hard to fix or not (patches are welcome of course!).
Comment 10 Odin Hørthe Omdal 2010-04-24 20:04:55 UTC
Created attachment 159484 [details] [review]
Fix left ripple edit

That patch took me 2 hours and 44 minutes (heh) although it is quite small.

I guess there is a better way to do this, but it works quite well on my computer, and I don't know how to better do this.

One particulary ugly bit is this one, to get the original duration: 

> original_duration = self.default_originals.values()[0][1]
Comment 11 Odin Hørthe Omdal 2010-07-22 22:23:17 UTC
Waiting for answer. :-)
Comment 12 DELETED 2010-07-26 14:11:45 UTC
I think the MAIN bug around pitivi is that there is NO active maintainer at all, who especially cares about the GUI part since a long time. This bug is a good example. You can see many bugs like this that are open here and nobody cares about. Jean-François is the only one who cleans up bug reports and tries to keep things going, unfortunately he's not a developer. Many people report bugs in pitivi and even submit patches, but as they start to get the feeling that there is no one in charge for the GUI part, they give up. It's a real bummer.

I'm not insulting anyone here. I really appreciate all the efforts that the developers have put into pitivi and I think it is a really great application! But it will be even better once all the annoying timeline bugs will be fixed fixed.

Couldn't the former main developer of the GUI call for help to get more people in charge?
Comment 13 Jean-François Fortin Tam 2010-07-26 15:39:59 UTC
Well, there are a couple of organizational problems, whose symptoms are accentuated by the fact that the main GUI dev (emdash) is currently on vacation and I, as a bug triager and tester, don't have enough time at the moment (summer job).

Calling for help: well, how exactly? I'm asking this in good faith as an informal project motivator and website maintainer. I tried everything I could so far to make it a more attractive to new contributors (ie: a clear website, a cleaner wiki, a list of gnome-love/pitivi-love bugs...), but if there is something that has not been tried yet, I am all ears. From what I can tell, people haven't been exactly flooding the gates asking to do the hard job. My hopes is that with more features, the project popularity will gradually rise and attain sustainability. 

Getting the critical mass is the hard thing and I'd love to know what can be improved to achieve that goal (though this discussion might be better on the mailing list than here I guess).
Comment 14 Alessandro Decina 2010-07-29 20:16:11 UTC
First of all, thanks a lot for this patch and apologies for taking so long to review it.

I just tried it and it does indeed feel more intuitive to me. I haven't looked closely at the code yet since it breaks the current tests in tests/test_timeline.py. Those tests assume the old behaviour, could you adapt them and make sure that the new behaviour is properly tested?
Comment 15 DELETED 2010-07-29 21:13:11 UTC
@Odin:
When I try your patch, the the following action you described as

1: [...clip 1...][...clip 2...][...clip 3...]
2: [...clip 1...][<=.clip 2...][...clip 3...]
3: [...clip 1...][......clip 2......][...clip 3...]

does not do what I understood you want it to do. With your patch, ripple trimming the start of clip 2 just changes the duration of clip 2 by shortening or lengthen it at its END (as I would expect it to behave when I ripple trim at the end). It does NOT change the starting point in clip 2. By that I mean that no matter how I ripple trim the left end of clip 2, when the playhead enters clip 2 it will still show the same frame. But as I understood it, after ripple trimming/extending the start of clip 2 (pulling the handle rightwards), when entering clip 2 there should be a later/earlier scene showing up than before.

Did I get something wrong?
Comment 16 DELETED 2010-07-29 21:28:09 UTC
Trying to find a short form to express what we want:

Ripple trim at the LEFT handle of clip A = add/substract time at the
BEGINNING of A and move all clips right of A accordingly.

Ripple trim at the RIGHT handle of clip A = add/substract time at the
END of A and move all clips right of A accordingly.

Right?
Comment 17 Odin Hørthe Omdal 2010-09-18 15:22:49 UTC
Started fixing tests. Then I found some bugs in my patch. I didn't handle changing of mode.

In PiTiVi, you can TRIM, and then press *shift* while your trimming, and you change mode to RIPPLE.

So I fixed that, also fixing another bug that the old code didn't hit (resetting of the trim mode).

But there is one strange problem I can't nail/find:

Going from TRIM->RIPPLE->TRIM->RIPPLE->TRIM a few times, sometimes the clip get a very strange out-point. I have a video showing the problem.

Also, I'll attach my patch so far (there are some ugly print statements in there, of course I will clean up when doing a real patch ;-) ).

It works quite nice if you don't change mode. I've been able to use it for some small stuff.
Comment 18 Odin Hørthe Omdal 2010-09-18 15:23:53 UTC
Created attachment 170543 [details]
Ripple edit bug, the bug surfaces at the end of the clip
Comment 19 Odin Hørthe Omdal 2010-09-18 15:24:55 UTC
Created attachment 170545 [details] [review]
Ripple edit patch, version 4
Comment 20 Odin Hørthe Omdal 2010-09-18 15:27:21 UTC
Also, I forgot to mention, thank you Volker at comment 15. You were right.

I used much time fixing it to the real, specified behaviour now. And this new patch has that behaviour.

I didn't try my old patch on real projects, but this new patch has stood a human test. :-)
Comment 21 Brandon Lewis 2010-11-26 17:06:13 UTC
Review of attachment 170545 [details] [review]:

Couple of minor comments:
1) remove the commented-out code
2) remove the print statements. use the logging interface for debugging info
Comment 22 Odin Hørthe Omdal 2011-05-25 18:49:29 UTC
I never finished this patch. I started doing the tests etc, but for some strange-strange reason it's jumping around in some specific places.

But I'd like to have this in and I guess that won't happen until I write a mergeable patch.
Comment 23 Jean-François Fortin Tam 2012-10-22 17:21:14 UTC
As far as I know, this is supposed to work with GES these days.