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

Fix fetch of video streams (when switching between tracks in a play queue) and subtitles when using a seamless transition between background and video players #8139

Conversation

AudricV
Copy link
Member

@AudricV AudricV commented Apr 3, 2022

What is it?

  • Bugfix (user facing)

Description of the changes in your PR

When doing #7349, I didn't notice, when switching tracks in a play queue, that the video stream was renabled and was so fetched in background. This PR fixes this, and thanks to the method setDisabledTrackTypes in DefaultTrackSelector.ParametersBuilder, now available because we are using the latest version of ExoPlayer, we can now easily disable video tracks in background for all streams in a play queue (even if they can be fetched for initialization purposes (requests tends to turn around some kilobytes), which is the case for ProgressiveMediaSources, right now used to fetch all contents in the app).

I also didn't noticed that subtitles were still fetched in background, which is now fixed too with this PR.

Remember that if you want a real background player when pausing the app, you have to use the Background command.

Fixes the following issue(s)

No issue was opened for these bugs.

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.

Due diligence

…ueue) and subtitles when using a seamless transition between background and video players

Make the use of the new method setDisabledTrackTypes in DefaultTrackSelector.ParametersBuilder, which disables selection of tracks type for every TrackGroup instead of the current group, which is the current behavior.
This removes the use of the deprecated of setSelectionOverride method.

Note that for progressive media, the content is still fetched, but only for initialization purposes (so requests are pretty small, most of times with a few kilobytes size).
@AudricV AudricV added bug Issue is related to a bug player Issues related to any player (main, popup and background) labels Apr 3, 2022
@Redirion
Copy link
Member

Redirion commented Apr 7, 2022

I really appreciate your effort to make this PR testable, however the code change looks clean and small enough to assess that this can be merged without issues as it is basically just affecting the background player. I like it and would like to merge it. Please remove the testing code. Thank you! :)

@AudricV
Copy link
Member Author

AudricV commented Apr 7, 2022

Done :)

@AudricV AudricV marked this pull request as ready for review April 7, 2022 14:50
@AudricV AudricV force-pushed the seamless-transition-video-subtitles-fetch-fix branch from ed45295 to 3261855 Compare April 7, 2022 14:50
@AudricV AudricV requested a review from Redirion April 7, 2022 14:50
@sonarqubecloud
Copy link

sonarqubecloud bot commented Apr 7, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

Copy link
Member

@Redirion Redirion left a comment

Choose a reason for hiding this comment

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

lgtm

@Redirion Redirion merged commit e16917f into TeamNewPipe:dev Apr 7, 2022
@AudricV AudricV deleted the seamless-transition-video-subtitles-fetch-fix branch April 7, 2022 15:14
@Stypox Stypox mentioned this pull request Apr 16, 2022
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is related to a bug player Issues related to any player (main, popup and background)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants