biigle / core

:large_blue_circle: Application core of BIIGLE
https://biigle.de
GNU General Public License v3.0
11 stars 16 forks source link

Implements 'Jump frame by frame' in video #823

Closed ToukL closed 1 week ago

ToukL commented 1 month ago

Splitted from here #821

Workaround to navigate frame by frame (forward and backward). Only works in Chrome for now, needs more tests.

mzur commented 1 month ago

Reference to my earlier response to this: https://github.com/biigle/core/pull/821#issuecomment-2090471105

ToukL commented 1 month ago

I think this still includes the "jump by seconds" code.

Ah yes I based this branch on "jump-by-seconds" but I can rebase on master, it would be cleaner indeed

ToukL commented 1 month ago
  • Please check for the availability of requestVideoFrameCallback when the video annotation tool loads. If it is not available, the feature is disabled and the settings button (see below) is disabled, too, so it cannot be clicked.

When you say "when the video annotation tool loads" is it in the loadVideo() method of the videoContainer or elsewhere (in the views part maybe) ?

mzur commented 1 month ago

The appropriate location would be in the created() method of the videoContainer. You can set a new attribute (e.g. this.supportsJumpByFrame) there and pass this on to the videoScreen and settings tab as props.

mzur commented 1 month ago

Please re-request my review once you are finished with all your changes so I know that I should have a look :wink:

ToukL commented 1 month ago

Please re-request my review once you are finished with all your changes so I know that I should have a look 😉

I just did (sorry I was a bit busy these days so it took me a while...)

I am not absolutely sure for the last point (the call to seek method to prevent double clicks). I only added events in VideoScreen that call the seek() method if this.seeking=false, according to my tests it seems to catch what we want without having to add an extra method in VideoContainer but maybe I'm missing something, tell me what you think :)

ToukL commented 3 weeks ago

Hello Martin, I re-requested the review but there is still an issue with the seek event that may freeze if the user click too many times (I don't know if you saw my comment above)