Skip to content

feat(Player): automatically select best buffer duration #7358

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

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ object PreferenceKeys {
const val SEEK_INCREMENT = "seek_increment"
const val DEFAULT_RESOLUTION = "default_res"
const val DEFAULT_RESOLUTION_MOBILE = "default_res_mobile"
const val BUFFERING_GOAL = "buffering_goal"
const val PLAYER_AUDIO_FORMAT = "player_audio_format"
const val PLAYER_AUDIO_QUALITY = "player_audio_quality"
const val PLAYER_AUDIO_QUALITY_MOBILE = "player_audio_quality_mobile"
Expand Down
49 changes: 30 additions & 19 deletions app/src/main/java/com/github/libretube/helpers/PlayerHelper.kt
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.github.libretube.helpers

import android.app.Activity
import android.app.ActivityManager
import android.content.Context
import android.content.Intent
import android.content.pm.ActivityInfo
Expand Down Expand Up @@ -187,12 +188,6 @@ object PlayerHelper {
false
)

private val bufferingGoal: Int
get() = PreferenceHelper.getString(
PreferenceKeys.BUFFERING_GOAL,
"50"
).toInt() * 1000

val sponsorBlockEnabled: Boolean
get() = PreferenceHelper.getBoolean(
"sb_enabled_key",
Expand Down Expand Up @@ -512,7 +507,7 @@ object PlayerHelper {
* Create a basic player, that is used for all types of playback situations inside the app
*/
@OptIn(androidx.media3.common.util.UnstableApi::class)
fun createPlayer(context: Context, trackSelector: DefaultTrackSelector, ): ExoPlayer {
fun createPlayer(context: Context, trackSelector: DefaultTrackSelector): ExoPlayer {
val dataSourceFactory = DefaultDataSource.Factory(context)
val audioAttributes = AudioAttributes.Builder()
.setUsage(C.USAGE_MEDIA)
Expand All @@ -532,22 +527,38 @@ object PlayerHelper {
}
}


/**
* Returns the recommended duration that should be buffer in milliseconds, based on the available memory.
*/
private fun getRecommendedBufferMs(): Int {
val activityManager =
LibreTubeApp.instance.getSystemService(Context.ACTIVITY_SERVICE) as ActivityManager
val memInfo = ActivityManager.MemoryInfo().also { memoryInfo ->
activityManager.getMemoryInfo(memoryInfo)
}
// device memory in MB
val availableMemory = memInfo.totalMem / (1024 * 1024)
return when {
availableMemory >= 8192 -> 1000 * 450
availableMemory >= 4096 -> 1000 * 300
availableMemory > 2048 -> 1000 * 60 * 2
else -> MINIMUM_BUFFER_DURATION
}
}

/**
* Get the load controls for the player (buffering, etc)
*/
@OptIn(androidx.media3.common.util.UnstableApi::class)
fun getLoadControl(): LoadControl {
return DefaultLoadControl.Builder()
// cache the last three minutes
.setBackBuffer(1000 * 60 * 3, true)
.setBufferDurationsMs(
MINIMUM_BUFFER_DURATION,
max(bufferingGoal, MINIMUM_BUFFER_DURATION),
DefaultLoadControl.DEFAULT_BUFFER_FOR_PLAYBACK_MS,
DefaultLoadControl.DEFAULT_BUFFER_FOR_PLAYBACK_AFTER_REBUFFER_MS
)
.build()
}
fun getLoadControl(): LoadControl = DefaultLoadControl.Builder()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this method is only called when the player is created for the first time, so it stays the same during autoplay or starting other videos (except if the current player is getting closed by the user).

But isn't it very likely that the available RAM is going to change when playing the next video, e.g. via autoplay?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method also isn't very accurate, since it doesn't include memory used by the kernel :)
Changed it to be based on the total memory that the device has.

Alternatively we could also just choose a value, but in either case I would like to remove this setting, as it's not really user-friendly or helpful.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for comments here but as a user I feel there should be two options in setting " auto" and "custom" . You can set "auto" by default but if some users want to change it they will. For me this setting save a lots of data uses

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR actually tries to simplify the preferences and remove "unnecessary"/"unintuitive" preferences that are not really easy to understand or useful.

So adding a new option to toggle between auto and custom would be like the opposite of what this PR is trying to achieve imho.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for comments here but as a user I feel there should be two options in setting " auto" and "custom" . You can set "auto" by default but if some users want to change it they will.

What's the issue with the (current) default, that requires you changing it?

For me this setting save a lots of data uses

The (network) data that is consumed is the same, it's just distributed differently.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to share my experience with adjusting the buffer size for video streaming. By default, the app has buffer size of 50 seconds, but due to limited data from my provider, I reduced it to 10 seconds.How does this save data? A smaller buffer size reduces data usage when I skip videos. For example, if I'm watching a video and decide I don’t like it after a few seconds, THEN I only consume 10 seconds of buffered data instead of 50 seconds. This prevents wasting data on content I don’t watch.
By the way I tested this PR with a 3-minute video, the app cached nearly the entire video in just a few seconds. If I stopped watching and switched to another video, the cached data for the first video was wasted. A larger buffer size means more data is downloaded upfront, which is inefficient if I frequently skip videos.This is why I recommend keeping a buffer size setting but allowing users to adjust it. A smaller buffer, like 10 seconds, balances smooth playback with minimal data waste, especially for users with limited data plans.

// cache the last three minutes
.setBackBuffer(1000 * 60 * 3, true).setBufferDurationsMs(
MINIMUM_BUFFER_DURATION,
getRecommendedBufferMs(),
DefaultLoadControl.DEFAULT_BUFFER_FOR_PLAYBACK_MS,
DefaultLoadControl.DEFAULT_BUFFER_FOR_PLAYBACK_AFTER_REBUFFER_MS
).build()

/**
* Load playback parameters such as speed and skip silence
Expand Down
2 changes: 0 additions & 2 deletions app/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,6 @@
<string name="show_updates">Check updates automatically</string>
<string name="related_streams">Related content</string>
<string name="related_streams_summary">Show similar streams alongside what you watch</string>
<string name="buffering_goal">Preloading</string>
<string name="buffering_goal_summary">Max. amount of seconds of video to buffer</string>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In any case we will also need to remove all translations of this because otherwise there are likely going to be issues with Weblate

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assumed Weblate would manage that itself when someone updates the translations for that language? It's not that much work, but it seems something that should be done automatically (e.g. it is for .po files).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not 100% sure. In the past, I already had build issues due to some string resources messed up by Weblate, but I'm not 100% what used to cause it back then.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's what happens if you only remove the source string: https://github.com/you-apps/WallYou/actions/runs/14822641921/job/41611687792#step:4:300. So no, Weblate doesn't seem to handle it.

<string name="playerVideoFormat">Video format for player</string>
<string name="no_audio">No audio</string>
<string name="no_video">No video</string>
Expand Down
7 changes: 0 additions & 7 deletions app/src/main/res/xml/player_settings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -166,13 +166,6 @@

<PreferenceCategory app:title="@string/misc">

<com.github.libretube.ui.preferences.EditNumberPreference
android:icon="@drawable/ic_time"
app:defaultValue="50"
app:key="buffering_goal"
app:summary="@string/buffering_goal_summary"
app:title="@string/buffering_goal" />

<SwitchPreferenceCompat
android:icon="@drawable/ic_call"
android:summary="@string/playback_during_call_summary"
Expand Down
Loading