pixop / video-compare

Split screen video comparison tool using FFmpeg and SDL2
GNU General Public License v2.0
969 stars 44 forks source link

Allow frame by frame navigation by buffering on-demand #46

Open jojje opened 1 year ago

jojje commented 1 year ago

Problem definition

It seems that in order to perform frame-by-frame navigation with the A and D keys, frames have to be buffered already. When navigating to some point in the clip(s) with the mouse or any of the key-shortcuts, the buffer is empty, so frame-by-frame navigation isn't possible. In order to get the frame navigation to work, the user has to use the following cumbersome process:

  1. Find interesting spots in the clips to be analyzed by jumping around with the available seek-options.
  2. Once a spot has been found, jump back one keyframe (e.g. - key).
  3. Press play until the buffer fills up to 50 frames, and be quick on the trigger to pause the video.
  4. Navigate back with the A key until the specific frame of interest is found.

Proposed improvement

Get rid of steps 2-4 by either:

With option A, the user could navigate the entire clip frame-by-frame seamlessly. With option B, they'd have to buffer manually, but would still allow pretty much uninterrupted workflow with just one extra key to press every now and then.

The reason I even mentioned option B was that after looking at the code, it might be easier to implement. Whatever improvement can be done in terms of frame-by-frame navigation would be greatly appreciated.

PS. fantastic tool. Just discovered it and am kicking myself for not finding it sooner.

jonfryd commented 1 year ago

Hi @jojje,

Thanks for sharing your ideas in such detail.

I'm in the process of finalizing the next version (which should be out this week), and once it's done, I'll take a break from working on the tool for at least three to four months. However, I'll keep your input in the back of my mind and see if there is something we can do in the future perhaps.

If someone wants to work on this, I'm willing to accept PRs, of course.

Cheers!

jojje commented 1 year ago

Looking forward to the new version. Hopefully I can implement this one myself on that code base.

Was staring for 30 minutes at CompareVideo::video function, but failed to find an obvious way to enhance the buffering due to so many different concerns being complected in that one function. Will see how the design has changed in the next version, and hopefully it'll simplify this task.

jonfryd commented 1 year ago

Looking forward to the new version. Hopefully I can implement this one myself on that code base.

Was staring for 30 minutes at CompareVideo::video function, but failed to find an obvious way to enhance the buffering due to so many different concerns being complected in that one function. Will see how the design has changed in the next version, and hopefully it'll simplify this task.

Haha, please don't expect a complete rewrite (even though that would be nice and long overdue). I'm mostly focusing on bug fixes and minor, incremental improvements for the sake of reliability.

Unfortunately, even after my unpushed changes, the entire video() method remains overly complex to comprehend and maintain. I appreciate that you had the courage to look into it, though.

teijiIshida commented 1 year ago

Frame by frame comparison is super useful and not many programs do this. I look forward to this feature whenever it will be implemented.

Frizlab commented 8 months ago

I’m looking forward to it too! It would be a game changer.

jonfryd commented 8 months ago

I've pushed a commit that enables fetching new frames when D is pressed. It's nothing fancy, but it's surely an improvement to the user experience. However, it doesn't resolve the process outlined by @jojje above, as addressing backward navigation—which requires frame-accurate seeks and actual pre-buffering—will be harder to do. Maybe at some later point...