Timeline Bug Fixes

[DylanC]

I hope to fix the follow issues:
#1383
#1454
#1587 (maybe if possible) - Will fix in a later PR.

Current the PR:
Fixes #1383
Fixes #1454

[DylanC]

@ferdnyc - Ready for your review.

[ferdnyc]

Awesome, it's a lot better now! I'm afraid it's still glitching under certain, limited circumstances, though. It doesn't do it every time, but only when you attempt to resize a Clip that's already at max length — you'll lose one frame of length.

Easiest way to reproduce:

1. Set up a Timeline with one video or audio clip
2. Zoom all the way in to 1s zoom, looking at the end of the Clip
3. Place the Playhead somewhere not far past where the Clip ends
   (I recommend using the ← / → arrow keys to make sure it's on a frame boundary, since I just noticed that you can drag the Playhead to any location you want, even between two frames, and it doesn't seem to mind, or to snap to a frame boundary.)
4. Drag the Clip so that its right edge is snapped up against the Playhead line
5. Grab the right edge of the Clip, attempt to drag it right, and then release it.

The Clip will shrink away from the Playhead line by one frame.

• If you try to grow it again, it will neither grow nor shrink, it'll stay 1 frame too short.

• If you resize the (now visually one frame short) Clip shorter, release, and then drag it back to full length, it will let you grow it to the full correct length.¹

It's only when attempting to grow a Clip that's already at full length, that it still drops one frame-length. Which, as I said, is still a HUGE improvement, and so close to a complete fix.

Notes

1. However: When doing that, like before the fix, while dragging it will limit itself to one length too short, and then grow to the correct length only after releasing the drag. When attempting to resize from max size, still that same thing happens (it "pulls back" to one frame less than the full length, the moment you move the mouse to the right during the resize drag)... and that "correction" growth that would get it back to the proper length still fails to occur, again only when starting from max length.

   So, I think you've definitely fixed the larger half of it, and the only thing still not quite working is the bit I mentioned before: The code that decides where to limit the max length during the drag, which is still coming up a frame short of what it should be. But now that limit is dependent only on the ACTUAL max duration of the Clip, not on the visual length of the clip rectangle, so it no longer compounds with every resize attempt.

[DylanC]

@ferdnyc - I tried to reproduce this behavior from your above description but it didn't work for me. Maybe this will need to be revisited at a later date. (unless I can reproduce soon) I feel #1587 more relates to your menu changes so I will leave that one alone.

Looking into #1839 at the moment. Would be a nice one to solve if possible.

[ferdnyc]

I tried to reproduce this behavior from your above description but it didn't work for me.

Hrm, that's frustrating. Well, definitely don't worry about it for now. I'll see if I can put together a screencast or something to better demonstrate.

I feel #1587 more relates to your menu changes so I will leave that one alone.

Not really at all... or, it does, but I wouldn't be solving any of that down the same paths, as I don't know how to build controls in the Angular world. (I could maybe figure it out, but it'd be a huge help and time-saver if you were to whip up an example/"template" or two.) That way I could keep plugging away inside the Python side of things. (And on the libopenshot configuration/build system, as I'm hoping to coordinate with @N3WWN on some enhancements to accompany the ReSVG PR.)

Ultimately some changes in the Python and Angular Timeline worlds will have to be coordinated with each other, but one "upside" (hah) of the loosely-coupled Angular/Python bridge is that, for the most part, whatever we do on either side won't impact the other unless/until we deliberately choose to.

What I'd propose, for #1587 — not at all what that issue is actually asking for, but it is in the same vein, or at least a stepping-stone on the way to addressing the issue itself — would be tackling what I mentioned earlier about the track lock. Would you be interested in replacing the current Lock indicator (a padlock icon that appears only when the track is locked) with an interactive control that's always visible, and can be clicked to toggle the track's Locked state? (Maybe it starts as a grayscale, 60%-opaque padlock image, and becomes solid blue when Lock is activated?) The Track menu option wouldn't even be affected; it'd still be there as an alternate way to toggle Lock status (because, why not?) — I have no plans of modifying the Track menu at all, since... well... it's five items and just fine the way it is, basically.

