Added the ability to automatically scroll the timeline to points of i…

[kartchnb]

…nterest. Specifically, the user can center the timeline on the current playhead position using a toolbar button or a keyboard shortcut. Additionally, when the playhead is moved to a marker or the start or end of the timeline, the timeline is scrolled to match. This eliminates the need to search for the playhead in long projects.

Added the function "centerOnTime" to controllers.js to allow the timeline to be centered on an arbitrary time.
Added the function "centerOnTime" to timeline_webview.py to call the "centerOnTime" function in controllers.js.
Added the action "actionCenterOnPlayhead" to main-window.ui and added it to the toolbox between "actionPreviousMarker" and "actionNextMarker".
Added the function "actionCenterOnPlayhead" to main_window.py to center the timeline on the current playhead position.
Modified main_window.py to automatically scroll the timeline when the playhead is moved to a marker or the start or end of the timeline.

[ferdnyc]

@kartchnb

Wow, this sounds amazing! I will definitely give it a look over the weekend. At first glance it looks awesome, though, the style looks good and the code seems generally sound.

I'll leave a couple of comments now, about minor things I noticed on a read-through — one of them I see you already addressed, the unnecessary scale param to centerOnTime() — and I'll follow up with a more complete review once I've had time to check out the PR branch and give it a spin.

This will be really great to have in OpenShot, though! In fact, it strikes me that my PR #2938 already adds code to determiner whether or not the playhead is visible in the window, which could be abstracted out into a boolean function. (It's not currently abstracted out of the zooming code, but it could and probably should be anyway.) Combined with this, and pretty much all the building blocks are in place to even take a stab at general playhead follow-scrolling, too!

[ferdnyc]

@kartchnb

Your call on closing the PR or not, but you always have the option to hard reset the branch to develop, force-push, and then you can reapply just the commits you want onto this branch (keeping the same PR). But opening a new one works, too. Your call.

[ferdnyc]

(Before resetting the branch, it's a good idea to either git format-patch export the commits you want to keep, or create a new branch off this one w/ git checkout -b newbranch center-timeline, so the commits don't end up orphaned and pruned by git.)

[kartchnb]

Thank you for the review, comments, and advice, Frank. I appreciate it. I removed the pull request because, although I've been coding for years, surprisingly, I've never worked with GIT and I'm afraid I may have got some of my branches crossed with some other changes I'm playing with getting mixed in. I don't know enough about GIT yet to know how to fix this, so I plan on just starting from scratch and manually move my changes over. I'll get this eventually! I apologize that you reviewed code that is no longer there. I actually attempted to do playhead follow-scrolling, but found that the performance seemed to suffer (at least on my machine), so I abandoned it for now. If it can be done efficiently, I think it would be nice to at least provide as an option. Again, thank you for the comments. This is my first attempt at contributing to an open-source project and it's a bit daunting. I appreciate the confidence boost from you, though.

…

On Sat, Aug 31, 2019 at 10:06 AM Frank Dana ***@***.***> wrote: (Before resetting the branch, it's a good idea to either git format-patch export the commits you want to keep, or create a new branch off this one w/ git checkout -b newbranch center-timeline, so the commits don't end up orphaned and pruned by git.) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2959?email_source=notifications&email_token=ANBR2HHFUPI75O5HUELPND3QHKJHZA5CNFSM4ISSK6FKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5TPTRA#issuecomment-526842308>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ANBR2HHCFGKBCX26E4RH7VDQHKJHZANCNFSM4ISSK6FA> .

-- - Brad Kartchner

[ferdnyc]

I actually attempted to do playhead follow-scrolling, but found that the performance seemed to suffer (at least on my machine), so I abandoned it for now. If it can be done efficiently, I think it would be nice to at least provide as an option.

Oh, yeah, as things currently stand I don't think it's feasible. I'd have to look at the code again, but IIRC we're updating the playhead position way too often as it is. Then you add repositioning the scroll into every one of those updates, and the drag on the system just gets immense.¹

But that was the part that got me thinking: We don't really need to reposition on every update. We only need to scroll — whether it's for a Next/Prev Marker movement, or while playing — IFF the playhead actually moves out of view. As long as it stays IN view, there's really no need to update the scroll position at all.

