-
-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Disable video and text tracks in background player and prefer DASH manifests over HLS ones for livestreams #12601
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
base: dev
Are you sure you want to change the base?
Disable video and text tracks in background player and prefer DASH manifests over HLS ones for livestreams #12601
Conversation
This is a hacky solution, a better one should be investigated and used.
As both subtitles and video tracks are disabled in this method, the goal of this rename is to highlight disabling/enabled subtitles.
This allows disabling these track types when stream info has been not loaded while the ExoPlayer instance is. It is now possible to do so with the background player, in order to disable fetching video and text tracks for manifest sources, especially used for livestreams. Also set the recovery first before reloading play queue manager in the useVideoAndSubtitles method of the Player class.
This reduces data usage for manifest sources with demuxed audio and video, such as livestreams, for non-HLS sources only due to an ExoPlayer bug.
Stypox
left a comment
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.
Looks good to me, thank you! I just have a couple of small questions/nitpicks, but then this can be merged
| return buildLiveMediaSource( | ||
| dataSource, info.getDashMpdUrl(), C.CONTENT_TYPE_DASH, tag); | ||
| } | ||
| if (!info.getHlsUrl().isEmpty()) { |
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.
Maybe add a comment explaining that we prefer Dash over HLS because of the exoplayer bug (which I guess is the reason why you reordered here).
| import org.schabi.newpipe.player.Player; | ||
|
|
||
| /** | ||
| * This is not a real UI for the background player, it used to disable fetching video and text |
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.
| * This is not a real UI for the background player, it used to disable fetching video and text | |
| * This is not a "graphical" UI for the background player, but it is used to disable fetching video and text |
| setRecovery(); | ||
| reloadPlayQueueManager(); |
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.
What is the reason behind swapping setRecovery() and reloadPlayQueueManager() here and above?
| }, () -> { | ||
| /* | ||
| The current metadata may be null sometimes (for e.g. when using an unstable connection | ||
| in livestreams) so we will be not able to execute the block below |
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.
Unrelated, but I guess this comment should say "the block above"
What is it?
Description of the changes in your PR
This PR prioritizes DASH manifests over HLS ones for live sources, and works around bugged YouTube DASH manifests in order to use them. This workaround has been only tested for running livestreams and not running premieres (i.e. in the time the video can be played as a livestream during its release), so please test if you can get a running premiere. YouTube DASH manifests allow to rewind up to 4 hours to the current live time.
It also disables fetching video and text tracks in background player, in order to reduce data usage for livestreams on services on which we can get DASH manifests. When there are only HLS ones, nothing will change, at least for video track fetching, due to an ExoPlayer bug which still fetches video (and subtitles?), even if it has been disabled with the track selector (a low priority has been given by the ExoPlayer team to this bug for several years without any change).
Fixes the following issue(s)
Fixes #12024 and fixes #11756 (for non-HLS sources).
Related issue: #10158
APK testing
The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR. You can find more info and a video demonstration on this wiki page.
Due diligence