Add frame step buttons to internal video player #1207
Conversation
ecawthorne
changed the title
src/main/java/org/quantumbadger/redreader/views/video/ExoPlayerWrapperView.java
Outdated
Show resolved
Hide resolved
| R.string.video_step_back, | ||
| view -> { | ||
| long frameRate = mVideoPlayer.getVideoFormat() != null ? 30 : (long) mVideoPlayer.getVideoFormat().frameRate; | ||
| mVideoPlayer.seekTo(mVideoPlayer.getCurrentPosition() - frameRate); |
There was a problem hiding this comment.
I don't think this is the correct way to do it? Frame rate is a number of frames in a second so you need to calculate 1000 / frameRate to get duration of a frame in milliseconds
There was a problem hiding this comment.
You're absolutely right, I'm not sure what I was thinking there. I also unintentionally inverted the null check which hid a different issue: apparently getVideoFormat().framerate is always null in this context. After looking into it a bit, it seems like it's due to the format being H.264/AVC which ExoPlayer doesn't supply a frame rate for.
The condition is fixed in 8e0efb3. If you have any suggestions on how to proceed for calculating the frame rate just let me know. I'm working on the assumption that there may be a format change at some point in the future so I kept the frame rate check there for now.
src/main/java/org/quantumbadger/redreader/views/video/ExoPlayerWrapperView.java
Outdated
Show resolved
Hide resolved
Created icon_restart in order to reuse icon_previous asset for the frame step back button
QuantumBadger
force-pushed
the
add-frame-step
branch
from
8e0efb3 to
5b7775b
Compare
|
Thanks for this, and sorry for the delay with reviewing! I've rebased this and made a few minor adjustments. With the playback speed control, there are now too many icons to fit on the screen horizontally, so I'll have a think about how to improve the layout. I'll merge this in the meantime -- thanks again! |
Hi! This PR is based on #716. It adds an option for stepping through videos approximately frame by frame.
The setting is disabled by default and the new buttons are grayed out and disabled when playing as per this comment #716 (comment) by @QuantumBadger
I apologize if I'm stepping on any toes, there's been no activity on #716 for over a year so I figured I'd pick it up. Any feedback or suggestions are very welcome if there's more that needs to be done.