Without having tried it, still, my suspicion on reading the PR was that re-centering on every move might feel "twitchy". (If you're at the start of the timeline and on the left half of the screen, it won't matter since the center can't move any farther left from where it is, but otherwise that sounds like an awful lot of scrolling.)

When a clip fits entirely within the timeline at the current zoom level, I actually like that I can skip around it from one end to the other without anything moving except the playhead.

Combine (what could be) my isPlayheadVisible() chocolate with your centerOnTime() peanut butter, and we can have the best of both worlds: Move only the playhead as long as it stays within the current view, and scroll only when necessary to avoid losing track of it.

And maybe, eventually, doing the same thing for playback.

Thank you for the review, comments, and advice, Frank. I appreciate it. I removed the pull request because, although I've been coding for years, surprisingly, I've never worked with GIT and I'm afraid I may have got some of my branches crossed with some other changes I'm playing with getting mixed in. I don't know enough about GIT yet to know how to fix this, so I plan on just starting from scratch and manually move my changes over. I'll get this eventually!

Oh, no worries at all. We've all done it at least once, it's basically a git rite of passage TBH. STILL happens occasionally (OpenShot/libopenshot#228) , no matter how long you've been working with it.

I find a mix of command-line and GUI tools works best. For managing branches and their ancestry, nothing beats plain old git shell commands. For managing the history & contents of those branches, a GUI often makes things clearer. If you're working on a Unix-based system, the gitk that comes with Git itself is good enough for starters. Lately I've been doing a lot of the more complex stuff with Sublime Merge (from the Sublime Text team, though I don't use Sublime Text as my editor), which has an absolutely dizzying array of branch-management functions, and one of the best merge conflict resolution tools I've seen so far.

I apologize that you reviewed code that is no longer there.

Oh, no, I would've even say I reviewed the code. I read the code, that's all. I'll read it again! It's not a worry. 😆

Again, thank you for the comments. This is my first attempt at contributing to an open-source project and it's a bit daunting. I appreciate the confidence boost from you, though.

It absolutely can be daunting, unfortunately. And we try to let every contributor know that we understand. It's no secret that git is complex and takes some time to get your brain wrapped around it, and GitHub Is Confusing™ in its own right. Partly because their fork-and-PR structure falls slightly short of really establishing a workable environment for developing code for submission to a different repository. (I have a Gist that details my own process for managing that, as well as the equivalent process using GitHub's hub command-line tool.)

The process can sometimes seem like more work (or at least more hassle) than the development. But every contribution is valued — this is an amazing first effort! Hell, I think my first OpenShot PR was a spelling formatting correction for the manual. 😉 (I'm not kidding.)

The problem of getting commit-bombed by long, rambling screeds from me... well, we haven't quite solved that one yet, either.

Notes

1. A digression:

   Traditional media players don't have nearly as much going on as OpenShot does, during playback, but they also have interfaces that can be decoupled from the player itself, and even displayed remotely. So, those interfaces have to be lightweight, and protocols for remote control/display (like MPRIS2) are designed with that in mind. The communication between the player and the frontend during playback is surprisingly minimal.

   When playback starts, the player sends a message to the frontend: "Play from $TIME index at $SPEED"... and that's it. They aren't expected to communicate again until one of them has a message for the other that changes that condition: Stop, Pause, Track Change, Play from $TIME (a seek), or Play at $SPEED (a rate change).

   The playback position (time) indicator displayed while playback in underway, ticking away once a second (if not once a frame) or whatever? After playback is started, the time index itself doesn't actually come from the player: It's computed locally by the frontend, and updated on a timer. The frontend knows the start time and it knows the playback speed, that's all it needs to compute and update the elapsed time as play continues. There's no reason the backend needs to provide further information unless something happens to delay the actual output. (Exceptional conditions, e.g. buffering, lag, read errors, etc.)

   A different project I'm involved with (the GSConect Gnome Shell extension) has a feature that mirrors media player controls to a paired Android device running the KDE Connect app. So, for example, when VLC is playing a video, I get a playback notification on my phone with standard transport controls, and I can effectively use my phone as a remote control. It's pretty neat.

   Anyway, we had a problem with a particular media player that was emitting MPRIS2 position updates just four times per second — it doesn't sound like a lot, but because each of those had to be transformed into a network packet and sent to KDE Connect, the drain on system resources got immense. The only solution was to simply turn off updates from that software. The KDE Connect architecture just isn't designed to handle 250ms-interval updates, it's just too much.

   When OpenShot is playing, forget four times a second. IIRC it's updating the playhead position every frame — 24, 30, even 60 times a second. You do the math.