-
Notifications
You must be signed in to change notification settings - Fork 6k
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
Tune time bar update based on duration #5496
Changes from all commits
c905891
62e6455
ecfce46
eaf0d40
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,6 +24,7 @@ | |
import android.os.SystemClock; | ||
import android.support.annotation.Nullable; | ||
import android.util.AttributeSet; | ||
import android.util.Log; | ||
import android.view.KeyEvent; | ||
import android.view.LayoutInflater; | ||
import android.view.MotionEvent; | ||
|
@@ -196,6 +197,9 @@ public interface VisibilityListener { | |
public static final int MAX_WINDOWS_FOR_MULTI_WINDOW_TIME_BAR = 100; | ||
|
||
private static final long MAX_POSITION_FOR_SEEK_TO_PREVIOUS = 3000; | ||
private static final int DEFAULT_UPDATE_DP = 5; | ||
private static final long MIN_UPDATE_FREQUENCY_MS = 50; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems we already use a minimum delay of 200ms for the cases where the playback speed is > 5.0f. Can you change that accordingly, and also use the same constants in updateProgress()? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure if I understood correctly. Should I change MIN_UPDATE_FREQUENCY_MS to 200ms? The other half is done. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thinking about it again, 50ms might actually be a better minimum value. :) |
||
private static final long MAX_UPDATE_FREQUENCY_MS = 1000; | ||
|
||
private final ComponentListener componentListener; | ||
private final View previousButton; | ||
|
@@ -832,21 +836,10 @@ private void updateProgress() { | |
long delayMs; | ||
if (player.getPlayWhenReady() && playbackState == Player.STATE_READY) { | ||
float playbackSpeed = player.getPlaybackParameters().speed; | ||
if (playbackSpeed <= 0.1f) { | ||
delayMs = 1000; | ||
} else if (playbackSpeed <= 5f) { | ||
long mediaTimeUpdatePeriodMs = 1000 / Math.max(1, Math.round(1 / playbackSpeed)); | ||
long mediaTimeDelayMs = mediaTimeUpdatePeriodMs - (position % mediaTimeUpdatePeriodMs); | ||
if (mediaTimeDelayMs < (mediaTimeUpdatePeriodMs / 5)) { | ||
mediaTimeDelayMs += mediaTimeUpdatePeriodMs; | ||
} | ||
delayMs = | ||
playbackSpeed == 1 ? mediaTimeDelayMs : (long) (mediaTimeDelayMs / playbackSpeed); | ||
} else { | ||
delayMs = 200; | ||
} | ||
int timeBarWidth = timeBar.getTimeBarWidthDp(); | ||
delayMs = getTimeBarUpdateDelayMs(timeBarWidth, position, duration, playbackSpeed); | ||
} else { | ||
delayMs = 1000; | ||
delayMs = MAX_UPDATE_FREQUENCY_MS; | ||
} | ||
postDelayed(updateProgressAction, delayMs); | ||
} | ||
|
@@ -1075,6 +1068,58 @@ private static boolean canShowMultiWindowTimeBar(Timeline timeline, Timeline.Win | |
return true; | ||
} | ||
|
||
/** | ||
* Calculates the update delay of time bar in millis | ||
* | ||
* @param timeBarWidth The width of time bar in dp | ||
* @param position The current position in millis | ||
* @param duration The duration of media in millis | ||
* @param playbackSpeed The playback speed | ||
* @return Update delay of time bar in millis | ||
*/ | ||
private static long getTimeBarUpdateDelayMs(int timeBarWidth, long position, | ||
long duration, float playbackSpeed) { | ||
if (timeBarWidth == 0 || duration == 0 || playbackSpeed == 0) { | ||
return MAX_UPDATE_FREQUENCY_MS; | ||
} | ||
|
||
// Calculate how many updates needs to be done with DEFAULT_UPDATE_DP | ||
// to fill up the time bar | ||
int numberOfUpdates = timeBarWidth / DEFAULT_UPDATE_DP; | ||
|
||
if (numberOfUpdates <= 0) { | ||
return MAX_UPDATE_FREQUENCY_MS; | ||
} | ||
|
||
// Calculate the designated update interval, taking duration into consideration as well | ||
long mediaTimeUpdatePeriodMs = duration / numberOfUpdates; | ||
|
||
if (mediaTimeUpdatePeriodMs <= 0) { | ||
return MAX_UPDATE_FREQUENCY_MS; | ||
} | ||
|
||
// Calculate the delay needed from the current position until the next update is due | ||
long mediaTimeDelayMs = mediaTimeUpdatePeriodMs - (position % mediaTimeUpdatePeriodMs); | ||
|
||
// If the delay would be too small, then skip the next update | ||
if (mediaTimeDelayMs < (mediaTimeUpdatePeriodMs / 5)) { | ||
mediaTimeDelayMs += mediaTimeUpdatePeriodMs; | ||
} | ||
|
||
// Calculate the delay until the next update (in real time), taking | ||
// playbackSpeed into consideration | ||
long delayMs = playbackSpeed == 1 ? mediaTimeDelayMs : (long) (mediaTimeDelayMs / playbackSpeed); | ||
|
||
// Limit the delay if needed, to avoid too frequent / infrequent updates | ||
if (delayMs < MIN_UPDATE_FREQUENCY_MS) { | ||
delayMs = MIN_UPDATE_FREQUENCY_MS; | ||
} else if (delayMs >= MAX_UPDATE_FREQUENCY_MS) { | ||
delayMs = MAX_UPDATE_FREQUENCY_MS; | ||
} | ||
|
||
return delayMs; | ||
} | ||
|
||
private final class ComponentListener | ||
implements Player.EventListener, TimeBar.OnScrubListener, OnClickListener { | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious: How did you choose this value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By checking a couple of values, how it looks visually. 20dp was too high and the progress jumped way too much. In my opinion of course.