Not that track-lock is a super high priority issue or a heavily-used part of OpenShot's functionality. But it's the one thing we currently have a visible icon for on the Timeline, so it's a prime and obvious candidate for the first conversion from menu-only feature to active interface element. Turning it into an interactive control would (a) set a precedent for migration of any other features that might make sense to have exposed in the GUI in a similar same fashion, and (b) give the rest of us a template/blueprint for building that type of feature within the Angular HTML/JS, so that we could cheat off your test instead of actually learning how it's done. 😉

Totally up to you, of course. It's just a idea I've had for a while, tho — seems silly to have to go into a menu and choose "Enable lock" to make an icon appear. Isn't the whole point of a GUI that you can click on icons to make things happen, directly?

[DylanC]

@ferdnyc

Hrm, that's frustrating. Well, definitely don't worry about it for now. I'll see if I can put together a screencast or something to better demonstrate.

That would be good. I did try your instructions and I felt I really wasn't seeing such behavior but I'm open minded to the fact I may have been doing something wrong.

Not that track-lock is a super high priority issue or a heavily-used part of OpenShot's functionality. But it's the one thing we currently have a visible icon for on the Timeline, so it's a prime and obvious candidate for the first conversion from menu-only feature to active interface element. Turning it into an interactive control would (a) set a precedent for migration of any other features that might make sense to have exposed in the GUI in a similar same fashion, and (b) give the rest of us a template/blueprint for building that type of feature within the Angular HTML/JS, so that we could cheat off your test instead of actually learning how it's done. 😉

Totally up to you, of course. It's just a idea I've had for a while, tho — seems silly to have to go into a menu and choose "Enable lock" to make an icon appear. Isn't the whole point of a GUI that you can click on icons to make things happen, directly?

I see. Sounds like something good to do indeed. Lock track would be a good start. I do know that one feature people miss from OpenShot which exists in v1.x is the ability to Enable/Disable Video on a track with one click and the same feature for audio is sorely missed. It probably won't be super easy to implement features direct on the track but yeah if I could it once we then have a blueprint for future track features.

[ferdnyc]

I do know that one feature people miss from OpenShot which exists in v1.x is the ability to Enable/Disable Video on a track with one click and the same feature for audio is sorely missed.

Those are definitely the two I was thinking of next: Track enable/disable, and Track mute/unmute.

[ferdnyc]

It probably won't be super easy to implement features direct on the track but yeah if I could it once we then have a blueprint for future track features.

Currently the simplest blueprint we have for how to do that is probably qt_log(), which is called at numerous points in the JavaScript code. That's defined at src/windows/views/timeline_webview.py:2596 as Timeline class method def qt_log(self, message=none), which is decorated as a pyqtSlot that takes one string argument.

So, calling "back" into the Python from the Angular is apparently doable — duh, really (on my part), or there wouldn't be any way to perform actions on the Timeline, period — and I can certainly supply qt_lock_track() and qt_unlock_track() methods that trigger the necessary Python-side updates in the same manner that actionLockTrack_trigger() and actionUnlockTrack_trigger() do (in main_window.py), when they're triggered by the corresponding menu selections.

[ferdnyc]

Hrm, that's frustrating. Well, definitely don't worry about it for now. I'll see if I can put together a screencast or something to better demonstrate.

Here you go. The image is a little "meh" because the screencasting software I used encodes straight to GIF. You also have to imagine the mouse clicks.

Basically, we're looking at the back end of a video clip at max length, which I move to align with the playhead line and then resize larger and smaller (or attempt to — every time you see the mouse pointer pull right from the Playhead line, I'm dragging the end of the clip). And ignore my fumbling trying to find the end handle, on the second attempt.

The GIF loops forever, so when you see me moving the clip around you've gone back to the start. I only do that once. Then I demonstrate the different resizes a couple of times.

[DylanC]

@ferdnyc - I was recording a .gif to show you it wasn't happening for me but I think it did actually happen or at least I noticed it for the first time. 🤣

Hmm.. now to try figure it out. I might move onto one of the other bugs first, as figuring this out will likely just slow me down in a big way.

[ferdnyc]

I mean, even the way it is now is still infinitely better than this nonsense, from 2.4.2 release:

[DylanC]

@ferdnyc - I might remove #1839 from the TODO on this PR. Doesn't seem all that much related to the timeline(web) side of things. Maybe better to spend time on #1587 tbh but even with that I would nearly prefer to open a new PR, since we have 2 valuable fixes here that could go in. (imho)