Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DefaultTimeBar: time label changes twice for single seek action #5767

Closed
levz opened this issue Apr 15, 2019 · 3 comments
Closed

DefaultTimeBar: time label changes twice for single seek action #5767

levz opened this issue Apr 15, 2019 · 3 comments
Assignees
Labels

Comments

@levz
Copy link

levz commented Apr 15, 2019

[REQUIRED] Issue description

DefaultTimeBar seek does not increment/decrement seek position accurately.

[REQUIRED] Reproduction steps

  • ExoPlayer 2.9.3
  • Use DefaultTimeBar as seek time bar.
  • Set seek increment using setKeyTimeIncrement(long increment)
  • Set video on pause (not necessary, but it is easier to see the problem)
  • Seek DefaultTimeBar with left\right dpad keys.

Expected:

  • time label changes once for every key press
  • time on the label changes from INITIAL_TIME to INITIAL_TIME +- INCREMENT

Actual behavior:

  • time label changes twice for a single key press:
    1. first time it changes for key press
    2. second time it changes when buffering starts
  • time on the label both time is not as expected

As far as I understood the problem occurs because:

  1. getScrubberPosition() is called twice for a single keypress and it operates on a different values, second time producing different result
  2. incorrent calculations are probably related to scrubber having its own width.

[REQUIRED] Link to test content

Any contend can be used, but video should have large enough duration (I tested on 2 hours movie). When testing on short 30 seconds ad I did not see this issue.

[REQUIRED] A full bug report captured from the device

No crash is involved.

[REQUIRED] Version of ExoPlayer being used

Used 2.9.3

[REQUIRED] Device(s) and version(s) of Android being used

Android TV 8.0
Android 5.0 phone (called onKeyPressed() artifically from code)

@ojw28
Copy link
Contributor

ojw28 commented Apr 16, 2019

The problem is that we're re-using code designed for touch scrubbing for key press scrubbing.

The touch scrubbing code calculates playback positions from scrubber positions as the user drags the scrubber. This incurs a rounding error because the accuracy of the calculation is limited to however much time is represented by 1 pixel of seek bar. Note that the rounding errors will be larger for longer duration content. For touch based scrubbing this is fine and working as intended, since there's no way for a user to position the scrubber with sub-pixel granularity. However for key press scrubbing we should be able to seek accurately based on the specified increment. The solution is to separate out a different code path for this.

@ojw28
Copy link
Contributor

ojw28 commented Apr 16, 2019

After a fix, it should probably be true that getScrubberPosition is only be called from onTouchEvent.

ojw28 added a commit that referenced this issue Apr 16, 2019
Issue: #5767
PiperOrigin-RevId: 243811443
@ojw28
Copy link
Contributor

ojw28 commented Apr 16, 2019

Should be fixed in dev-v2. Please verify if possible.

@ojw28 ojw28 closed this as completed Apr 16, 2019
@ojw28 ojw28 self-assigned this May 14, 2019
@google google locked and limited conversation to collaborators Aug 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants